From 488c2e8066c538e496ff5d7caf8655a065525976 Mon Sep 17 00:00:00 2001 From: Bill Somerville Date: Mon, 16 Mar 2020 13:58:38 +0000 Subject: [PATCH] Cleanup and refactor Fortran shared memory usage also added some missing locking for the ipc(1) value in ft8_decode(). --- CMakeLists.txt | 11 +++----- Detector/Detector.cpp | 2 ++ commons.h | 12 +++++---- getfile.cpp | 2 ++ lib/ft8_decode.f90 | 23 +++++++++++++--- lib/ipcomm.cpp | 48 ---------------------------------- lib/jt9a.f90 | 59 +++++++++++++++++++----------------------- lib/shmem.cpp | 20 ++++++++++++++ widgets/mainwindow.cpp | 12 ++++----- widgets/plotter.cpp | 2 ++ 10 files changed, 87 insertions(+), 104 deletions(-) delete mode 100644 lib/ipcomm.cpp create mode 100644 lib/shmem.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 86d0ba604..89b278489 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -224,6 +224,7 @@ set (WSJT_QT_CONF_DESTINATION ${QT_CONF_DESTINATION} CACHE PATH "Path for the qt # set (wsjt_qt_CXXSRCS qt_helpers.cpp + lib/shmem.cpp widgets/MessageBox.cpp MetaDataRegistry.cpp Network/NetworkServerLookup.cpp @@ -298,10 +299,6 @@ set (jt9_FSRCS lib/jt9a.f90 ) -set (jt9_CXXSRCS - lib/ipcomm.cpp - ) - set (wsjtx_CXXSRCS logbook/logbook.cpp Network/psk_reporter.cpp @@ -360,6 +357,7 @@ endif (WIN32) set (wsjt_FSRCS # put module sources first in the hope that they get rebuilt before use + lib/shmem.f90 lib/crc.f90 lib/fftw3mod.f90 lib/hashing.f90 @@ -715,7 +713,6 @@ set (all_CXXSRCS ${wsjt_CXXSRCS} ${wsjt_qt_CXXSRCS} ${wsjt_qtmm_CXXSRCS} - ${jt9_CXXSRCS} ${wsjtx_CXXSRCS} ${qcp_CXXSRCS} ) @@ -1221,7 +1218,7 @@ add_executable (fcal lib/fcal.f90 wsjtx.rc) add_executable (fmeasure lib/fmeasure.f90 wsjtx.rc) -add_executable (jt9 ${jt9_FSRCS} ${jt9_CXXSRCS} wsjtx.rc) +add_executable (jt9 ${jt9_FSRCS} wsjtx.rc) if (${OPENMP_FOUND} OR APPLE) if (APPLE) # On Mac we don't have working OpenMP support in the C/C++ @@ -1254,7 +1251,7 @@ if (${OPENMP_FOUND} OR APPLE) LINK_FLAGS -Wl,--stack,16777216 ) endif () - target_link_libraries (jt9 wsjt_fort_omp wsjt_cxx Qt5::Core) + target_link_libraries (jt9 wsjt_fort_omp wsjt_cxx wsjt_qt) else (${OPENMP_FOUND} OR APPLE) target_link_libraries (jt9 wsjt_fort wsjt_cxx Qt5::Core) endif (${OPENMP_FOUND} OR APPLE) diff --git a/Detector/Detector.cpp b/Detector/Detector.cpp index 0cd0714e7..06b9c6b2d 100644 --- a/Detector/Detector.cpp +++ b/Detector/Detector.cpp @@ -11,6 +11,8 @@ extern "C" { void fil4_(qint16*, qint32*, qint16*, qint32*); } +extern dec_data_t dec_data; + Detector::Detector (unsigned frameRate, double periodLengthInSeconds, unsigned downSampleFactor, QObject * parent) : AudioDevice (parent) diff --git a/commons.h b/commons.h index 04be9db2d..24441ea06 100644 --- a/commons.h +++ b/commons.h @@ -7,9 +7,7 @@ #ifdef __cplusplus #include -extern "C" { -#endif -#ifndef __cplusplus +#else #include #endif @@ -17,7 +15,7 @@ extern "C" { * This structure is shared with Fortran code, it MUST be kept in * sync with lib/jt9com.f90 */ -extern struct dec_data { +typedef struct dec_data { int ipc[3]; float ss[184*NSMAX]; float savg[NSMAX]; @@ -67,7 +65,11 @@ extern struct dec_data { char hiscall[12]; char hisgrid[6]; } params; -} dec_data; +} dec_data_t; + +#ifdef __cplusplus +extern "C" { +#endif extern struct { float syellow[NSMAX]; diff --git a/getfile.cpp b/getfile.cpp index 96c76ad8a..3b1059e35 100644 --- a/getfile.cpp +++ b/getfile.cpp @@ -20,6 +20,8 @@ #include "commons.h" +extern dec_data dec_data; + void getfile(QString fname, int ntrperiod) { // struct WAVHDR { diff --git a/lib/ft8_decode.f90 b/lib/ft8_decode.f90 index 41b8555b7..ab004571f 100644 --- a/lib/ft8_decode.f90 +++ b/lib/ft8_decode.f90 @@ -35,7 +35,10 @@ contains subroutine decode(this,callback,iwave,nQSOProgress,nfqso,nftx,newdat, & nutc,nfa,nfb,nzhsym,ndepth,ncontest,nagain,lft8apon,lapcqonly, & napwid,mycall12,hiscall12,hisgrid6,ipc1,ldiskdat) + use iso_c_binding, only: c_bool, c_int use timer_module, only: timer + use shmem, only: shmem_lock, shmem_unlock + include 'ft8/ft8_params.f90' class(ft8_decoder), intent(inout) :: this @@ -49,6 +52,7 @@ contains logical, intent(in) :: lft8apon,lapcqonly,nagain logical newdat,lsubtract,ldupe,lrefinedt logical*1 ldiskdat + integer(c_int), volatile, intent(inout) :: ipc1 logical lsubtracted(MAX_EARLY) character*12 mycall12,hiscall12 character*6 hisgrid6 @@ -62,9 +66,10 @@ contains integer itime(8) real f1_save(MAX_EARLY) real xdt_save(MAX_EARLY) + integer(c_int) :: ihsym + logical(c_bool) :: ok save s,dd,dd1,ndec_early,itone_save,f1_save,xdt_save,lsubtracted - volatile ipc1 this%callback => callback write(datetime,1001) nutc !### TEMPORARY ### @@ -90,7 +95,12 @@ contains lrefinedt) lsubtracted(i)=.true. endif - if(.not.ldiskdat .and. ipc1.ge.49) then !Bail out before done + ok=shmem_lock() + if(.not.ok) call abort + ihsym=ipc1 !read latest from shared memory + ok=shmem_unlock() + if(.not.ok) call abort + if(.not.ldiskdat .and. ihsym.ge.49) then !Bail out before done call timer('sub_ft8b',1) dd1=dd go to 700 @@ -176,8 +186,13 @@ contains call this%callback(sync,nsnr,xdt,f1,msg37,iaptype,qual) endif endif + ok=shmem_lock() + if(.not.ok) call abort + ihsym=ipc1 !read latest from shared memory + ok=shmem_unlock() + if(.not.ok) call abort if(.not.ldiskdat .and. nzhsym.eq.41 .and. & - ipc1.ge.46) go to 700 !Bail out before done + ihsym.ge.46) go to 700 !Bail out before done enddo enddo go to 800 @@ -189,7 +204,7 @@ contains tseq=mod(itime(7)+0.001*itime(8),15.0) if(tseq.lt.9.0) tseq=tseq+15.0 sec=itime(7)+0.001*itime(8) - write(71,3001) 'CC Bailout ',tsec,nzhsym,ipc1,tseq, & + write(71,3001) 'CC Bailout ',tsec,nzhsym,ihsym,tseq, & itime(5)-itime(4)/60,itime(6),sec,ndecodes 3001 format(a15,f11.3,2i6,f8.3,i4.2,':',i2.2,':',f6.3,i6) flush(71) diff --git a/lib/ipcomm.cpp b/lib/ipcomm.cpp deleted file mode 100644 index 6f65dcae9..000000000 --- a/lib/ipcomm.cpp +++ /dev/null @@ -1,48 +0,0 @@ -#include -#include -#include -#include - -#include "../commons.h" - -// Multiple instances: KK1D, 17 Jul 2013 -QSharedMemory mem_jt9; - -// Semaphore not changed, as the acquire/release calls do not -// appear to be used anywhere. -QSystemSemaphore sem_jt9("sem_jt9", 1, QSystemSemaphore::Open); - -extern "C" { - bool attach_jt9_(); - bool create_jt9_(int nsize); - bool detach_jt9_(); - bool lock_jt9_(); - bool unlock_jt9_(); - struct jt9com * address_jt9_(); - int size_jt9_(); -// Multiple instances: wrapper for QSharedMemory::setKey() - bool setkey_jt9_(char* mykey, int mykey_len); - - bool acquire_jt9_(); - bool release_jt9_(); -} - -bool attach_jt9_() {return mem_jt9.attach();} -bool create_jt9_(int nsize) {return mem_jt9.create(nsize);} -bool detach_jt9_() {return mem_jt9.detach();} -bool lock_jt9_() {return mem_jt9.lock();} -bool unlock_jt9_() {return mem_jt9.unlock();} -struct jt9com * address_jt9_() {return reinterpret_cast(mem_jt9.data());} -int size_jt9_() {return (int)mem_jt9.size();} - -// Multiple instances: -bool setkey_jt9_(char* mykey, int mykey_len) { - char *tempstr = (char *)calloc(mykey_len+1,1); - memset(tempstr, 0, mykey_len+1); - strncpy(tempstr, mykey, mykey_len); - QString s1 = QString(QLatin1String(tempstr)); - mem_jt9.setKey(s1); - return true;} - -bool acquire_jt9_() {return sem_jt9.acquire();} -bool release_jt9_() {return sem_jt9.release();} diff --git a/lib/jt9a.f90 b/lib/jt9a.f90 index 917ebf3e8..38ce978b3 100644 --- a/lib/jt9a.f90 +++ b/lib/jt9a.f90 @@ -1,30 +1,17 @@ subroutine jt9a() - use, intrinsic :: iso_c_binding, only: c_f_pointer + use, intrinsic :: iso_c_binding, only: c_f_pointer, c_null_char, c_bool use prog_args use timer_module, only: timer use timer_impl, only: init_timer !, limtrace + use shmem include 'jt9com.f90' -! These routines connect the shared memory region to the decoder. - interface - function address_jt9() - use, intrinsic :: iso_c_binding, only: c_ptr - type(c_ptr) :: address_jt9 - end function address_jt9 - end interface - integer*2 id2a(180000) - integer*1 attach_jt9 - integer size_jt9 ! Multiple instances: - character*80 mykey - type(dec_data), pointer :: shared_data + type(dec_data), pointer, volatile :: shared_data !also makes target volatile type(params_block) :: local_params - volatile shared_data - -! Multiple instances: - i0 = len(trim(shm_key)) + logical(c_bool) :: ok call init_timer (trim(data_dir)//'/timer.out') ! open(23,file=trim(data_dir)//'/CALL3.TXT',status='unknown') @@ -32,36 +19,39 @@ subroutine jt9a() ! limtrace=-1 !Disable all calls to timer() ! Multiple instances: set the shared memory key before attaching - mykey=trim(repeat(shm_key,1)) - i0 = len(mykey) - i0=setkey_jt9(trim(mykey)) - i1=attach_jt9() + call shmem_setkey(trim(shm_key)//c_null_char) + ok=shmem_attach() + if(.not.ok) call abort msdelay=30 - call c_f_pointer(address_jt9(),shared_data) + call c_f_pointer(shmem_address(),shared_data) ! Wait here until GUI has set ips(2) to 1.0 -10 call lock_jt9() +10 ok=shmem_lock() + if(.not.ok) call abort if(shared_data%ipc(2).eq.999.0) then - call unlock_jt9() - i1=detach_jt9() + ok=shmem_unlock() + ok=shmem_detach() go to 999 endif if(shared_data%ipc(2).ne.1.0) then - call unlock_jt9() + ok=shmem_unlock() + if(.not.ok) call abort call sleep_msec(msdelay) go to 10 endif shared_data%ipc(2)=0 - nbytes=size_jt9() + nbytes=shmem_size() if(nbytes.le.0) then - call unlock_jt9() - print*,'jt9a: Shared memory mem_jt9 does not exist.' + ok=shmem_unlock() + ok=shmem_detach() + print*,'jt9a: Shared memory does not exist.' print*,"Must start 'jt9 -s ' from within WSJT-X." go to 999 endif local_params=shared_data%params !save a copy because wsjtx carries on accessing - call unlock_jt9() + ok=shmem_unlock() + if(.not.ok) call abort call flush(6) call timer('decoder ',0) if(local_params%nmode.eq.8 .and. local_params%ndiskdat) then @@ -87,14 +77,17 @@ subroutine jt9a() ! Wait here until GUI routine decodeDone() has set ipc(3) to 1.0 -100 call lock_jt9() +100 ok=shmem_lock() + if(.not.ok) call abort if(shared_data%ipc(3).ne.1.0) then - call unlock_jt9() + ok=shmem_unlock() + if(.not.ok) call abort call sleep_msec(msdelay) go to 100 endif shared_data%ipc(3)=0 - call unlock_jt9() + ok=shmem_unlock() + if(.not.ok) call abort go to 10 999 call timer('decoder ',101) diff --git a/lib/shmem.cpp b/lib/shmem.cpp new file mode 100644 index 000000000..95a86614f --- /dev/null +++ b/lib/shmem.cpp @@ -0,0 +1,20 @@ +#include +#include + +// Multiple instances: KK1D, 17 Jul 2013 +QSharedMemory shmem; + +struct jt9com; + +// C wrappers for a QSharedMemory class instance +extern "C" +{ + bool shmem_create (int nsize) {return shmem.create(nsize);} + void shmem_setkey (char * const mykey) {shmem.setKey(QLatin1String{mykey});} + bool shmem_attach () {return shmem.attach();} + int shmem_size () {return static_cast (shmem.size());} + struct jt9com * shmem_address () {return reinterpret_cast(shmem.data());} + bool shmem_lock () {return shmem.lock();} + bool shmem_unlock () {return shmem.unlock();} + bool shmem_detach () {return shmem.detach();} +} diff --git a/widgets/mainwindow.cpp b/widgets/mainwindow.cpp index 1d9436b7f..d8599338f 100644 --- a/widgets/mainwindow.cpp +++ b/widgets/mainwindow.cpp @@ -178,7 +178,7 @@ extern "C" { int volatile itone[NUM_ISCAT_SYMBOLS]; //Audio tones for all Tx symbols int volatile itone0[NUM_ISCAT_SYMBOLS]; //Dummy array, data not actually used int volatile icw[NUM_CW_SYMBOLS]; //Dits for CW ID -struct dec_data dec_data; // for sharing with Fortran +dec_data_t dec_data; // for sharing with Fortran int outBufSize; int rc; @@ -3072,13 +3072,11 @@ void::MainWindow::fast_decode_done() void MainWindow::to_jt9(qint32 n, qint32 istart, qint32 idone) { - int ipc[3]; + dec_data_t * dd = reinterpret_cast (mem_jt9->data()); mem_jt9->lock (); - memcpy(ipc,(char*)mem_jt9->data(),12); - ipc[0]=n; - if(istart>=0) ipc[1]=istart; - if(idone>=0) ipc[2]=idone; - memcpy((char*)mem_jt9->data(),ipc,12); + dd->ipc[0]=n; + if(istart>=0) dd->ipc[1]=istart; + if(idone>=0) dd->ipc[2]=idone; mem_jt9->unlock (); } diff --git a/widgets/plotter.cpp b/widgets/plotter.cpp index 203635430..74fc2c020 100644 --- a/widgets/plotter.cpp +++ b/widgets/plotter.cpp @@ -19,6 +19,8 @@ extern "C" { void plotsave_(float swide[], int* m_w , int* m_h1, int* irow); } +extern dec_data dec_data; + CPlotter::CPlotter(QWidget *parent) : //CPlotter Constructor QFrame {parent}, m_set_freq_action {new QAction {tr ("&Set Rx && Tx Offset"), this}},