Skip to content

Commit 65b33a3

Browse files
committed
Improve GenTL trigger routing safety and tests
Make GenTLCameraBackend trigger configuration more robust and adjust tests. - Use the waits_for_hardware_trigger property when converting GenTL timeouts to user-facing errors instead of inferring from the role string. - Read and record the configured trigger role early in _configure_trigger_input. - Check results when setting TriggerSelector, TriggerSource and TriggerActivation (selector_ok, source_ok, activation_ok). If selector/source routing fails in non-strict mode, disable the trigger, reset internal trigger state and log a warning to avoid arming the camera on a previous/default input line. If activation fails, warn and continue using the camera default. - If enabling TriggerMode=On fails, disable the trigger and reset internal state instead of only warning. - Validate LineMode and LineSource when configuring master output and log if configuration is incomplete. Tests updated to reflect safety changes: - Expect waits_for_hardware_trigger to be set for external input configuration. - Rename and change a non-strict invalid-source test to assert the trigger is disabled, waits_for_hardware_trigger is False, and trigger_actual is persisted as off. - Add a new test ensuring an invalid selector in non-strict mode disables the trigger and persists the off state. These changes prevent the camera from being left armed on an unintended/default input if routing nodes could not be applied, improving safety and predictability.
1 parent e2c15b3 commit 65b33a3

2 files changed

Lines changed: 98 additions & 15 deletions

File tree

dlclivegui/cameras/backends/gentl_backend.py

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -498,8 +498,7 @@ def read(self) -> tuple[np.ndarray, float]:
498498
except ValueError:
499499
frame = array.copy()
500500
except HarvesterTimeoutError as exc:
501-
role = str(self._trigger_attr(getattr(self, "_trigger", None), "role", "off") or "off").lower()
502-
if role in {"external", "follower"}:
501+
if self.waits_for_hardware_trigger:
503502
raise TimeoutError(str(exc) + " (GenTL timeout; waiting for hardware trigger?)") from exc
504503
raise TimeoutError(str(exc) + " (GenTL timeout)") from exc
505504

@@ -1123,24 +1122,59 @@ def _configure_trigger_off(self, node_map, *, strict: bool = False) -> None:
11231122
self._set_enum_node(node_map, "TriggerMode", "Off", strict=strict)
11241123

11251124
def _configure_trigger_input(self, node_map, cfg, *, strict: bool = False) -> None:
1125+
role = str(self._trigger_attr(cfg, "role", "external") or "external").strip().lower()
11261126
selector = str(self._trigger_attr(cfg, "selector", "FrameStart") or "FrameStart")
11271127
source = str(self._trigger_attr(cfg, "source", "Line0") or "Line0")
11281128
activation = str(self._trigger_attr(cfg, "activation", "RisingEdge") or "RisingEdge")
11291129

11301130
# Disable trigger while changing trigger-related nodes.
11311131
self._set_enum_node(node_map, "TriggerMode", "Off", strict=False)
11321132

1133-
self._set_enum_node(node_map, "TriggerSelector", selector, strict=strict)
1134-
self._set_enum_node(node_map, "TriggerSource", source, strict=strict)
1135-
self._set_enum_node(node_map, "TriggerActivation", activation, strict=strict)
1133+
selector_ok = self._set_enum_node(node_map, "TriggerSelector", selector, strict=strict)
1134+
source_ok = self._set_enum_node(node_map, "TriggerSource", source, strict=strict)
1135+
activation_ok = self._set_enum_node(node_map, "TriggerActivation", activation, strict=strict)
1136+
1137+
# TriggerSelector and TriggerSource are required routing nodes.
1138+
# If either failed in non-strict mode, do not arm TriggerMode=On.
1139+
# Otherwise the camera may wait on a previous/default input line.
1140+
if not (selector_ok and source_ok):
1141+
LOG.warning(
1142+
"Could not apply GenTL trigger input routing "
1143+
"(selector_ok=%s, source_ok=%s); disabling trigger. "
1144+
"requested role=%s selector=%s source=%s activation=%s",
1145+
selector_ok,
1146+
source_ok,
1147+
role,
1148+
selector,
1149+
source,
1150+
activation,
1151+
)
1152+
self._configure_trigger_off(node_map, strict=False)
1153+
self._trigger = CameraTriggerSettings()
1154+
return
1155+
1156+
if not activation_ok:
1157+
LOG.warning(
1158+
"Could not apply GenTL TriggerActivation=%s; using camera default/current activation.",
1159+
activation,
1160+
)
11361161

11371162
self._set_enum_node(node_map, "AcquisitionMode", "Continuous", strict=False)
11381163

11391164
if not self._set_enum_node(node_map, "TriggerMode", "On", strict=strict):
1140-
if strict:
1141-
raise RuntimeError("Could not enable GenTL TriggerMode=On")
1142-
else:
1143-
LOG.warning("Could not enable GenTL TriggerMode=On; trigger mode may not be correctly configured.")
1165+
LOG.warning("Could not enable GenTL TriggerMode=On; disabling trigger.")
1166+
self._configure_trigger_off(node_map, strict=False)
1167+
self._trigger = CameraTriggerSettings()
1168+
return
1169+
1170+
LOG.info(
1171+
"GenTL trigger input configured: role=%s selector=%s source=%s activation=%s activation_ok=%s",
1172+
role,
1173+
selector,
1174+
source,
1175+
activation,
1176+
activation_ok,
1177+
)
11441178

11451179
LOG.info(
11461180
"GenTL trigger input configured: role=%s selector=%s source=%s activation=%s",
@@ -1174,8 +1208,16 @@ def _configure_trigger_master(self, node_map, cfg, *, strict: bool = False) -> N
11741208
)
11751209
return
11761210

1177-
self._set_enum_node(node_map, "LineMode", "Output", strict=strict)
1178-
self._set_enum_node(node_map, "LineSource", output_source, strict=strict)
1211+
mode_ok = self._set_enum_node(node_map, "LineMode", "Output", strict=strict)
1212+
source_ok = self._set_enum_node(node_map, "LineSource", output_source, strict=strict)
1213+
1214+
if not (mode_ok and source_ok):
1215+
LOG.warning(
1216+
"GenTL trigger master output configuration incomplete (LineMode ok=%s, LineSource ok=%s).",
1217+
mode_ok,
1218+
source_ok,
1219+
)
1220+
return
11791221

11801222
LOG.info(
11811223
"GenTL trigger master configured: output_line=%s output_source=%s",

tests/cameras/backends/test_gentl_trigger.py

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ def test_trigger_external_configures_input_line_and_timeout(patch_gentl_sdk, gen
7575
assert nm.TriggerSource.value == "Line0"
7676
assert nm.TriggerActivation.value == "RisingEdge"
7777
assert nm.TriggerMode.value == "On"
78+
assert be.waits_for_hardware_trigger is True
7879
assert be._timeout == pytest.approx(10.0)
7980

8081
ns = settings.properties["gentl"]
@@ -140,7 +141,7 @@ def test_trigger_master_configures_output_line_and_keeps_trigger_off(patch_gentl
140141
be.close()
141142

142143

143-
def test_trigger_invalid_source_non_strict_does_not_crash(patch_gentl_sdk, gentl_settings_factory):
144+
def test_trigger_invalid_source_non_strict_disables_trigger(patch_gentl_sdk, gentl_settings_factory):
144145
gb = patch_gentl_sdk
145146

146147
settings = _gentl_trigger_settings(
@@ -158,9 +159,17 @@ def test_trigger_invalid_source_non_strict_does_not_crash(patch_gentl_sdk, gentl
158159

159160
# Source was unsupported, so the fake node should retain its default.
160161
assert nm.TriggerSource.value == "Line0"
161-
# Non-strict mode should still allow opening; TriggerMode may be enabled
162-
# because TriggerSource failure is best-effort in this mode.
163-
assert be._acquirer is not None
162+
163+
# Safety behavior: do not arm TriggerMode on the previous/default source.
164+
assert nm.TriggerMode.value == "Off"
165+
166+
# Controller should not treat timeouts as expected trigger waits.
167+
assert be.waits_for_hardware_trigger is False
168+
169+
# trigger_actual is persisted after _configure_trigger(); since we reset
170+
# self._trigger to off, the effective trigger state is off.
171+
actual = settings.properties["gentl"]["trigger_actual"]
172+
assert actual["role"] == "off"
164173

165174
be.close()
166175

@@ -331,3 +340,35 @@ def test_trigger_actual_is_persisted_for_debugging(patch_gentl_sdk, gentl_settin
331340
assert actual["timeout"] == pytest.approx(9.0)
332341

333342
be.close()
343+
344+
345+
def test_trigger_invalid_selector_non_strict_disables_trigger(patch_gentl_sdk, gentl_settings_factory):
346+
gb = patch_gentl_sdk
347+
348+
settings = _gentl_trigger_settings(
349+
gentl_settings_factory,
350+
{
351+
"role": "external",
352+
"selector": "NotARealSelector",
353+
"source": "Line1",
354+
"strict": False,
355+
},
356+
)
357+
be = gb.GenTLCameraBackend(settings)
358+
359+
be.open()
360+
nm = be._acquirer.remote_device.node_map
361+
362+
# Selector was unsupported, so the fake node should retain its default.
363+
assert nm.TriggerSelector.value == "FrameStart"
364+
365+
# Source may have been applied, but trigger must not be armed because
366+
# the required selector routing failed.
367+
assert nm.TriggerSource.value == "Line1"
368+
assert nm.TriggerMode.value == "Off"
369+
assert be.waits_for_hardware_trigger is False
370+
371+
actual = settings.properties["gentl"]["trigger_actual"]
372+
assert actual["role"] == "off"
373+
374+
be.close()

0 commit comments

Comments
 (0)