Add UI to configure Trigger mode#79
Conversation
There was a problem hiding this comment.
Pull request overview
Adds camera-level hardware trigger configuration to the camera configuration dialog, focused on GenTL-backed cameras and preview restart behavior when trigger settings change.
Changes:
- Adds a new
TriggerConfigDialogfor editing trigger role, input/output lines, activation, timeout, and strict mode. - Adds a “Trigger Settings…” UI button and trigger role indicators in active camera labels.
- Initializes default GenTL trigger settings and restarts previews when trigger configuration changes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
dlclivegui/gui/camera_config/ui_blocks.py |
Adds the trigger settings button to the camera settings form. |
dlclivegui/gui/camera_config/trigger_config_dialog.py |
Implements the trigger configuration dialog and persistence into camera backend properties. |
dlclivegui/gui/camera_config/camera_config_dialog.py |
Wires trigger settings into the dialog, labels, defaults, and preview restart logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
279a952 to
1780065
Compare
ae2664b to
662313a
Compare
Update display tests to assert that tiling and tile computations preserve frame insertion/display order (no longer sorting by camera ID) and add coverage for tile offsets, scaling, and tiled frame content. Add a suite of unit tests for MultiCameraController utilities and behavior: get_camera_id, trigger role aliasing, camera start priority, preserving user display order on start, frame_ready emission order, clearing display order on stop, hardware trigger timeouts (non-fatal), and non-trigger timeouts (fatal). Also import newly-tested helper functions from multi_camera_controller.
Introduce a TriggerConfigDialog to edit per-camera hardware trigger settings (CameraTriggerSettings). The dialog provides fields for role, selector, source, activation, output line/source, timeout and strict mode, and persists settings into camera.properties[<backend>]['trigger']. Also add a "Trigger Settings…" button to the camera settings UI (disabled by default) with tooltip and icon; wiring to open the dialog can be connected elsewhere.
Introduce support for per-camera hardware trigger configuration in the camera config dialog. Imports TriggerConfigDialog and CameraTriggerSettings, wires the trigger settings button to open a modal, and adds _open_trigger_settings_dialog to commit edits, show the dialog, apply updates, and restart the preview if needed. Adds helpers: _ensure_default_trigger_config to initialize gentl.trigger defaults, _trigger_role_for_label to show trigger role in camera list labels, and _trigger_dict_for_cam to compare trigger settings when deciding to restart previews. Also integrates the hardware trigger field into the settings summary and ensures new/loaded cameras get a default trigger config.
Replace direct checks of self._preview.state == PreviewState.ACTIVE with self._is_preview_live() in camera_config_dialog to centralize preview-active logic and ensure consistent behavior when restarting the preview after trigger or camera setting changes. In trigger_config_dialog, only set payload["timeout"] for external/follower roles when timeout > 0, and explicitly set payload["timeout"] = None when role == "off" to clear any stale timeout when disabling the trigger.
Allow pending preview restarts to proceed when the preview is ACTIVE (previously only IDLE) and return early to avoid double UI sync in camera_config_dialog.py. Also wrap CameraTriggerSettings loading in a try/except and fall back to a default instance when parsing fails, making trigger_config_dialog.py tolerant of malformed or missing trigger data.
Make trigger dialog friendlier and more robust: recommend using 'auto' for trigger source and switch default/fallback source from "Line0" to "auto", update source placeholder and info/timeout tooltips, and import QMessageBox. Wrap CameraTriggerSettings.from_any in a try/except to show a critical error dialog on failure and abort apply. Store trigger settings via trigger.to_properties() instead of model_dump(exclude_none=True). These changes provide clearer defaults and better error feedback when applying trigger settings.
Introduce WorkerTimingStats (utils/stats.py) to collect simple timing counters (per-named section totals, frame/timeouts/errors) and periodically emit debug logs. Integrate it into SingleCameraWorker: create a timing instance (configurable via new SINGLE_CAMERA_WORKER_DO_LOG_TIMING in config.py), wrap backend.read and frame emit calls with timed sections, and call note_frame/note_timeout/note_error + maybe_log to accumulate and flush stats. Also update imports accordingly and use a 1s default log interval.
Introduce MULTI_CAMERA_WORKER_DO_LOG_TIMING and disable single-worker timing by default (SINGLE_CAMERA_WORKER_DO_LOG_TIMING=false). Add per-camera WorkerTimingStats storage and a _timing_for_camera factory that respects the new config. Wrap _on_frame_captured processing in timed sections (total, apply_transforms, update_latest), call note_frame/maybe_log per camera, and emit frames as before. Also adjust SingleCameraWorker timing labels from GenTL.* to Single.* for clearer logs.
Introduce a human-readable representation for CameraSettings by adding a pretty() method and overriding __str__ and __repr__. The pretty output formats key fields (name, index, backend, enabled, fps, size, exposure, gain, rotation) and displays a readable crop region (showing 'none' or coordinate range with 'edge' fallbacks). This aids debugging and logging without changing existing validation or behavior.
Refactor _on_frame_captured to minimize time spent under _frame_lock and improve timing granularity. Introduces a frame_data local, renames timing labels (e.g. Multi.apply_transforms, Multi.store_latest, Multi.build_ordered, Multi.construct_frame_data), and moves frame_data construction inside measured blocks. The frame_ready emit is now performed outside the lock and guarded by a None check to avoid holding the lock during signal emission. Also small whitespace and cleanup changes.
Add debugging helpers for GenTL trigger and frame-rate nodes and log their values during camera configuration. Improve GenTL trigger input handling by treating TriggerSource as best-effort (supporting read-only/auto cases) and emitting clearer warnings. Implement a dedicated master/output path for TIS/DMK 37U cameras using Strobe* nodes (StrobeEnable/Polarity/Operation/Duration/Delay) with a fallback to generic Line* configuration; respect strict mode and surface informative errors. Extend CameraTriggerSettings with strobe fields (polarity, operation, duration, delay) and validation/coercion. Update the trigger configuration dialog to expose strobe controls, tooltips, UI sync logic, and include strobe values in the saved payload.
Introduce a static _node_value(node_map, name, default) helper that performs a best-effort read of GenICam node values. It looks up the node, returns default if missing, then tries node.value and falls back to node.GetValue() while swallowing exceptions. This prevents debug helpers and open() from failing when test/SDK nodes expose different accessors or raise errors.
Replace manual TriggerSource symbol checks with _resolve_trigger_source and only set TriggerSource when supported. Delay setting TriggerActivation until after source resolution and use non-strict writes for activation. Add safety check: do not arm TriggerMode=On unless both TriggerSelector and TriggerSource succeeded (avoids waiting on a previous/default input). Improve log messages to include selector_ok, source_ok, requested/ resolved source and activation for clearer diagnostics.
b1337ba to
732f5fd
Compare
deruyter92
left a comment
There was a problem hiding this comment.
Good PR! Left some comments. Let me know if you want to discuss together
| def _restore_trigger_idle(self, node_map) -> None: | ||
| """Best-effort restore to a safe non-triggering state after acquisition stops. | ||
|
|
There was a problem hiding this comment.
Should this also restore StrobeEnable to off? or only LineSource / LineMode?
| resolved_source, source_supported = self._resolve_trigger_source( | ||
| node_map, | ||
| source, | ||
| strict=strict, | ||
| ) | ||
|
|
||
| source_ok = False | ||
| if source_supported: | ||
| source_ok = self._set_enum_node( | ||
| node_map, | ||
| "TriggerSource", | ||
| resolved_source, | ||
| strict=strict, | ||
| ) | ||
|
|
There was a problem hiding this comment.
Hypothetical case: source_supported is not True because TriggerSource is read-only on some camera. This is currently treated as couldn't write TriggerSource -> abort trigger setup.
Would it be better to accept the camera default as fallback (best-effort) and just emit a warning for that, instead of aborting the setup. If you are also unsure, maybe good to think about it together.
This pull request adds comprehensive support for configuring hardware trigger and synchronization settings for individual cameras in the camera configuration dialog. It introduces a new
TriggerConfigDialogfor editing per-camera trigger settings, updates the UI to display trigger roles, and ensures that trigger changes are properly handled and reflected in the camera preview and configuration.Key changes include:
Hardware Trigger Configuration Support:
TriggerConfigDialog(trigger_config_dialog.py) that allows users to configure per-camera hardware trigger and synchronization settings, such as role, selector, source, activation, output line, output source, timeout, and strict mode. The dialog updates the camera's trigger configuration in a structured and user-friendly way.trigger_settings_btn) and connecting it to open the dialog for the selected camera. [1] [2] [3]UI and Usability Improvements:
Configuration and Preview Handling:
applyutility to support the new trigger settings button, maintaining consistent UI state management.These changes collectively provide robust and user-friendly management of hardware trigger and synchronization settings for cameras in the application.