Pr turret by veerwang · Pull Request #609 · Cephla-Lab/Squid

veerwang · 2026-08-12T08:00:11Z

Summary

Ports the objective-turret controller changes from veerwang's SingleMotor tool onto this branch, then tightens the port against SingleMotor's hardware-tested behaviour:

Re-port SingleMotor logic: software homing (sweep → backoff → fine-search → set zero), DI1 origin switch, factory params auto-calibrated on connect
Sync SingleMotor tuning: fine-step 2, backoff 60, max speed 150, fine-search accel 50
Motor direction inversion (OBJECTIVE_TURRET_DIRECTION_INVERTED) and origin-switch polarity inversion (OBJECTIVE_TURRET_DI_INVERT)
Gear backlash compensation (OBJECTIVE_TURRET_BACKLASH_DEG): every slot change approaches its target from below
Uniform slot spacing from a single offset (43c6464): slot n = OBJECTIVE_TURRET_OFFSET_PULSES + (n−1) × 2200. The per-slot OBJECTIVE_TURRET_CALIBRATED_PULSES map from an earlier revision was dropped on purpose — one measured offset is enough on a 90°-indexed turret. A microstep mismatch is refused at start and never auto-written: the register reads back the pending value but only takes effect after a power cycle.
Objective switch runs off the GUI thread (acada20, 97ee5a3)
Homing backoff jogs before reading (f133442), matching SingleMotor, so a decel-stop that coasts past the far edge of the ~50-pulse sensor window is pulled back in instead of the fine search walking away
Fixed-period sweep poll + move-start grace (a5fdef8): the 20 ms DI poll is a period with the Modbus round trip inside it, as in SingleMotor's timer, instead of a 20 ms gap after each read (which doubled detection lag and caused the far-edge case routinely). An idle status word is accepted as move-complete only after RUNNING was seen or 0.8 s elapsed, so a move starting inside the position tolerance is not cut short. Move wait uses the same batched status snapshot as homing.
Restore failure after a successful home now raises (4e93b18) instead of leaving later moves at homing speed/acceleration
tools/turret_setup.py (f4f546d): writes the factory params + microstep to the drive, saves to EEPROM, verifies, and walks through the power cycle; --check afterwards confirms the values persisted

Files changed

software/control/_def.py, objective_turret_controller.py, microscope.py, modbus_rtu.py, widgets.py
software/tools/turret_setup.py (new)
software/tests/control/test_objective_turret_controller.py, test_turret_setup_tool.py (new), test_objectives_widget.py

Notes

Toggling direction/DI inversion requires re-homing and re-measuring OBJECTIVE_TURRET_OFFSET_PULSES
New drive: python3 tools/turret_setup.py → power-cycle the drive → python3 tools/turret_setup.py --check → start the GUI
Current registers 0x16..0x18 count in % of the drive's 3 A rating (95 ≈ 3 A, ~31.6 mA/unit) per SingleMotor's 2026-07-31 oscilloscope ruling, not x10 mA; the register values are unchanged
Tests: turret controller + setup tool 76 passed (simulation, no hardware). Items 8–10 have not yet been run on hardware; first run should log position/DI at the sweep stop (a stop depth under 60 pulses confirms the poll fix)

🤖 Generated with Claude Code

https://claude.ai/code/session_01JRAcmjXX4CEN9wXSnp6HzK

Copilot

Pull request overview

Ports updated objective turret controller behavior, expanding the real controller to use software homing and adding configuration knobs for backlash compensation, motor-direction inversion, and DI (origin switch) polarity inversion; updates the Modbus client and tests accordingly.

Changes:

Implement software homing (sweep → backoff → fine search) and initialize/calibrate a factory parameter set on connect.
Add backlash compensation to slot changes, plus optional direction inversion and DI polarity inversion (wired/configurable via _def.py and passed through microscope.py).
Extend Modbus RTU client with a batched FC0x04 input-register read API and add substantial test coverage for the new turret behaviors.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
software/control/objective_turret_controller.py	Adds software homing, backlash compensation, direction/DI inversion handling, and init-time parameter calibration.
software/control/modbus_rtu.py	Adds `read_input_registers()` to read a consistent input-register snapshot in one FC0x04 transaction.
software/control/microscope.py	Passes new turret configuration kwargs (backlash/direction/DI inversion) from global config into controller construction.
software/control/_def.py	Introduces new turret configuration defaults and documents intended behavior/upgrade implications.
software/tests/control/test_objective_turret_controller.py	Adds tests for homing flows, init calibration, backlash compensation, direction inversion, and DI inversion.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Port two positioning features from the SingleMotor source project: - OBJECTIVE_TURRET_CALIBRATED_PULSES: per-slot calibrated absolute pulse targets (slot 1..4 -> pulses from homing zero). A calibrated slot is used verbatim; uncalibrated slots fall back to the theoretical (slot-1)*pulses_per_position + OBJECTIVE_TURRET_OFFSET_PULSES. - OBJECTIVE_TURRET_BACKLASH_DEG: gear backlash compensation (0..1 turret degrees). When > 0 every slot change overshoots below the target and approaches it from below, so the final approach direction is always the same and gear backlash cancels out. Both are validated at init (slot range, integer pulses, deviation bound of one slot vs theoretical, degree range) so a bad machine .ini fails fast. Defaults keep behavior identical to before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

… switch, factory params Re-merge from the SingleMotor source project (2026-07-23..27 changes): - Homing rewritten as software homing (sweep in velocity mode polling the DI level -> backoff -> fine-search -> SET_ZERO at the trigger edge), replacing the driver's built-in homing modes. Repeatability +/-5 pulses; ends clamped at home with holding torque. Default home timeout raised to 120s. - DI1 is now permanently "origin switch" (3). The old scheme (DI1 temporarily mapped to negative limit + homing method 17) faults FF0E on the current firmware whenever a move passes the sensor; all homing-register calibration is removed. - Factory parameter set ported and auto-calibrated on connect: accel/decel 1000, max speed 200, min speed 16 (written before max — fixes the min/max write-order bug), currents overload 1.3A / idle 0.6A / accel+run+decel 0.95A, microstep forced to 16 (write + save + fail fast asking for a power cycle when it differs). Decel current is marked volatile (firmware drops the write) so it cannot trigger an EEPROM save every connect. Direction register written to RAM after the EEPROM save. - modbus_rtu: add read_input_registers() batch read so the homing sweep gets DI + position + alarm in one frame per poll (the ~50-pulse sensor window must not be crossable between two polls). - _def.py: fix the calibrated-pulses doc example (6640 deviates more than one slot and fails validation) and note that slots must be re-measured after upgrading from driver homing (the zero reference moved). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ne-search accel, max speed 150 Port SingleMotor f366df9..4119c32 (manager-specified values): - Homing: fine step 5->2 (repeatability +/-2), backoff 150->60, sweep 50->60 Step/s with 20ms poll (52ms window crossing >= 2.6 polls), fine travel limit 400->200 - Fine search now temporarily lowers acceleration (0x005F) to 50 to soften the microstep approach to the trigger edge; restored after - Max speed 200->150: the 0.95A current cap loses steps under load at 200 - Idle current 60->21 (displayed 0.69A); drop the decel-current (0x15) calibration and its volatile mechanism — the SDM42 drive has no such parameter (writes silently dropped, reads back 0) SingleMotor's reconnect-polling fix (8ab21dc) is Qt-panel-specific and does not apply to this synchronous controller. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…db6c Some turret motor models are wired with the opposite phase order, so the same commands spin the other way. Add OBJECTIVE_TURRET_DIRECTION_INVERTED (per-machine .ini, default False = behavior unchanged): - Inversion applied only at the register boundary: absolute-move targets, jog signs, homing-sweep direction bit, and position readbacks; slot mapping / calibration / backlash / homing logic stay in logical coordinates untouched - Init direction-register (0x0052) expected value follows the flag (inverted expects 0), still RAM-only after the EEPROM save - microscope.py passes the flag via turret_kwargs; the simulation twin accepts it for constructor parity - 12 new tests incl. a default-off regression guard; theoretical-target assertions pass explicit offset/calibration to stay independent of machine .ini values loaded into _def After toggling on an existing machine, re-home and re-measure the calibrated slots (the physical zero moves with the sweep direction). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ngleMotor 8874934 New objective changers sense the origin switch on the opposite logic level (homing direction flipped, jerky). Add a per-machine software option that inverts the DI1 trigger verdict during software homing / distance search. - _def.py: OBJECTIVE_TURRET_DI_INVERT (default False = old logic) - objective_turret_controller.py: di_invert ctor kwarg (def fallback + bool validation, same pattern as direction_inverted); the verdict flips in _read_status_snapshot so the sweep/backoff/fine state machine, direction logic and calibration stay in the same logical frame - microscope.py: pass di_invert through turret_kwargs - tests: 6 di_invert tests (inside/outside window homing, backoff direction unchanged, def fallback, non-bool raises, sim accepts kwarg); 74 passed in a CI-equivalent env. Note: toggling requires re-homing — the fine-search edge (physical zero) sits on the other side of the sensor window. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…step auto-write Replace the per-slot OBJECTIVE_TURRET_CALIBRATED_PULSES map with uniform slot targets: offset + (slot-1) x PULSES_PER_SLOT. The 2200 pulses/slot spacing is now a named constant, cross-checked at init against the scale derived from the mechanics constants and the microstep readback so the two cannot drift apart. One measured offset per machine is enough; per-slot values are dropped. On a microstep mismatch, init now raises without writing: the register reads back the pending value (vendor-confirmed), so the previous write-then-raise would let the next start pass the check while the drive still runs the old scale until power-cycled. The error message directs to the SingleMotor setup tool instead. All factory register values and the motion/homing logic are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Single source of truth: PULSES_PER_SLOT is now computed from the mechanics constants (steps/rev x microsteps x gear ratio / slots) instead of being a hand-copied 2200 held in sync by a runtime cross-check. The duplicate _pulses_per_position instance field, the unreachable init check, and its monkeypatch-only test are gone; a one-line test pins the value at 2200. Also drops two tests fully subsumed by existing ones and inlines the single-caller _target_pulses helper. No behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

ObjectivesWidget called move_to_objective synchronously from the dropdown's Qt slot. A switch blocks for seconds (Z retract at 2 mm/s, changer motion, Z restore), which starved the event loop past the desktop's ~5 s threshold and raised the "force quit or wait" dialog on every switch. Run the changer through control.utils.threaded_operation_helper — the same pattern the XLight widget already uses — with the dropdown disabled until completion (doubles as the re-entry guard) and store update / signal emit marshalled back to the GUI thread. Any changer failure, not just KeyError, now ends in the warning + revert path instead of escaping the slot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- Read the objective name from the (disabled) dropdown in the completion slot instead of marshalling it across the thread boundary. - Re-enable the dropdown only after the modal failure warning: the dialog's nested event loop would otherwise accept a second switch behind it. - Sync the dropdown to the store at construction with signals blocked, so the widget no longer drives the changer (now a worker thread) on construction. - Tests: use the real ObjectiveStore stub, the repo's patch.object(QMessageBox) idiom, and a release Event instead of sleeps (deterministic, ~0 ms). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

_backoff_off_sensor read the switch first and only jogged when it still read triggered. SingleMotor's HomeSearch backoff phase does the opposite: it always steps before re-reading. The two agree on the normal path (the leg starts inside the window, and the first released read ends it either way) but diverge when the decel-stop coast punched through the window's FAR edge. There the pre-read version saw the switch already released and returned without moving, leaving the turret beyond the far edge with the switch reading clear; the fine search — which moves in the sweep direction — then walked away from the sensor until it overran HOMING_FINE_TRAVEL_LIMIT. Stepping first recovers a punch-through of up to HOMING_BACKOFF_STEP pulses: the jog pulls the turret back toward the window, the loop re-enters it and exits past the near edge, so _fine_search_to_edge approaches the same edge as on the normal path and the home reference does not shift. The recovery is bounded by that one jog — a deeper punch-through still leaves the first read released with the turret beyond the far edge (SingleMotor has the same bound). Tests: the DI scripts are physical level sequences consumed one per snapshot read, so they follow the read order, which now begins after the first jog. Four scripts shift by one frame; the reversed-direction test's direction-bit assertion now also sees the backoff jog it previously missed. Verified all four direction_inverted x di_invert combinations plus the punch-through path. Note: this branch was reset to the pre-85afd6c state and the port deliberately follows SingleMotor's semantics rather than 85afd6c's seen_trigger guard. Not yet verified on hardware. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…wait read Homing sweep: HOMING_POLL_S is a poll *period* with the Modbus round trip inside it (SingleMotor polls from a 20 ms QTimer), not a gap after each read. Sleeping the full 20 ms after every read stretched the period to 20 ms + round trip and roughly doubled the detection lag, so the decel-stop routinely landed past the ~50-pulse sensor window and homing depended on the far-edge backoff recovery (the 2026-09-09 "fine search overran the sensor window" on a Windows scope). Sleep only the remainder of the period; a read slower than the period polls again immediately. Move wait: an "idle" status word is no longer accepted as move-complete until the RUNNING bit has been seen or MOVE_START_GRACE_S (0.8 s, SingleMotor's start blackout) has elapsed. Without it a move that starts inside POSITION_TOLERANCE_PULSES of its target (the backlash final leg) could be declared done, and de-energized by the caller's finally block, before the drive raised RUNNING. The wait now takes status word and position from the same single-frame snapshot the homing loops use (one round trip per poll instead of two). Stall detection and the position check are unchanged. Shared with tools/turret_setup.py (next commit): module-level calibrate_register / read_register_value / write_register_value / format_register_value, prepare_for_parameter_writes, save_to_eeprom, public INIT_PARAMS and find_port, and POWER_CYCLE_PARAMS carrying the never-write microstep policy. The microstep refusal message now points at that tool. Comments: registers 0x16..0x18 are a percentage of the drive's 3 A rating (~31.6 mA/unit; 95 ~= 3 A), per SingleMotor's 2026-07-31 oscilloscope ruling, not x10mA. Values unchanged. Tests: deterministic _FakeTime clock installed on the controller module; _FakeModbus reports RUNNING for one snapshot after a move trigger, supports a scripted status word and a per-read clock cost. Five new tests cover the fixed-period remainder sleep, back-to-back polling on slow reads, the grace hold, early return once RUNNING clears, and stall detection. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRAcmjXX4CEN9wXSnp6HzK

One-time setup of a NiMotion turret drive. The controller verifies the microstep register but never writes it: 0x1A reads back the pending value yet only takes effect after a power cycle, so an auto-write would let the next start pass the check while the drive still runs the old scale. The write, the EEPROM save and the power cycle have to happen together, at the bench. The tool reads POWER_CYCLE_PARAMS + INIT_PARAMS through the controller's own calibrate_register (same values, same DI1 nibble mask), prints device vs expected, asks for confirmation, clears faults and disables the motor (prepare_for_parameter_writes), writes only the mismatched registers, always saves to EEPROM, reads back to verify, and tells the user to power-cycle and re-run with --check. The direction register (0x52) is runtime-only and never written. Serial number, slave id and baud rate default to the machine .ini; --port overrides. python3 tools/turret_setup.py # report, confirm, write, save python3 tools/turret_setup.py --check # read-only; exit 1 on mismatch Tests use a register-memory fake so read-backs reflect writes: check mode writes nothing, apply writes each mismatch once from a single read pass and saves last, save happens even when nothing differs, a dropped write fails verification, a declined confirmation writes nothing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRAcmjXX4CEN9wXSnp6HzK

Copilot

🟡 Changes recommended

Per-slot calibration is absent, and homing parameter writes can occur before the drive reaches its required disabled state.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

software/control/objective_turret_controller.py:537

The preceding jog leaves the drive enabled, and the acceleration register is written immediately after requesting CW_DISABLE. Since parameter writes are documented above as rejected until the disabled transition settles, this can leave the fine search at the original acceleration. Wait for CONTROL_WORD_SETTLE_S before writing REG_ACCEL.

                self._write_control(CW_DISABLE)  # parameter writes require the disabled state
                self._modbus.write_register_32bit(self._slave_id, REG_ACCEL, HOMING_FINE_ACCEL)

software/control/objective_turret_controller.py:546

The restoration writes immediately follow the disable request even though this drive requires time to enter the disabled state before accepting parameter writes. A rejected restore leaves the temporary homing speed/acceleration active for later objective moves. Wait for the control-word transition whenever either parameter needs restoration.

            self._deenergize()

software/control/objective_turret_controller.py:555

If either restore fails on an otherwise successful homing run, the exception is only logged; the method then clamps, reports success, and future moves use the temporary homing speed or acceleration. Preserve the original homing exception during failure cleanup, but surface restoration failure (or mark the controller unusable) when the homing body succeeded.

                try:
                    self._modbus.write_register_32bit(self._slave_id, REG_MAX_SPEED, orig_max_speed)
                except Exception as exc:
                    logger.warning("Failed to restore max speed after homing: %s", exc)
            if accel_lowered:
                try:
                    self._modbus.write_register_32bit(self._slave_id, REG_ACCEL, orig_accel)
                except Exception as exc:

Files reviewed: 9/9 changed files
Comments generated: 2
Review effort level: Balanced

        self._require_open()
+        deadline = time.monotonic() + timeout_s
+        # Parameter writes require the disabled state.
        self._write_control(CW_DISABLE)


    def _rotate_to(self, objective_name: str, timeout_s: float) -> None:
        position_index = _resolve_position(objective_name, self._positions)
-        target_pulses = (position_index - 1) * self._pulses_per_position + self._offset_pulses
+        target_pulses = (position_index - 1) * PULSES_PER_SLOT + self._offset_pulses


home() temporarily lowers max speed (and, for the fine search, acceleration) and restores both in a finally block. The restores were best-effort so that cleanup could never mask the fault or timeout that ended a failed run, but that also swallowed a failed restore after a *successful* run: home() then clamped, returned normally, and every later objective move ran at the homing speed or acceleration. Keep the best-effort behaviour while an exception is propagating, and raise a RuntimeError (chained to the Modbus error) once the homing body has completed if either restore failed. Both restores are still attempted independently and the motor is left de-energized, not clamped. Addresses the Copilot review comment on the finally block (2026-09-14). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRAcmjXX4CEN9wXSnp6HzK

Alpaca233 requested a lite review from Copilot August 22, 2026 04:09

Copilot started reviewing on behalf of Alpaca233 August 22, 2026 04:10 View session

Copilot AI reviewed Aug 22, 2026

View reviewed changes

Comment thread software/control/objective_turret_controller.py Outdated

veerwang and others added 11 commits September 9, 2026 23:07

docs(turret): tighten comments

f9bea61

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

docs(turret): fix inverted homing comment (Copilot review)

0d6101e

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Alpaca233 force-pushed the pr-turret branch from a7099ce to 97ee5a3 Compare September 10, 2026 06:08

veerwang and others added 3 commits September 11, 2026 16:58

Alpaca233 requested a balanced review from Copilot September 14, 2026 22:27

Copilot started reviewing on behalf of Alpaca233 September 14, 2026 22:27 View session

Copilot AI reviewed Sep 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

Pr turret - #609

Pr turret#609
veerwang wants to merge 15 commits into
Cephla-Lab:masterfrom
veerwang:pr-turret

veerwang commented Aug 12, 2026 •

edited by Alpaca233

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

veerwang commented Aug 12, 2026 • edited by Alpaca233 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Files changed

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (3)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

veerwang commented Aug 12, 2026 •

edited by Alpaca233

Loading