enable sending camera triggers from board#127
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds camera trigger functionality to enable the board to send camera trigger signals for software-triggered image acquisition during stage scanning, and extends stage scanning to support Z-axis positioning.
Changes:
- New
CameraTriggerclass that handles incoming camera trigger signals from ESP32 firmware ({"cam":1}) - Extended
start_stage_scanningmethod with Z-axis parameters (zstart, zstep, nz) - Updated documentation to reflect Z-axis support in coordinate-based scanning
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| uc2rest/camera_trigger.py | New module implementing camera trigger callback handling with support for registering multiple callbacks and tracking trigger statistics |
| uc2rest/motor.py | Added Z-axis parameters to stage scanning methods and updated example documentation to include Z coordinates |
| uc2rest/UC2Client.py | Integrated CameraTrigger module into UC2Client initialization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Register callback for camera trigger on serial loop | ||
| if hasattr(self._parent, "serial"): | ||
| self._parent.serial.register_callback(self._callback_camera_trigger, pattern="cam") |
There was a problem hiding this comment.
The CameraTrigger is initialized unconditionally, but the serial callback registration (line 57) only works when a serial connection is used. When using WiFi mode (host is not None), the parent will not have a 'serial' attribute, so the callback won't be registered. However, camera triggers from the board may not work in WiFi mode anyway. Consider documenting this limitation in the class docstring or adding a log message when the callback cannot be registered.
| self._parent.logger.debug(f"Camera trigger received: {trigger_info}") | ||
| else: | ||
| # Handle extended trigger data | ||
| trigger_info = { | ||
| "trigger": cam_data.get("trigger", 1), | ||
| "frame_id": cam_data.get("frame_id", self._trigger_count), | ||
| "illumination": cam_data.get("illumination", -1), | ||
| "timestamp": self._last_trigger_time | ||
| } | ||
| self._parent.logger.debug(f"Camera trigger with data received: {trigger_info}") |
There was a problem hiding this comment.
The code accesses self._parent.logger without checking if parent is None or if it has a logger attribute. While this is likely safe given the UC2Client initialization, consider adding a defensive check or using hasattr similar to line 56 for consistency. Alternatively, you could check for a DEBUG flag before logging, as is done in motor.py (line 512).
| def start_stage_scanning(self, xstart=0, xstep=1000, nx=20, ystart=0, ystep=1000, ny=10, tsettle=5, tExposure=50, illumination=(0,0,0,0), led=0, speed=20000, acceleration=None): | ||
| # {"task": "/motor_act", "stagescan": {"xStart": 0, "yStart": 0, "xStep": 500, "yStep": 500, "nX": 10, "nY": 10, "tPre": 50, "tPost": 50, "illumination": [0, 1, 0, 0], "led": 255}} | ||
| def start_stage_scanning(self, xstart=0, xstep=1000, nx=20, ystart=0, ystep=1000, ny=10, zstart=0, zstep=1000, nz=10, tsettle=5, tExposure=50, illumination=(0,0,0,0), led=0, speed=20000, acceleration=None): | ||
| # {"task": "/motor_act", "stagescan": {"xStart": 0, "yStart": 0, "zStart": 0, "xStep": 500, "yStep": 500, "zStep": 500, "nX": 10, "nY": 10, "nZ": 10, "tPre": 50, "tPost": 50, "illumination": [0, 1, 0, 0], "led": 255}} |
There was a problem hiding this comment.
The indentation uses tabs instead of spaces, which is inconsistent with Python's PEP 8 style guide and likely the rest of the codebase. This could cause issues with mixed indentation. Please replace the tabs with spaces (typically 4 or 8 spaces depending on the project's convention).
| # {"task": "/motor_act", "stagescan": {"xStart": 0, "yStart": 0, "zStart": 0, "xStep": 500, "yStep": 500, "zStep": 500, "nX": 10, "nY": 10, "nZ": 10, "tPre": 50, "tPost": 50, "illumination": [0, 1, 0, 0], "led": 255}} | |
| # {"task": "/motor_act", "stagescan": {"xStart": 0, "yStart": 0, "zStart": 0, "xStep": 500, "yStep": 500, "zStep": 500, "nX": 10, "nY": 10, "nZ": 10, "tPre": 50, "tPost": 50, "illumination": [0, 1, 0, 0], "led": 255}} |
| import numpy as np | ||
| import time | ||
| import json |
There was a problem hiding this comment.
The imports 'numpy' and 'json' are not used anywhere in this file. Consider removing these unused imports to keep the code clean and reduce unnecessary dependencies.
| import numpy as np | |
| import time | |
| import json | |
| import time |
1 participant
No description provided.