fix: speed up XLight iris commands and avoid response mismatch by Alpaca233 · Pull Request #509 · Cephla-Lab/Squid

Alpaca233 · 2026-03-06T21:14:54Z

Summary

Use write_and_read instead of write_and_check for iris commands — avoids retry loop when XLight prepends board address to response (e.g. B1 J800 instead of J800)
Reduce iris read_delay from 3s to 2s (saves 2s per channel switch when both irises change)
Skip hardware call when iris value hasn't changed (common case: re-selecting same channel)
Remove redundant init-time iris defaults (XLIGHT_ILLUMINATION_IRIS_DEFAULT, XLIGHT_EMISSION_IRIS_DEFAULT) — first channel switch applies values from acquisition config

Test plan

Verify iris commands complete without serial mismatch warnings
Verify channel switching applies correct iris values from config
Verify re-selecting the same channel skips iris hardware calls (no 2s delay)
Verify iris slider/spinbox changes still apply to hardware

🤖 Generated with Claude Code

- Use write_and_read instead of write_and_check for iris commands to avoid retry loop when XLight prepends board address (e.g. "B1 J800") - Reduce iris read_delay from 3s to 2s - Skip hardware call when iris value hasn't changed - Remove redundant init-time iris defaults (XLIGHT_ILLUMINATION_IRIS_DEFAULT, XLIGHT_EMISSION_IRIS_DEFAULT) — first channel switch applies config values Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR speeds up XLight iris commands by switching from write_and_check to write_and_read (avoiding retry loops when the XLight prepends a board address to its response), reducing the read_delay from 3s to 2s, and adding a short-circuit to skip hardware calls when the iris value is unchanged. It also removes the init-time iris default constants that are no longer needed since acquisition config applies values on first channel switch.

Changes:

Replace write_and_check with write_and_read for iris commands and reduce read_delay from 3s to 2s
Add early-return in set_illumination_iris and set_emission_iris when the requested value matches the current cached value
Remove XLIGHT_ILLUMINATION_IRIS_DEFAULT and XLIGHT_EMISSION_IRIS_DEFAULT constants and their init-time usage

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
software/control/serial_peripherals.py	Adds cached-value short-circuit to iris setters, switches to `write_and_read`, reduces read delay, removes init-time iris default calls
software/control/_def.py	Removes unused `XLIGHT_ILLUMINATION_IRIS_DEFAULT` and `XLIGHT_EMISSION_IRIS_DEFAULT` constants

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T21:18:47Z

+        if value == self.illumination_iris:
+            return self.illumination_iris


Bug: self.illumination_iris is never initialized in XLight.__init__. The removed code at lines 273-276 (old) used to call self.set_illumination_iris(XLIGHT_ILLUMINATION_IRIS_DEFAULT), which set this attribute. Now the very first call to set_illumination_iris will access self.illumination_iris on this line before it has ever been assigned, raising an AttributeError.

You need to initialize self.illumination_iris (and self.emission_iris) in __init__, similar to how XLight_Simulation.__init__ does it (see line 171-172). For example, add self.illumination_iris = None (or 0) in __init__, and use is not None or an appropriate sentinel comparison to handle the first call.

[Claude Code] Fixed in e26085c - initialized self.illumination_iris = 0 and self.emission_iris = 0 in XLight.init.

Copilot · 2026-03-06T21:18:48Z

+        if value == self.emission_iris:
+            return self.emission_iris


Bug: Same issue as set_illumination_iris — self.emission_iris is never initialized in XLight.__init__. The first call to this method will raise AttributeError when accessing self.emission_iris here. Initialize self.emission_iris in __init__ (e.g., to None) to fix this.

Suggested change

if value == self.emission_iris:

return self.emission_iris

current_emission_iris = getattr(self, "emission_iris", None)

if value == current_emission_iris:

return current_emission_iris

[Claude Code] Fixed in e26085c - same fix, both attributes initialized to 0 in init.

sliderReleased doesn't fire on click-to-position, only on drag release. Use valueChanged with isSliderDown() guard to catch both cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

XLIGHT_ILLUMINATION_IRIS_DEFAULT and XLIGHT_EMISSION_IRIS_DEFAULT in _def.py are now referenced by the confocal model registry and config generator instead of hardcoding 100.0 in multiple places. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T21:38:42Z

+# Iris defaults come from _def.py (XLIGHT_ILLUMINATION_IRIS_DEFAULT / XLIGHT_EMISSION_IRIS_DEFAULT)
+# This fallback is used when no confocal_config.yaml or model registry is available
+from control._def import XLIGHT_ILLUMINATION_IRIS_DEFAULT
+
+DEFAULT_IRIS_VALUE = float(XLIGHT_ILLUMINATION_IRIS_DEFAULT)


Missing constant: XLIGHT_ILLUMINATION_IRIS_DEFAULT is imported from control._def but does not exist in _def.py. This will cause an ImportError at module load time. The constant needs to be defined in _def.py.

Additionally, this import is placed mid-file (after top-level constants), which violates PEP 8 import ordering. It should be moved to the import block at the top of the file (lines 9-30).

[Claude Code] Fixed in eba12aa - moved import to top-level import block and replaced DEFAULT_IRIS_VALUE with per-iris ALL_IRIS_DEFAULTS dict.

Copilot · 2026-03-06T21:38:42Z

+# This fallback is used when no confocal_config.yaml or model registry is available
+from control._def import XLIGHT_ILLUMINATION_IRIS_DEFAULT
+
+DEFAULT_IRIS_VALUE = float(XLIGHT_ILLUMINATION_IRIS_DEFAULT)


Semantic mismatch: DEFAULT_IRIS_VALUE is used as the fallback for both illumination_iris and emission_iris (see lines 72 and 75), but this change derives it only from XLIGHT_ILLUMINATION_IRIS_DEFAULT. If the illumination and emission iris defaults are ever set to different values, the emission iris fallback would incorrectly use the illumination default. Consider keeping the original hardcoded 100.0 or using separate defaults for each iris property in the fallback paths.

[Claude Code] Fixed in eba12aa - replaced single DEFAULT_IRIS_VALUE with ALL_IRIS_DEFAULTS dict that maps each iris property to its own default from _def.py.

Copilot · 2026-03-06T21:38:42Z

+from control._def import XLIGHT_EMISSION_IRIS_DEFAULT, XLIGHT_ILLUMINATION_IRIS_DEFAULT
+



Missing constants: XLIGHT_ILLUMINATION_IRIS_DEFAULT and XLIGHT_EMISSION_IRIS_DEFAULT are imported from control._def but do not exist in _def.py. This will cause an ImportError at module load time, preventing the application from starting when confocal hardware is configured.

You need to add these constants to software/control/_def.py, e.g. near line 961 alongside the other XLIGHT_* constants.

Suggested change

from control._def import XLIGHT_EMISSION_IRIS_DEFAULT, XLIGHT_ILLUMINATION_IRIS_DEFAULT

from control import _def as _control_def

try:

XLIGHT_EMISSION_IRIS_DEFAULT = _control_def.XLIGHT_EMISSION_IRIS_DEFAULT

XLIGHT_ILLUMINATION_IRIS_DEFAULT = _control_def.XLIGHT_ILLUMINATION_IRIS_DEFAULT

except AttributeError:

# Fallback defaults if constants are missing from control._def.

# Values chosen to be neutral and safe; they can be overridden by adding

# the constants to control._def when available.

XLIGHT_EMISSION_IRIS_DEFAULT = 0.0

XLIGHT_ILLUMINATION_IRIS_DEFAULT = 0.0

[Claude Code] False positive - constants were added to _def.py in commit 6b41dcb (same PR, earlier commit).

Without init, the skip-if-unchanged check in set_illumination_iris / set_emission_iris would raise AttributeError on first call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Address Copilot review: ALL_IRIS_DEFAULTS dict maps each iris property to its own default from _def.py, avoiding semantic mismatch if illumination and emission defaults ever diverge. Moved import to top-level import block per PEP 8. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T22:00:03Z

        self.emission_iris = value
-        value = str(int(10 * value))
-        self.serial_connection.write_and_check("V" + value + "\r", "V" + value, read_delay=3)
+        self.serial_connection.write_and_read("V" + str(int(10 * value)) + "\r", read_delay=2)


Same issue as with set_illumination_iris: self.emission_iris = value is assigned before the serial command. If write_and_read fails, the cached value will be wrong and the skip-if-unchanged check on line 421 will prevent future retries. Move the assignment after the write_and_read call.

Copilot · 2026-03-06T22:00:04Z

        self.illumination_iris = value
-        value = str(int(10 * value))
-        self.serial_connection.write_and_check("J" + value + "\r", "J" + value, read_delay=3)
+        self.serial_connection.write_and_read("J" + str(int(10 * value)) + "\r", read_delay=2)


The cached value self.illumination_iris is updated before the serial command is sent. If write_and_read raises a SerialDeviceError (e.g., no response after max attempts), the cached value will reflect the intended value rather than the actual hardware state. Combined with the new skip-if-unchanged check on line 408, subsequent calls with the same value will be silently skipped, leaving the hardware in the wrong state with no way to retry.

Consider moving self.illumination_iris = value to after the write_and_read call succeeds, so that a failed command doesn't poison the cache.

Alpaca233 requested a review from Copilot March 6, 2026 21:16

Copilot started reviewing on behalf of Alpaca233 March 6, 2026 21:16 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

fix: iris slider click-to-position now applies to hardware and persists

4b1dbd9

sliderReleased doesn't fire on click-to-position, only on drag release. Use valueChanged with isSliderDown() guard to catch both cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Alpaca233 requested a review from Copilot March 6, 2026 21:34

Copilot started reviewing on behalf of Alpaca233 March 6, 2026 21:34 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Alpaca233 and others added 2 commits March 6, 2026 13:42

fix: initialize iris attributes in XLight.__init__

e26085c

Without init, the skip-if-unchanged check in set_illumination_iris / set_emission_iris would raise AttributeError on first call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Alpaca233 requested a review from Copilot March 6, 2026 21:55

Copilot started reviewing on behalf of Alpaca233 March 6, 2026 21:55 View session

Alpaca233 merged commit 35321c9 into Cephla-Lab:master Mar 6, 2026
4 checks passed

Copilot AI reviewed Mar 6, 2026

View reviewed changes

		if value == self.illumination_iris:
		return self.illumination_iris

-        if value == self.emission_iris:
-            return self.emission_iris
+        current_emission_iris = getattr(self, "emission_iris", None)
+        if value == current_emission_iris:
+            return current_emission_iris

		from control._def import XLIGHT_EMISSION_IRIS_DEFAULT, XLIGHT_ILLUMINATION_IRIS_DEFAULT

-from control._def import XLIGHT_EMISSION_IRIS_DEFAULT, XLIGHT_ILLUMINATION_IRIS_DEFAULT
+from control import _def as _control_def
+try:
+    XLIGHT_EMISSION_IRIS_DEFAULT = _control_def.XLIGHT_EMISSION_IRIS_DEFAULT
+    XLIGHT_ILLUMINATION_IRIS_DEFAULT = _control_def.XLIGHT_ILLUMINATION_IRIS_DEFAULT
+except AttributeError:
+    # Fallback defaults if constants are missing from control._def.
+    # Values chosen to be neutral and safe; they can be overridden by adding
+    # the constants to control._def when available.
+    XLIGHT_EMISSION_IRIS_DEFAULT = 0.0
+    XLIGHT_ILLUMINATION_IRIS_DEFAULT = 0.0

Conversation

Alpaca233 commented Mar 6, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Alpaca233 Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Alpaca233 Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Alpaca233 Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Alpaca233 Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Alpaca233 Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants