feat: Add hardware trigger mode setting to Settings > Advanced by hongquanli · Pull Request #503 · Cephla-Lab/Squid

hongquanli · 2026-03-01T02:35:13Z

Summary

Adds Edge/Level hardware trigger mode dropdown in Settings > Advanced > Hardware Configuration
Only visible when camera type is Toupcam or Tucsen (cameras that support level trigger)
Takes effect on next acquisition/live mode without requiring app restart — achieved by reading HARDWARE_TRIGGER_MODE via control._def module reference instead of stale local bindings from wildcard import

Test plan

All 124 widget tests pass
App launches in simulation mode without errors
Open Settings > Advanced > Hardware Configuration, verify dropdown shows Edge/Level
Change to Level, save — verify no restart prompt
Start acquisition/live mode — verify trigger mode is applied
Restart app, reopen Settings — verify Level is persisted
Check INI file contains hardware_trigger_mode = 1
With non-Toupcam config, verify dropdown is hidden

🤖 Generated with Claude Code

Add Edge/Level trigger mode dropdown in Hardware Configuration section. The setting takes effect on next acquisition without restart by reading HARDWARE_TRIGGER_MODE via control._def module reference instead of stale local bindings from wildcard import. Only visible when camera type is Toupcam or Tucsen (cameras that support level trigger). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Adds a user-facing setting for selecting hardware trigger mode (Edge vs Level) under Settings > Advanced > Hardware Configuration, and ensures runtime trigger mode changes are picked up without restart by reading HARDWARE_TRIGGER_MODE from the control._def module.

Changes:

Add “Hardware Trigger Mode” dropdown (Edge/Level) in Advanced settings and persist it to the INI config.
Update runtime control._def.HARDWARE_TRIGGER_MODE when applying settings so the next acquisition/live session uses the new mode.
Replace stale wildcard-import bindings by referencing control._def.HARDWARE_TRIGGER_MODE in key trigger setup paths.

Reviewed changes

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

File	Description
software/control/widgets.py	Adds the trigger mode UI, persists config, and updates runtime `control._def.HARDWARE_TRIGGER_MODE`; includes change-summary reporting.
software/control/gui_hcs.py	Uses `control._def.HARDWARE_TRIGGER_MODE` when configuring microcontroller trigger mode for hardware trigger.
software/control/core/live_controller.py	Uses `control._def.HARDWARE_TRIGGER_MODE` when switching to hardware trigger mode during live control.
software/control/camera_toupcam.py	Uses `control._def.HARDWARE_TRIGGER_MODE` to select Toupcam hardware trigger configuration behavior.

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

Copilot · 2026-03-01T02:39:57Z

software/control/widgets.py

+        self.hw_trigger_mode_combo.setToolTip(
+            "Edge: Fixed pulse width (TRIGGER_PULSE_LENGTH_us)\n" "Level: Variable pulse width (illumination_on_time)"
+        )
+        hw_trigger_value = self._get_config_int("GENERAL", "hardware_trigger_mode", 0)


hw_trigger_value is read directly from the INI and passed to setCurrentIndex(). If the config contains an unexpected value (e.g., manual edit/corruption), the combobox can end up with an invalid selection and the app may later write back an out-of-range mode. Consider clamping/validating the value to the available indices (defaulting to Edge) before calling setCurrentIndex().

Suggested change

hw_trigger_value = self._get_config_int("GENERAL", "hardware_trigger_mode", 0)

hw_trigger_value = self._get_config_int("GENERAL", "hardware_trigger_mode", 0)

if hw_trigger_value < 0 or hw_trigger_value >= self.hw_trigger_mode_combo.count():

# Default to "Edge" (index 0) if configuration value is out of range

hw_trigger_value = 0

Copilot · 2026-03-01T02:39:58Z

software/control/widgets.py

+            mode_names = ["Edge", "Level"]
+            changes.append(("Hardware Trigger Mode", mode_names[old_val], mode_names[new_val], False))


This change log uses mode_names[old_val] / mode_names[new_val]. If either value is outside {0,1} (e.g., INI edited to 2 or negative), this will raise IndexError when opening/applying settings. Use a safer mapping (e.g., dict lookup with fallback like f"Unknown ({val})") and/or clamp values before indexing.

Suggested change

mode_names = ["Edge", "Level"]

changes.append(("Hardware Trigger Mode", mode_names[old_val], mode_names[new_val], False))

mode_names = {0: "Edge", 1: "Level"}

old_mode_name = mode_names.get(old_val, f"Unknown ({old_val})")

new_mode_name = mode_names.get(new_val, f"Unknown ({new_val})")

changes.append(("Hardware Trigger Mode", old_mode_name, new_mode_name, False))

- Clamp hardware_trigger_mode from INI to valid range [0,1] to prevent IndexError in change detection if INI is manually corrupted - Use HardwareTriggerMode.EDGE/LEVEL constants when setting runtime value instead of raw int, for type safety Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Change HardwareTriggerMode from plain class to IntEnum, enabling HardwareTriggerMode(value) for validation instead of manual clamping with magic numbers. Simplifies widgets.py code that reads/writes the setting. 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 4 comments.

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

Copilot · 2026-03-01T03:22:19Z

software/control/widgets.py

+        control._def.HARDWARE_TRIGGER_MODE = HardwareTriggerMode(self.hw_trigger_mode_combo.currentIndex())



control._def.HARDWARE_TRIGGER_MODE is updated before the config file write succeeds. If saving fails (caught later), the app's runtime trigger mode will still change even though the user was told the settings could not be saved. Consider moving this runtime update into _apply_live_settings() (which runs after a successful write) or otherwise only mutating _def after the file save completes.

Suggested change

control._def.HARDWARE_TRIGGER_MODE = HardwareTriggerMode(self.hw_trigger_mode_combo.currentIndex())

Copilot · 2026-03-01T03:22:20Z

software/control/widgets.py

+            hw_trigger_value = HardwareTriggerMode(self._get_config_int("GENERAL", "hardware_trigger_mode", 0))
+        except ValueError:


This new Advanced > Hardware Trigger Mode control doesn’t appear to have widget-test coverage (visibility for Toupcam/Tucsen vs hidden otherwise, persistence to INI, and _def.HARDWARE_TRIGGER_MODE update). There is already a comprehensive PreferencesDialog test suite (e.g. software/tests/control/test_preferences_dialog.py)—adding a small set of tests for this new setting would help prevent regressions.

Suggested change

hw_trigger_value = HardwareTriggerMode(self._get_config_int("GENERAL", "hardware_trigger_mode", 0))

except ValueError:

hw_trigger_value = HardwareTriggerMode(

self._get_config_int("GENERAL", "hardware_trigger_mode", 0)

)

except (ValueError, TypeError):

logger.warning(

"Invalid 'hardware_trigger_mode' value in config; defaulting to EDGE.",

exc_info=True,

)

Copilot · 2026-03-01T03:22:20Z

software/control/core/live_controller.py

 from squid.abc import CameraAcquisitionMode, AbstractCamera

+import control._def
 from control._def import *


This file now relies on control._def.HARDWARE_TRIGGER_MODE for runtime updates, but still does from control._def import *, which also imports a stale HARDWARE_TRIGGER_MODE name into the local namespace. To reduce the chance of future regressions (someone accidentally using the local binding again), consider replacing the wildcard import with explicit imports (or relying solely on the module import) in this module.

Suggested change

from control._def import *

from control._def import TriggerMode

Copilot · 2026-03-01T03:22:20Z

software/control/camera_toupcam.py

 from squid.config import CameraConfig, ToupcamCameraModel
+import control._def
 from control._def import *



Same concern as in live_controller.py: this file imports control._def for runtime access but also keeps from control._def import *, which pulls a stale HARDWARE_TRIGGER_MODE into the local namespace. Consider switching away from the wildcard import (or otherwise preventing accidental use of the local binding) to avoid reintroducing the original bug later.

Suggested change

# Avoid using a potentially stale HARDWARE_TRIGGER_MODE imported via wildcard.

# The authoritative, runtime-updated value should be accessed as control._def.HARDWARE_TRIGGER_MODE.

if "HARDWARE_TRIGGER_MODE" in globals():

del HARDWARE_TRIGGER_MODE

- Log warning when hardware_trigger_mode INI value is invalid instead of silently falling back to EDGE - Build combo box items from HardwareTriggerMode enum members instead of hardcoding ["Edge", "Level"], eliminating duplication Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

hongquanli requested review from Alpaca233 and Copilot March 1, 2026 02:35

Copilot started reviewing on behalf of hongquanli March 1, 2026 02:36 View session

Copilot AI reviewed Mar 1, 2026

View reviewed changes

hongquanli and others added 2 commits February 28, 2026 18:48

hongquanli requested a review from Copilot March 1, 2026 03:16

Copilot started reviewing on behalf of hongquanli March 1, 2026 03:16 View session

Copilot AI reviewed Mar 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add hardware trigger mode setting to Settings > Advanced#503

feat: Add hardware trigger mode setting to Settings > Advanced#503
hongquanli wants to merge 4 commits intomasterfrom
feat/hw-trigger-mode-setting

hongquanli commented Mar 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        hw_trigger_value = self._get_config_int("GENERAL", "hardware_trigger_mode", 0)
+        hw_trigger_value = self._get_config_int("GENERAL", "hardware_trigger_mode", 0)
+        if hw_trigger_value < 0 or hw_trigger_value >= self.hw_trigger_mode_combo.count():
+            # Default to "Edge" (index 0) if configuration value is out of range
+            hw_trigger_value = 0

		mode_names = ["Edge", "Level"]
		changes.append(("Hardware Trigger Mode", mode_names[old_val], mode_names[new_val], False))

-            mode_names = ["Edge", "Level"]
-            changes.append(("Hardware Trigger Mode", mode_names[old_val], mode_names[new_val], False))
+            mode_names = {0: "Edge", 1: "Level"}
+            old_mode_name = mode_names.get(old_val, f"Unknown ({old_val})")
+            new_mode_name = mode_names.get(new_val, f"Unknown ({new_val})")
+            changes.append(("Hardware Trigger Mode", old_mode_name, new_mode_name, False))

		control._def.HARDWARE_TRIGGER_MODE = HardwareTriggerMode(self.hw_trigger_mode_combo.currentIndex())

		hw_trigger_value = HardwareTriggerMode(self._get_config_int("GENERAL", "hardware_trigger_mode", 0))
		except ValueError:

	from control._def import *
	from control._def import TriggerMode

+# Avoid using a potentially stale HARDWARE_TRIGGER_MODE imported via wildcard.
+# The authoritative, runtime-updated value should be accessed as control._def.HARDWARE_TRIGGER_MODE.
+if "HARDWARE_TRIGGER_MODE" in globals():
+    del HARDWARE_TRIGGER_MODE

Conversation

hongquanli commented Mar 1, 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 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 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 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants