From d71e373b16f5753e991c2d49789c6e99c068c254 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Nogueira?= Date: Thu, 16 Apr 2026 18:23:14 -0300 Subject: [PATCH 1/5] Remove redundant refCount setting file->refCount is already set to 'parse+save' in the pollTask loop that initializes all file objects. --- eigerApp/src/eigerDetector.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/eigerApp/src/eigerDetector.cpp b/eigerApp/src/eigerDetector.cpp index cf6e6ba..e228801 100644 --- a/eigerApp/src/eigerDetector.cpp +++ b/eigerApp/src/eigerDetector.cpp @@ -1195,8 +1195,6 @@ void eigerDetector::downloadTask (void) FLOW_ARGS("file=%s", file->name); - file->refCount = file->parse + file->save; - // Download the file if(mApi.getFile(file->name, &file->data, &file->len)) { From e3ab376a61a4bd428b827a35e8b2d60a7b6c03df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Nogueira?= Date: Wed, 22 Apr 2026 17:12:13 -0300 Subject: [PATCH 2/5] Fix FileWriter waiting in controlTask The original implementation had three main issues: - It called mFWState.get() without a corresponding fetch call, so it wasn't updating the value of the FileWriter state. Considering eigerStatus() doesn't update this value during an ongoing acquisition, it likely would be set to "ready" during this loop. - mFWState.get(), which reads a parameter from the param library, was called without holding the port lock. - It wouldn't update the FWState PV with the new value. These issues were solved with this commit. It was decided to keep the locking scheme around waiting for the FileWriter and Stream tasks the same, since these are conditional code paths, and it's simpler to reason about them by knowing we always start without holding the lock. We also added some rate limiting to the mFWState requests, using the same 0.1s delay as other driver tasks. The hardcoded 0.5s sleep, which was added in 960e6cc (Fix acquisition with External Enable mode, 2017-01-20) and increased in a4a4c30 (Major rewrite of how Eiger parameters are handled, 2017-04-17) was kept, since it's unclear if it can be removed. --- eigerApp/src/eigerDetector.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/eigerApp/src/eigerDetector.cpp b/eigerApp/src/eigerDetector.cpp index e228801..bec6952 100644 --- a/eigerApp/src/eigerDetector.cpp +++ b/eigerApp/src/eigerDetector.cpp @@ -1045,11 +1045,17 @@ void eigerDetector::controlTask (void) { // Wait FileWriter to go out of the "acquire" state FLOW("waiting for FileWriter"); - string fwAcquire; - do + + for(;;) { - mFWState->get(fwAcquire); - }while(fwAcquire == "acquire"); + string fwAcquire; + lock(); + mFWState->fetch(fwAcquire); + callParamCallbacks(); + unlock(); + if (fwAcquire != "acquire") break; + epicsThreadSleep(0.1); + } epicsThreadSleep(0.5); // Request polling task to stop From b8ecd8db10b9490fe2dc60a6b44261477443c7b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Nogueira?= Date: Thu, 16 Apr 2026 19:45:18 -0300 Subject: [PATCH 3/5] Replace numImagesCounter with detector state in controlTask The comment in the branch that handled TMExternalSeries and TMExternalEnable trigger modes was misleading, since it claimed the Eiger does not indicate when an acquisition is complete, even though the detector state parameter is available. This fix is important for the trigger modes above because otherwise they won't ever complete acquisitions where DataSource is None, because numImagesCounter won't be incremented. With this commit, such acquisitions can be completed. This logic, however, isn't specific to acquisitions with those trigger modes, so it can be applied to all of them equally, which simplifies the implementation. However, it also delays disarming the detector for the other trigger modes until the acquisition is over. Moving the disarm command to after waiting for the poll and stream tasks was considered, but that wouldn't have been enough if the driver was used to simply control the detector (DataSource is None and SaveFiles is Disabled). Doing this on top of the current commit, as a precaution, was also considered, but it would delay disarming and user feedback further for little gain. For the implementation, it was necessary to check for three possible states, because the detector state will be "configure" when sending the acquisition commands, then, when waiting for triggers, will be "ready", and ongoing acquisitions will indicate "acquiring". The change to the verification loop also allowed us to remove the epicsThreadSleep call and sleep while waiting for mStopEvent, because we can simply relinquish the lock for its entire duration and for the disarm command afterward. Fetching the state in controlTask also allowed us to include a callParamCallbacks call afterward, allowing the State PV to be updated during this part of the acquisition (since eigerStatus() currently doesn't do anything during acquisitions). It should be noted, however, that when using internal triggers, the RestAPI::trigger method waits enough time for the acquisition to complete, so the state should be "idle" in those cases. --- eigerApp/src/eigerDetector.cpp | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/eigerApp/src/eigerDetector.cpp b/eigerApp/src/eigerDetector.cpp index bec6952..a7163d4 100644 --- a/eigerApp/src/eigerDetector.cpp +++ b/eigerApp/src/eigerDetector.cpp @@ -1011,26 +1011,25 @@ void eigerDetector::controlTask (void) getIntegerParam(ADStatus, &adStatus); } } - else // TMExternalSeries or TMExternalEnable + + // Wait for detector to stop acquiring. + // Essential for TMExternalSeries or TMExternalEnable, + // which have to wait for external action. + FLOW("waiting for detector state"); + string state; + for(;;) { - // The Eiger does not indicate when acquisition is complete. - // Wait either until the NumImagesCounter is the expected value or - // until there is a manual stop event. - int expectedImages = numImages * numTriggers; - int numImagesCounter; - for(;;) - { - getIntegerParam(ADNumImagesCounter, &numImagesCounter); - if (numImagesCounter >= expectedImages) break; - if (mStopEvent.tryWait()) break; - unlock(); - epicsThreadSleep(0.1); - lock(); - } + mState->fetch(state); + callParamCallbacks(); + // We must exit this loop without the lock + unlock(); + if (state != "configure" && state != "ready" && state != "acquire") break; + if (mStopEvent.wait(0.1)) break; + // If we haven't exited yet, grab the lock so we can update the param library + lock(); } // All triggers issued, disarm the detector - unlock(); status = mApi.disarm(); lock(); From 0046208b6fc10ae8c38d85ec97de97076736a2b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Nogueira?= Date: Wed, 22 Apr 2026 16:58:21 -0300 Subject: [PATCH 4/5] Document synchronization of boolean flags mPollStop doesn't have a comment because access to it is currently not properly protected. mPollComplete is reset when writes to mPollQueue start, set before mPollDoneEvent is signaled, and read after waiting for mPollDoneEvent. mStreamComplete is reset before mStreamEvent is signaled, set before mStreamDoneEvent is signaled, and read after waiting for mStreamDoneEvent. --- eigerApp/src/eigerDetector.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/eigerApp/src/eigerDetector.h b/eigerApp/src/eigerDetector.h index 551c1e9..d72546a 100644 --- a/eigerApp/src/eigerDetector.h +++ b/eigerApp/src/eigerDetector.h @@ -269,7 +269,11 @@ class eigerDetector : public ADDriver mPollDoneEvent, mRestartEvent, mInitializeEvent; epicsMessageQueue mPollQueue, mDownloadQueue, mParseQueue, mSaveQueue, mReapQueue; - bool mPollStop, mPollComplete, mStreamComplete; + bool mPollStop; + // Access to this variable is synchronized by mPollQueue and mPollDoneEvent + bool mPollComplete; + // Access to this variable is synchronized by mStreamEvent and mStreamDoneEvent + bool mStreamComplete; unsigned int mFrameNumber; uid_t mFsUid, mFsGid; EigerParamSet mParams; From 11d75a27280ae5c443b2fb93e14d5665c587922a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89rico=20Nogueira?= Date: Thu, 23 Apr 2026 11:18:58 -0300 Subject: [PATCH 5/5] Synchronize access to mPollStop mPollStop is used from controlTask to signal pollTask to stop checking for data files, so reads and writes to it should be properly synchronized between these threads. Since it's only a flag, the simplest solution for this is making it atomic. To avoid complicating usage of the variable, we decided not to use stores and loads with explicit memory ordering, though it should be possible to use relaxed atomics for this purpose. --- eigerApp/src/eigerDetector.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eigerApp/src/eigerDetector.h b/eigerApp/src/eigerDetector.h index d72546a..8831485 100644 --- a/eigerApp/src/eigerDetector.h +++ b/eigerApp/src/eigerDetector.h @@ -1,6 +1,7 @@ #ifndef EIGER_DETECTOR_H #define EIGER_DETECTOR_H +#include #include #include @@ -269,7 +270,7 @@ class eigerDetector : public ADDriver mPollDoneEvent, mRestartEvent, mInitializeEvent; epicsMessageQueue mPollQueue, mDownloadQueue, mParseQueue, mSaveQueue, mReapQueue; - bool mPollStop; + std::atomic mPollStop; // Access to this variable is synchronized by mPollQueue and mPollDoneEvent bool mPollComplete; // Access to this variable is synchronized by mStreamEvent and mStreamDoneEvent