From f4e7b1c7205ac63b124564b53e482afc8bc5658e Mon Sep 17 00:00:00 2001 From: Alex Landolt Date: Wed, 25 Mar 2026 16:36:14 +0100 Subject: [PATCH 1/5] NIDAQ: Implement SetGateOpen/GetGateOpen for DigitalOutputPort The DigitalOutputPort class (CStateDeviceBase) did not override SetGateOpen/GetGateOpen, so the State Device Shutter utility could not control the digital outputs as a software shutter. The default CStateDeviceBase gate methods had no effect on the hardware. This adds: - SetGateOpen: writes the current state (open) or Closed_Position (closed) to the port. When blanking or sequencing is active, only the internal flag is updated (hardware controls the outputs). - GetGateOpen: returns the stored gate state. - OnState: when blanking is active, always uses the requested state regardless of gate, since hardware controls on/off timing. This enables software-timed shutter control via the State Device Shutter when hardware blanking is disabled. Tested on a Ni-6321 with a Lumencor Spectra X --- DeviceAdapters/NIDAQ/NIDAQ.h | 4 ++ DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp | 49 ++++++++++++++++++-- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/DeviceAdapters/NIDAQ/NIDAQ.h b/DeviceAdapters/NIDAQ/NIDAQ.h index 73c4f876c..0349027e3 100644 --- a/DeviceAdapters/NIDAQ/NIDAQ.h +++ b/DeviceAdapters/NIDAQ/NIDAQ.h @@ -424,6 +424,10 @@ class DigitalOutputPort : public CStateDeviceBase, virtual unsigned long GetNumberOfPositions()const { return numPos_; } // replace with numPos_ once API allows not creating Labels + // Gate (shutter) interface + int SetGateOpen(bool open); + int GetGateOpen(bool& open); + // action interface // ---------------- int OnState(MM::PropertyBase* pProp, MM::ActionType eAct); diff --git a/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp b/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp index 575628568..3a1816765 100644 --- a/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp +++ b/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp @@ -211,6 +211,38 @@ void DigitalOutputPort::GetName(char* name) const } +int DigitalOutputPort::SetGateOpen(bool open) +{ + if (open == open_) + return DEVICE_OK; + + open_ = open; + + // When a hardware-timed task (blanking/sequencing) is active, + // just update the flag — hardware controls the outputs. + if (blanking_ || sequenceRunning_) + return DEVICE_OK; + + if (open) + { + return SetState(pos_); + } + else + { + long closedPosition; + GetProperty(MM::g_Keyword_Closed_Position, closedPosition); + return SetState(closedPosition); + } +} + + +int DigitalOutputPort::GetGateOpen(bool& open) +{ + open = open_; + return DEVICE_OK; +} + + int DigitalOutputPort::OnState(MM::PropertyBase* pProp, MM::ActionType eAct) { if (eAct == MM::BeforeGet) @@ -229,9 +261,20 @@ int DigitalOutputPort::OnState(MM::PropertyBase* pProp, MM::ActionType eAct) if ((pos == pos_) && (open_ == gateOpen)) return DEVICE_OK; - long closed_state; - GetProperty(MM::g_Keyword_Closed_Position, closed_state); - long newState = gateOpen ? pos : closed_state; + // When blanking is active, hardware controls on/off via the trigger + // input, so always use the requested state. Gate only applies in + // software-timed mode (blanking off). + long newState; + if (blanking_) + { + newState = pos; + } + else + { + long closed_state; + GetProperty(MM::g_Keyword_Closed_Position, closed_state); + newState = gateOpen ? pos : closed_state; + } // pause blanking, otherwise most cards will error int err; From 990cdbbee92de6343287c4d48fc51f479707e601 Mon Sep 17 00:00:00 2001 From: Alex Landolt Date: Wed, 25 Mar 2026 16:37:22 +0100 Subject: [PATCH 2/5] PVCAM: Call AcqFinished in all acquisition stop paths abortAcquisitionInternal() only called GetCoreCallback()->AcqFinished() in the circular-buffer + callbacks-enabled path. The polling thread and non-circular-buffer (AcqThread) paths did not notify the core, so AutoShutter never closed the shutter when live mode was stopped. This caused the shutter (e.g. State Device Shutter wrapping NIDAQ digital outputs) to remain open after stopping live acquisition, leaving illumination on indefinitely. Add AcqFinished() calls to the two missing paths, matching the existing behavior of the callbacks-enabled path. Usefull in combination with the NIDAQ patch when you want to use a NIDAQ without using blanking (I know quite a stupid combination, it's for a legacy reason in our lab) --- DeviceAdapters/PVCAM/PVCAMUniversal.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/DeviceAdapters/PVCAM/PVCAMUniversal.cpp b/DeviceAdapters/PVCAM/PVCAMUniversal.cpp index 1f9a9de2d..3a8a8118e 100644 --- a/DeviceAdapters/PVCAM/PVCAMUniversal.cpp +++ b/DeviceAdapters/PVCAM/PVCAMUniversal.cpp @@ -5716,11 +5716,17 @@ int Universal::abortAcquisitionInternal() { pollingThd_->setStop(true); pollingThd_->wait(); + // Notify the core that acquisition has finished so that + // AutoShutter can close the shutter. + GetCoreCallback()->AcqFinished(this, nRet); } } else { acqThd_->Pause(); + // Notify the core that acquisition has finished so that + // AutoShutter can close the shutter. + GetCoreCallback()->AcqFinished(this, nRet); } customDiskWriter_->Stop(); From cec8dfadd6fa11e5ed3c55943969e49e2fe6b207 Mon Sep 17 00:00:00 2001 From: Alex Landolt Date: Mon, 30 Mar 2026 20:58:00 +0200 Subject: [PATCH 3/5] NIDAQ: Fix label support for DigitalOutputPort, prevent hang on wide ports The original GetNumberOfPositions() returned numPos_ directly, which is the maximum state value (a bitmask). For 16/32-bit ports this is 65535 or 2+ billion. MMCore iterates 0..GetNumberOfPositions() in several places (getStateLabels, config save, HCW), causing hangs or crashes on cards like the NI-6323 that report 32-bit port widths. Changes: - Add Label property to DigitalOutputPort so state labels are visible in the property browser and usable for presets - Cap GetNumberOfPositions() to prevent MMCore enumeration hangs: - 8-bit ports (<=255 states): fully enumerable, all labels pre-populated - Wider ports: returns 0 by default; grows dynamically when labels are explicitly defined (e.g. via config file), up to a 65536 cap - Override GetPositionLabel() to return the numeric state as a default label when no explicit label exists, preventing "Invalid state (position) requested" errors during HCW and normal operation - Override SetPositionLabel() to automatically fill gaps with default numeric labels when a label is defined for a non-contiguous position (e.g. config file defines label for state 10 -> states 0-9 get auto-filled). This ensures MMCore's sequential iteration works. - Fix undefined behavior: (1 << 32) for 32-bit ports without blanking is UB in C++; now handled explicitly with 0x7FFFFFFF Behavior by port type: - 8-bit no blanking: numPos_=255, labels 0-255 pre-populated - 8-bit blanking: numPos_=127, labels 0-127 pre-populated - 16-bit no blanking: numPos_=65535, labels on demand via config - 16-bit blanking: numPos_=32767, labels on demand via config - 32-bit (any): numPos_=0x7FFFFFFF, labels on demand via config For wide ports, users can define labels in the config file like: Label,NIDAQDO-Dev1/port0,5,Laser488 These labels and auto-filled gaps survive config save (up to cap). The State property limits still allow the full bitmask value range regardless of the label cap. --- DeviceAdapters/NIDAQ/NIDAQ.h | 47 +++++++++++++++++++- DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp | 17 ++++++- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/DeviceAdapters/NIDAQ/NIDAQ.h b/DeviceAdapters/NIDAQ/NIDAQ.h index 0349027e3..3d7cf860e 100644 --- a/DeviceAdapters/NIDAQ/NIDAQ.h +++ b/DeviceAdapters/NIDAQ/NIDAQ.h @@ -422,7 +422,51 @@ class DigitalOutputPort : public CStateDeviceBase, virtual void GetName(char* name) const; virtual bool Busy() { return false; } - virtual unsigned long GetNumberOfPositions()const { return numPos_; } // replace with numPos_ once API allows not creating Labels + // For 8-bit ports, all states are enumerable. For wider ports, only + // enumerate positions that have been explicitly labeled (e.g. via config + // file). This prevents MMCore from iterating billions of states. + // The State property limits still allow the full value range. + static const long maxEnumerablePositions_ = 65536; + virtual unsigned long GetNumberOfPositions()const { + if (numPos_ <= 255) + return numPos_ + 1; + return (highestLabeledPos_ >= 0 && highestLabeledPos_ < maxEnumerablePositions_) + ? (unsigned long)(highestLabeledPos_ + 1) : 0; + } + + // Return the numeric string as default when no explicit label exists. + // This prevents errors when the Label property is read for unlabeled states. + virtual int GetPositionLabel(long pos, char* label) const + { + int ret = CStateDeviceBase::GetPositionLabel(pos, label); + if (ret != DEVICE_OK) + { + CDeviceUtils::CopyLimitedString(label, std::to_string(pos).c_str()); + return DEVICE_OK; + } + return ret; + } + + // Override to fill in default labels for gaps when labels are added + // beyond the auto-generated range (e.g. from config file on wide ports). + virtual int SetPositionLabel(long pos, const char* label) + { + // Fill in default labels for any positions below pos that have no label + if (pos > highestLabeledPos_ && pos < maxEnumerablePositions_) + { + char buf[MM::MaxStrLength]; + for (long i = highestLabeledPos_ + 1; i < pos; i++) + { + // Only fill if no label exists yet + if (CStateDeviceBase::GetPositionLabel(i, buf) != DEVICE_OK) + { + CStateDeviceBase::SetPositionLabel(i, std::to_string(i).c_str()); + } + } + highestLabeledPos_ = pos; + } + return CStateDeviceBase::SetPositionLabel(pos, label); + } // Gate (shutter) interface int SetGateOpen(bool open); @@ -458,6 +502,7 @@ class DigitalOutputPort : public CStateDeviceBase, bool open_; long pos_; long numPos_; + long highestLabeledPos_; uInt32 portWidth_; long inputLine_; long firstStateSlider_; diff --git a/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp b/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp index 3a1816765..a2cbe2544 100644 --- a/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp +++ b/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp @@ -35,6 +35,7 @@ DigitalOutputPort::DigitalOutputPort(const std::string& port) : open_(true), pos_(0), numPos_(0), + highestLabeledPos_(-1), portWidth_(0), nrOfStateSliders_(4), inputLine_(8), @@ -142,13 +143,27 @@ int DigitalOutputPort::Initialize() } else { - numPos_ = (1 << portWidth_) - 1; + numPos_ = (portWidth_ >= 32) ? 0x7FFFFFFF : (1L << portWidth_) - 1; } pAct = new CPropertyAction(this, &DigitalOutputPort::OnState); CreateIntegerProperty("State", 0, false, pAct); SetPropertyLimits("State", 0, numPos_); + pAct = new CPropertyAction(this, &CStateBase::OnLabel); + CreateProperty(MM::g_Keyword_Label, "", MM::String, false, pAct); + + // For 8-bit ports, pre-populate labels for all states. + // For wider ports, labels are only created when explicitly set + // (e.g. via config file). The SetPositionLabel override fills gaps. + if (numPos_ <= 255) + { + for (long i = 0; i <= numPos_; i++) + { + SetPositionLabel(i, std::to_string(i).c_str()); + } + } + // In case someone left some pins high: SetState(0); From 2277a5448a37cbf8abf8fe762565f2eade5af9a5 Mon Sep 17 00:00:00 2001 From: Alex Landolt Date: Mon, 30 Mar 2026 21:14:01 +0200 Subject: [PATCH 4/5] NIDAQ DigitalOutputPort: fix gate state sync with base class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #907 review feedback: the adapter maintained its own open_ member variable separately from the base class gateOpen_, causing the two to go out of sync. - Remove open_ member; use base class gateOpen_ via GetGateOpen() - Remove GetGateOpen() override — base class version is sufficient - SetGateOpen() delegates to base class (which updates gateOpen_ and calls SetPosition -> OnState), except during sequencing where OnState rejects state changes - OnState reads gate state from base class GetGateOpen() instead of open_ --- DeviceAdapters/NIDAQ/NIDAQ.h | 2 - DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp | 44 ++++++-------------- 2 files changed, 12 insertions(+), 34 deletions(-) diff --git a/DeviceAdapters/NIDAQ/NIDAQ.h b/DeviceAdapters/NIDAQ/NIDAQ.h index 3d7cf860e..013f69d8a 100644 --- a/DeviceAdapters/NIDAQ/NIDAQ.h +++ b/DeviceAdapters/NIDAQ/NIDAQ.h @@ -470,7 +470,6 @@ class DigitalOutputPort : public CStateDeviceBase, // Gate (shutter) interface int SetGateOpen(bool open); - int GetGateOpen(bool& open); // action interface // ---------------- @@ -499,7 +498,6 @@ class DigitalOutputPort : public CStateDeviceBase, bool sequenceRunning_; bool blanking_; bool blankOnLow_; - bool open_; long pos_; long numPos_; long highestLabeledPos_; diff --git a/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp b/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp index a2cbe2544..f1857a362 100644 --- a/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp +++ b/DeviceAdapters/NIDAQ/NIDigitalOutputPort.cpp @@ -32,7 +32,6 @@ DigitalOutputPort::DigitalOutputPort(const std::string& port) : sequenceRunning_(false), blanking_(false), blankOnLow_(true), - open_(true), pos_(0), numPos_(0), highestLabeledPos_(-1), @@ -169,7 +168,6 @@ int DigitalOutputPort::Initialize() // Gate Closed Position CreateProperty(MM::g_Keyword_Closed_Position, "0", MM::Integer, false); - GetGateOpen(open_); if (supportsBlankingAndSequencing_ && (uint32_t) nrOfStateSliders_ >= portWidth_) { nrOfStateSliders_ = portWidth_ - 1; @@ -228,33 +226,18 @@ void DigitalOutputPort::GetName(char* name) const int DigitalOutputPort::SetGateOpen(bool open) { - if (open == open_) - return DEVICE_OK; - - open_ = open; - - // When a hardware-timed task (blanking/sequencing) is active, - // just update the flag — hardware controls the outputs. - if (blanking_ || sequenceRunning_) + // During blanking/sequencing, hardware controls the outputs via + // the trigger input. We cannot delegate to the base class because + // it calls SetPosition -> OnState, which rejects changes during + // sequencing. Just record the gate state for when we return to + // software-timed mode. + if (sequenceRunning_) return DEVICE_OK; - if (open) - { - return SetState(pos_); - } - else - { - long closedPosition; - GetProperty(MM::g_Keyword_Closed_Position, closedPosition); - return SetState(closedPosition); - } -} - - -int DigitalOutputPort::GetGateOpen(bool& open) -{ - open = open_; - return DEVICE_OK; + // Delegate to base class, which updates gateOpen_ and calls + // SetPosition -> OnState. OnState handles the blanking vs + // software-timed distinction. + return CStateDeviceBase::SetGateOpen(open); } @@ -269,12 +252,10 @@ int DigitalOutputPort::OnState(MM::PropertyBase* pProp, MM::ActionType eAct) if (sequenceRunning_) return ERR_SEQUENCE_RUNNING; - bool gateOpen; - GetGateOpen(gateOpen); long pos; pProp->Get(pos); - if ((pos == pos_) && (open_ == gateOpen)) - return DEVICE_OK; + bool gateOpen; + GetGateOpen(gateOpen); // When blanking is active, hardware controls on/off via the trigger // input, so always use the requested state. Gate only applies in @@ -306,7 +287,6 @@ int DigitalOutputPort::OnState(MM::PropertyBase* pProp, MM::ActionType eAct) if (err == DEVICE_OK) { pos_ = pos; - open_ = gateOpen; } else { From 5f81cbc0601f0b75c27a1d8c513ba48c3ea9692d Mon Sep 17 00:00:00 2001 From: Alex Landolt Date: Mon, 30 Mar 2026 21:14:32 +0200 Subject: [PATCH 5/5] PVCAM: Remove duplicate AcqFinished() call in polling path Address PR #907 review feedback: in the polling thread path of abortAcquisitionInternal(), PollingThreadExiting() already calls AcqFinished() before pollingThd_->wait() returns. The added call after wait() was a duplicate notification. The acqThd_ (non-circular-buffer) path addition is kept. AcqThread does not call AcqFinished(), so that was a genuinely missing notification needed for AutoShutter to close the shutter. --- DeviceAdapters/PVCAM/PVCAMUniversal.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/DeviceAdapters/PVCAM/PVCAMUniversal.cpp b/DeviceAdapters/PVCAM/PVCAMUniversal.cpp index 3a8a8118e..aed364425 100644 --- a/DeviceAdapters/PVCAM/PVCAMUniversal.cpp +++ b/DeviceAdapters/PVCAM/PVCAMUniversal.cpp @@ -5716,9 +5716,7 @@ int Universal::abortAcquisitionInternal() { pollingThd_->setStop(true); pollingThd_->wait(); - // Notify the core that acquisition has finished so that - // AutoShutter can close the shutter. - GetCoreCallback()->AcqFinished(this, nRet); + // AcqFinished() is already called by PollingThreadExiting() } } else