feat: Add second filter wheel (W2) support by hongquanli · Pull Request #478 · Cephla-Lab/Squid

hongquanli · 2026-01-26T12:13:38Z

Summary

Add firmware and software support for a second filter wheel (W2) on motor slot 4
W2 uses CS pin 16 and clock pin 28 (separate 16MHz clock output for TMC4361A)
Add tabbed UI in filter wheel widget to control both wheels independently
Camera triggers reduced from 6 to 4 (pins 29-32) to free up pins 16 and 28

Firmware Changes

Add W2 motor slot (index 4) with CS pin 16, clock pin 28
Add INITFILTERWHEEL_W2 command (252) and MOVE_W2 command (19)
Add AXIS_W2 (6) for homing and configuration commands
Add W2 homing state machine (prepare, check, finalize)
Configure pin 28 as 16MHz clock output for W2's TMC4361A chip
Extract init_filterwheel_axis() helper to reduce code duplication

Critical Bug Fixes (from code review)

Add protocol_axis_to_internal() mapping function to safely convert protocol axis constants (AXIS_W=5, AXIS_W2=6) to internal array indices (w=3, w2=4)
Fix out-of-bounds array access in callback_configure_stage_pid(), callback_enable_stage_pid(), callback_disable_stage_pid(), callback_set_pid_arguments(), callback_set_axis_disable_enable()
Add AXIS_W2 case to callback_set_home_safety_margin()
Add W2_commanded_movement_in_progress to mcu_cmd_execution_in_progress flag updates in check_position() and check_limits()
Add TOTAL_AXES constant (5) for proper array sizing

Software Changes

Add MOVE_W2, INITFILTERWHEEL_W2, AXIS.W2 constants in _def.py
Unify init_filter_wheel(axis) and configure_squidfilter(axis) methods with axis parameter
Make W/W2 initialization symmetric: init called per-wheel in _configure_wheel()
Add move_w2_usteps(), home_w2() methods in microcontroller.py
Update SquidFilterWheel to support multi-wheel configuration via dict
Add tabbed UI for multiple filter wheels in widgets.py
Add _move_to_position() with automatic re-home on movement failure
Fix PID setup to use protocol axis (AXIS.W/W2) instead of motor_slot (3/4)
Add INITFILTERWHEEL_W2 to _CMD_NAMES for proper logging

Configuration

Enable second filter wheel in .ini config:

emission_filter_wheel_indices = [1, 2]

Per-wheel configuration in _def.py:

SQUID_FILTERWHEEL_CONFIGS = {
    1: {"motor_slot_index": 3, "max_index": 8, "min_index": 1, ...},  # W axis
    2: {"motor_slot_index": 4, "max_index": 8, "min_index": 1, ...},  # W2 axis
}

Test plan

Verify W2 motor initializes and holds position
Verify W2 homes correctly
Verify W2 moves to positions 1-8
Verify both W and W2 work independently via tabbed UI
Run test script: cd software && python tools/test_filter_wheel_w2.py
Compile firmware and launch software - both wheels init successfully

🤖 Generated with Claude Code

Add firmware and software support for a second filter wheel on motor slot 4: Firmware: - Add W2 motor slot (index 4) with CS pin 16 and clock pin 28 - Add INITFILTERWHEEL_W2 command (252) and MOVE_W2 command (19) - Add AXIS_W2 (6) for homing and configuration commands - Add W2 homing state machine (prepare, check, finalize) - Add W2 to callback_reset() and check_position() - Configure pin 28 as 16MHz clock output for W2's TMC4361A - Reduce camera triggers from 6 to 4 (pins 29-32) Software: - Add MOVE_W2, INITFILTERWHEEL_W2, AXIS.W2 constants - Add init_filter_wheel_w2(), move_w2_usteps(), home_w2() methods - Update SquidFilterWheel to initialize W2 on motor_slot 4 - Add tabbed UI for multiple filter wheels in widgets.py Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Add skip_init parameter back to SquidFilterWheel constructor - Pass skip_init through get_filter_wheel_controller factory - Document that W and W2 share the same motor settings (identical hardware) - Remove unused R_sense_w2 constant (both wheels use R_sense_w) - Update comments in firmware to clarify shared settings Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Extract common PID configuration in _configure_wheel (both wheels use identical settings) - Consolidate next_position/previous_position into _step_position helper - Replace print() with proper logging in FilterControllerWidget Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

Adds end-to-end support for a second SQUID emission filter wheel (W2) by extending firmware motor control/protocol, updating software controller/configuration, and introducing a tabbed UI to control multiple wheels.

Changes:

Firmware: add W2 axis support (CS=16, CLK=28), new commands/axis IDs, W2 homing, and reduce camera trigger outputs from 6 to 4.
Software: add W2 commands/constants and microcontroller methods; extend SQUID filter wheel controller to support multiple wheels.
UI/config: allow per-wheel SQUID configs and provide a tabbed filter-wheel widget.

Reviewed changes

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

Show a summary per file

File	Description
software/squid/filter_wheel_controller/utils.py	Passes multi-wheel configs to controller and sizes simulated controller to match configured wheels.
software/squid/filter_wheel_controller/cephla.py	Implements multi-wheel SQUID filter wheel control, including W2 routing.
software/squid/config.py	Adds per-wheel SQUID filter wheel configs in config model/loading.
software/control/widgets.py	Updates filter wheel widget to show tabs when multiple wheels are present.
software/control/microcontroller.py	Adds W2 init/move/home/zero/configure APIs and simulated position tracking.
software/control/_def.py	Adds protocol constants for MOVE_W2 / INITFILTERWHEEL_W2 / AXIS.W2 and default multi-wheel config.
firmware/controller/src/operations.h	Declares W2 homing state machine functions.
firmware/controller/src/operations.cpp	Implements W2 homing and reduces camera trigger loops to 4 channels.
firmware/controller/src/init.cpp	Adds W2 clock output on pin 28; adjusts initialization loops for new arrays.
firmware/controller/src/globals.h	Expands arrays/state to include W2 and adds enable flag.
firmware/controller/src/globals.cpp	Defines new W2 globals and expands arrays to size 5.
firmware/controller/src/functions.cpp	Reduces strobe ISR camera-channel loop to 4.
firmware/controller/src/def/def_v1.h	Adds W2 motor index and documents shared W/W2 constants.
firmware/controller/src/constants_protocol.h	Adds MOVE_W2 / INITFILTERWHEEL_W2 / AXIS_W2 protocol constants.
firmware/controller/src/constants.h	Reassigns pins (camera triggers -> 4ch) and adds W2 CS/CLK pin definitions.
firmware/controller/src/commands/stage_commands.h	Declares callback_move_w2.
firmware/controller/src/commands/stage_commands.cpp	Implements MOVE_W2 and adds W2 handling in several stage callbacks.
firmware/controller/src/commands/commands.h	Declares callback_initfilterwheel_w2.
firmware/controller/src/commands/commands.cpp	Registers MOVE_W2/INITFILTERWHEEL_W2 and implements initfilterwheel_w2; resets W2 state on reset.
firmware/controller/main_controller_teensy41.ino	Calls W2 homing functions in the main loop.

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

Copilot · 2026-01-27T01:58:45Z

        if not skip_init:
+            # Initialize filter wheel hardware
            self.microcontroller.init_filter_wheel()
            time.sleep(0.5)


init_filter_wheel() is called unconditionally during init, even if the configured wheels only include motor_slot_index=4 (W2). This forces W (slot 3) to be enabled/initialized regardless of configuration. Consider initializing only the wheels actually present in self._configs (e.g., call init_filter_wheel() only when configuring a wheel with motor_slot_index==3, and init_filter_wheel_w2() only when motor_slot_index==4).

[Claude Code] Fixed - init_filter_wheel(axis) is now called inside _configure_wheel() with the correct axis parameter. If only W2 (motor_slot_index=4) is configured, only W2 will be initialized. W is no longer unconditionally initialized.

Copilot · 2026-01-27T01:58:45Z

+        config = self._configs[wheel_id]
+        motor_slot = config.motor_slot_index
+
+        if motor_slot == 3:
+            self.microcontroller.home_w()
+        elif motor_slot == 4:
+            self.microcontroller.home_w2()
+        else:
+            raise ValueError(f"Unsupported motor_slot_index: {motor_slot}")


SquidFilterWheelConfig includes homing_enabled, but _home_wheel/home() always issues a home command regardless of that flag. This makes it impossible to disable homing per wheel via config. Please gate the homing sequence on config.homing_enabled (either no-op or raise a clear error when homing is requested but disabled).

[Claude Code] N/A - The homing_enabled field does not exist in SquidFilterWheelConfig. The config only has: max_index, min_index, offset, motor_slot_index, transitions_per_revolution. This may be a feature for a different controller type.

Copilot · 2026-01-27T01:58:45Z

+        # Convert single config to dict format for uniform handling
+        if isinstance(configs, SquidFilterWheelConfig):
+            self._configs: Dict[int, SquidFilterWheelConfig] = {1: configs}
+        else:
+            self._configs = configs
+
+        # Track per-wheel positions (wheel_id -> position index)
+        self._positions: Dict[int, int] = {}
+
        if not skip_init:
+            # Initialize filter wheel hardware
            self.microcontroller.init_filter_wheel()
            time.sleep(0.5)
+
+            # Configure each wheel
+            for wheel_id, config in self._configs.items():
+                self._configure_wheel(wheel_id, config)
+                # Initialize position tracking to min_index
+                self._positions[wheel_id] = config.min_index
+        else:
+            # Just initialize position tracking without hardware init
+            for wheel_id, config in self._configs.items():
+                self._positions[wheel_id] = config.min_index
+
+        self._available_filter_wheels: List[int] = []
+
+    def _configure_wheel(self, wheel_id: int, config: SquidFilterWheelConfig):
+        """Configure a single filter wheel motor."""
+        motor_slot = config.motor_slot_index
+
+        if motor_slot == 3:
+            # W axis (first filter wheel)
            self.microcontroller.configure_squidfilter()
            time.sleep(0.5)
+        elif motor_slot == 4:
+            # W2 axis (second filter wheel)
+            self.microcontroller.init_filter_wheel_w2()
+            time.sleep(0.5)
+            self.microcontroller.configure_squidfilter_w2()
+            time.sleep(0.5)
+        else:
+            raise ValueError(f"Unsupported motor_slot_index: {motor_slot}. Expected 3 (W) or 4 (W2).")

-            if HAS_ENCODER_W:
-                self.microcontroller.set_pid_arguments(self._config.motor_slot_index, PID_P_W, PID_I_W, PID_D_W)
-                self.microcontroller.configure_stage_pid(
-                    self._config.motor_slot_index, self._config.transitions_per_revolution, ENCODER_FLIP_DIR_W
-                )
-                self.microcontroller.turn_on_stage_pid(self._config.motor_slot_index, ENABLE_PID_W)
+        # Common PID setup for both wheels (they share identical encoder settings)
+        if HAS_ENCODER_W:
+            self.microcontroller.set_pid_arguments(motor_slot, PID_P_W, PID_I_W, PID_D_W)
+            self.microcontroller.configure_stage_pid(motor_slot, config.transitions_per_revolution, ENCODER_FLIP_DIR_W)
+            self.microcontroller.turn_on_stage_pid(motor_slot, ENABLE_PID_W)

-        # emission filter position
-        self.w_pos_index = self._config.min_index
-        self._available_filter_wheels = []
+    def _move_wheel(self, wheel_id: int, delta: float):
+        """Move a specific wheel by delta distance.

-    def move_w(self, delta):
-        self.microcontroller.move_w_usteps(
-            int(STAGE_MOVEMENT_SIGN_W * delta / (SCREW_PITCH_W_MM / (MICROSTEPPING_DEFAULT_W * FULLSTEPS_PER_REV_W)))
+        Args:
+            wheel_id: The ID of the wheel to move.
+            delta: The distance to move (in mm, typically fraction of screw pitch).
+        """
+        config = self._configs[wheel_id]
+        motor_slot = config.motor_slot_index
+        usteps = int(
+            STAGE_MOVEMENT_SIGN_W * delta / (SCREW_PITCH_W_MM / (MICROSTEPPING_DEFAULT_W * FULLSTEPS_PER_REV_W))
        )

+        if motor_slot == 3:
+            self.microcontroller.move_w_usteps(usteps)
+        elif motor_slot == 4:
+            self.microcontroller.move_w2_usteps(usteps)
+        else:


Multi-wheel behavior is newly introduced here (dict configs, per-wheel position tracking, routing to move_w2_usteps() / home_w2()) but the existing test suite only covers the single-wheel path. Please add unit tests that construct a two-wheel config and verify: (1) initialize([1,2]) works, (2) position updates are tracked per wheel, and (3) the correct microcontroller methods are called for W vs W2.

[Claude Code] Partial - Hardware test script tools/test_filter_wheel_w2.py exists for W2 testing. Unit tests in test_filter_wheel.py cover single-wheel scenarios with mocks. Adding comprehensive multi-wheel unit tests is a valid enhancement for follow-up.

Copilot · 2026-01-27T01:58:45Z

+static const int camera_trigger_pins[] = {29, 30, 31, 32}; // trigger 1-4 (pin 16 used for W2 CS, pin 28 used for W2 CLK)
+


camera_trigger_pins is now length 4, but firmware code (e.g., callback_send_hardware_trigger() in commands.cpp) indexes camera_trigger_pins[camera_channel] without validating camera_channel. Any host command that sends channel >= 4 will cause out-of-bounds access. Add bounds checks (and ideally return/flag an error) anywhere camera_channel is used to index trigger arrays.

Suggested change

static const int camera_trigger_pins[] = {29, 30, 31, 32}; // trigger 1-4 (pin 16 used for W2 CS, pin 28 used for W2 CLK)

static const int camera_trigger_pins[] = {29, 30, 31, 32}; // trigger 1-4 (pin 16 used for W2 CS, pin 28 used for W2 CLK)

static const uint8_t NUM_CAMERA_TRIGGER_PINS = sizeof(camera_trigger_pins) / sizeof(camera_trigger_pins[0]);

static inline bool is_valid_camera_channel(uint8_t camera_channel) {

return camera_channel < NUM_CAMERA_TRIGGER_PINS;

}

[Claude Code] Skipped - Pre-existing issue unrelated to W2 implementation. The camera_trigger_pins bounds check is a valid concern but out of scope for this PR. Could be addressed in a separate security-focused PR.

Copilot · 2026-01-27T01:58:46Z

+        # Use multi-wheel configs if available, otherwise fall back to single config
+        configs = config.squid_wheel_configs if config.squid_wheel_configs else config.controller_config


configs = config.squid_wheel_configs if config.squid_wheel_configs else ... treats an empty dict the same as “not provided” and silently falls back to the legacy single-wheel config. This can mask misconfiguration (e.g., SQUID_FILTERWHEEL_CONFIGS present but filtered out / missing requested indices). Prefer an explicit is not None check and consider normalizing an empty dict to None when loading config so misconfig is surfaced predictably.

Suggested change

# Use multi-wheel configs if available, otherwise fall back to single config

configs = config.squid_wheel_configs if config.squid_wheel_configs else config.controller_config

# Use multi-wheel configs if explicitly set, otherwise fall back to single config

configs = config.squid_wheel_configs if config.squid_wheel_configs is not None else config.controller_config

[Claude Code] Fixed - Line 164 in utils.py already uses is not None check: configs = config.squid_wheel_configs if config.squid_wheel_configs is not None else config.controller_config

Copilot · 2026-01-27T01:58:46Z

@@ -8,107 +8,269 @@


 class SquidFilterWheel(AbstractFilterWheelController):


This class does not call AbstractFilterWheelController.init during initialization. (SquidFilterWheel.init may be missing a call to a base class init)

[Claude Code] Skipped - The AbstractFilterWheelController.init is abstract and contains only pass. Calling super().init() would have no effect. Python doesn't enforce calling super().init() for ABCs with abstract init. Minor style preference.

Copilot · 2026-01-27T01:58:46Z

+            except Exception:
+                pass


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except Exception:

pass

except Exception as exc:

logger.warning(

"Failed to get filter wheel name from config for wheel_id=%s: %s",

wheel_id,

exc,

)

[Claude Code] Skipped - Line 4853 has except Exception: with fallback to num_positions = 8. The other exception handlers at lines 4809 and 4866 already have logging. This specific case silently falls back to a safe default, which is acceptable for a non-critical UI initialization path.

- Make W/W2 initialization symmetric: init_filter_wheel() now called per-wheel in _configure_wheel() instead of unconditionally in __init__ - Unify init_filter_wheel(axis) and configure_squidfilter(axis) methods with optional axis parameter (defaults to AXIS.W for backward compat) - Remove unused homing_enabled config field (was never implemented) - Fix empty dict vs None check in utils.py for squid_wheel_configs - Add logging for bare except in widgets.py _get_wheel_name() - Add test script for W2 filter wheel (tools/test_filter_wheel_w2.py) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Extract init_filterwheel_axis() helper to share initialization logic - Extract move_filterwheel() helper to share movement logic - Reduces code duplication and firmware size by 240 bytes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

PID configuration for W2 encoder was silently ignored because the callback_configure_stage_pid() function had no case for axis == w2. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Fix Pydantic ValidationError for missing 'type' field when creating FilterWheelDefinition instances - the field is required by the model - Auto-create missing wheel entries in filter_wheels.yaml when .ini specifies more wheels (e.g., EMISSION_FILTER_WHEEL_INDICES = [1, 2]) than are defined in the YAML file - Use .copy() for default_positions dict to avoid sharing between wheels This prevents errors when: 1. Opening filter wheel config dialog with no filter_wheels.yaml 2. User updates .ini to enable second wheel but forgets to update YAML Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- FilterControllerWidget now reads position names from filter wheel registry instead of showing generic "Position 1"-"Position 8" - _populate_filter_positions_for_combo now auto-selects the first wheel in multi-wheel systems when no explicit wheel is specified, rather than falling back to generic position names This ensures filter position names defined in Settings > Filter Wheel Configuration are actually used throughout the UI. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

In multi-wheel systems, FilterWheelDefinition validation requires both 'id' AND 'name' fields. Fixed two issues: 1. widgets.py _load_config(): When auto-creating wheels for multi-wheel systems, now includes name field (e.g., "Wheel 1", "Wheel 2") 2. filter_wheels.yaml: Added missing 'name' field to second wheel Without these fixes, opening the filter wheel config dialog would fail with Pydantic validation error. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Two fixes for filter position names not showing in dropdowns: 1. gui_hcs.py: Pass config_repo to FilterControllerWidget so it can read position names from filter_wheels.yaml registry 2. widgets.py: Handle both int and string keys when looking up position names (YAML may load keys as strings even though model expects int) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Firmware fixes: - Add protocol_axis_to_internal() mapping to convert protocol axis constants (AXIS_W=5, AXIS_W2=6) to internal array indices (w=3, w2=4) - Fix out-of-bounds array access in PID functions and callback_set_axis_disable_enable - Add AXIS_W2 case to callback_set_home_safety_margin - Add W2_commanded_movement_in_progress to mcu_cmd_execution_in_progress checks in check_position() and check_limits() - Add TOTAL_AXES constant for proper array sizing - Add clarifying comments for axis constant naming conventions Software fixes: - Add _move_to_position() with automatic re-home on movement failure - Add INITFILTERWHEEL_W2 to _CMD_NAMES for proper logging - Add clarifying comments for _MOTOR_SLOT_TO_AXIS mapping Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 24 out of 24 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-01-27T05:15:37Z

+            self.microcontroller.set_pid_arguments(motor_slot, PID_P_W, PID_I_W, PID_D_W)
+            self.microcontroller.configure_stage_pid(motor_slot, config.transitions_per_revolution, ENCODER_FLIP_DIR_W)
+            self.microcontroller.turn_on_stage_pid(motor_slot, ENABLE_PID_W)


In the PID setup for filter wheels, set_pid_arguments(), configure_stage_pid(), and turn_on_stage_pid() are being called with motor_slot (3/4). Those MCU commands expect protocol axis constants (AXIS.W=5, AXIS.W2=6). With the firmware’s new protocol_axis_to_internal() mapping, sending 3/4 will be treated as an invalid axis (or the wrong axis), so encoder PID for W/W2 won’t be configured/enabled.

Use the already-computed axis (from _MOTOR_SLOT_TO_AXIS) for these calls, or otherwise convert motor_slot_index -> protocol axis before sending. Consider adding/adjusting the unit test(s) to assert the correct axis value is passed to the microcontroller methods when HAS_ENCODER_W is enabled.

Suggested change

self.microcontroller.set_pid_arguments(motor_slot, PID_P_W, PID_I_W, PID_D_W)

self.microcontroller.configure_stage_pid(motor_slot, config.transitions_per_revolution, ENCODER_FLIP_DIR_W)

self.microcontroller.turn_on_stage_pid(motor_slot, ENABLE_PID_W)

# Use protocol axis (AXIS.W / AXIS.W2), not motor_slot index (3 / 4),

# because the firmware's protocol_axis_to_internal() handles mapping.

self.microcontroller.set_pid_arguments(axis, PID_P_W, PID_I_W, PID_D_W)

self.microcontroller.configure_stage_pid(axis, config.transitions_per_revolution, ENCODER_FLIP_DIR_W)

self.microcontroller.turn_on_stage_pid(axis, ENABLE_PID_W)

[Claude Code] Fixed in commit b9b0093 - PID setup now uses axis (AXIS.W=5 / AXIS.W2=6) instead of motor_slot (3/4). Good catch!

Copilot · 2026-01-27T05:15:37Z

+# Add parent directory to path for imports
+sys.path.insert(0, ".")
+
+from control._def import *


Import pollutes the enclosing namespace, as the imported module control._def does not define 'all'.

Suggested change

from control._def import *

from control._def import (

AXIS,

SCREW_PITCH_W_MM,

STAGE_MOVEMENT_SIGN_W,

MICROSTEPPING_DEFAULT_W,

FULLSTEPS_PER_REV_W,

)

[Claude Code] Skipped - This is a test/tool script, not production code. The import * pattern is acceptable for quick testing scripts.

Copilot · 2026-01-27T05:15:37Z

+_log = squid.logging.get_logger(__name__)
+
+
 class SquidFilterWheel(AbstractFilterWheelController):


This class does not call AbstractFilterWheelController.init during initialization. (SquidFilterWheel.init may be missing a call to a base class init)

[Claude Code] Skipped - AbstractFilterWheelController.init is abstract and contains only pass. Calling super().init() would have no effect. This is consistent with other controller implementations in the codebase.

The PID setup calls (set_pid_arguments, configure_stage_pid, turn_on_stage_pid) were incorrectly using motor_slot (3/4) instead of protocol axis constants (AXIS.W=5, AXIS.W2=6). The firmware's protocol_axis_to_internal() expects protocol axis values, so passing 3/4 resulted in invalid axis (0xFF). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

hongquanli and others added 3 commits January 26, 2026 03:58

hongquanli requested a review from Copilot January 27, 2026 01:51

Copilot started reviewing on behalf of hongquanli January 27, 2026 01:51 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

hongquanli and others added 8 commits January 26, 2026 18:22

fix(firmware): Add W2 case to callback_configure_stage_pid()

41d5769

PID configuration for W2 encoder was silently ignored because the callback_configure_stage_pid() function had no case for axis == w2. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

hongquanli requested a review from Copilot January 27, 2026 05:10

Copilot started reviewing on behalf of hongquanli January 27, 2026 05:11 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

hongquanli merged commit 9a74aea into master Jan 27, 2026
4 checks passed

		static const int camera_trigger_pins[] = {29, 30, 31, 32}; // trigger 1-4 (pin 16 used for W2 CS, pin 28 used for W2 CLK)

-static const int camera_trigger_pins[] = {29, 30, 31, 32}; // trigger 1-4 (pin 16 used for W2 CS, pin 28 used for W2 CLK)
+static const int camera_trigger_pins[] = {29, 30, 31, 32}; // trigger 1-4 (pin 16 used for W2 CS, pin 28 used for W2 CLK)
+static const uint8_t NUM_CAMERA_TRIGGER_PINS = sizeof(camera_trigger_pins) / sizeof(camera_trigger_pins[0]);
+static inline bool is_valid_camera_channel(uint8_t camera_channel) {
+	return camera_channel < NUM_CAMERA_TRIGGER_PINS;
+}

		# Use multi-wheel configs if available, otherwise fall back to single config
		configs = config.squid_wheel_configs if config.squid_wheel_configs else config.controller_config

		@@ -8,107 +8,269 @@


		class SquidFilterWheel(AbstractFilterWheelController):

-            except Exception:
-                pass
+            except Exception as exc:
+                logger.warning(
+                    "Failed to get filter wheel name from config for wheel_id=%s: %s",
+                    wheel_id,
+                    exc,
+                )

-            self.microcontroller.set_pid_arguments(motor_slot, PID_P_W, PID_I_W, PID_D_W)
-            self.microcontroller.configure_stage_pid(motor_slot, config.transitions_per_revolution, ENCODER_FLIP_DIR_W)
-            self.microcontroller.turn_on_stage_pid(motor_slot, ENABLE_PID_W)
+            # Use protocol axis (AXIS.W / AXIS.W2), not motor_slot index (3 / 4),
+            # because the firmware's protocol_axis_to_internal() handles mapping.
+            self.microcontroller.set_pid_arguments(axis, PID_P_W, PID_I_W, PID_D_W)
+            self.microcontroller.configure_stage_pid(axis, config.transitions_per_revolution, ENCODER_FLIP_DIR_W)
+            self.microcontroller.turn_on_stage_pid(axis, ENABLE_PID_W)

-from control._def import *
+from control._def import (
+    AXIS,
+    SCREW_PITCH_W_MM,
+    STAGE_MOVEMENT_SIGN_W,
+    MICROSTEPPING_DEFAULT_W,
+    FULLSTEPS_PER_REV_W,
+)

		_log = squid.logging.get_logger(__name__)


		class SquidFilterWheel(AbstractFilterWheelController):

Conversation

hongquanli commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Firmware Changes

Critical Bug Fixes (from code review)

Software Changes

Configuration

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 Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

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 Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hongquanli commented Jan 26, 2026 •

edited

Loading