feat(worker): log actual z after each per-channel z-offset move - #573
Merged
Conversation
Add an info log in _move_z_for_offset (covers both the apply and reset paths) reporting the delta moved and the resulting position: the measured stage z, plus the piezo position on the piezo path. Makes it easy to confirm from the log where z actually landed after each channel offset. Give the offset test stub's stage.get_pos() a numeric z_mm so the new log f-string formats under the mock. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds info-level telemetry to MultiPointWorker so each per-channel z-offset adjustment logs the requested/achieved delta and the resulting measured stage z position (read back via stage.get_pos()), covering both apply and reset paths through _move_z_for_offset.
Changes:
- Emit an info log after each z-offset move in
_move_z_for_offset, for both piezo and stage paths, including a stage z readback. - Update the MultiPointWorker offset unit test stub to return a numeric
z_mmfromstage.get_pos()so the new formatted log line doesn’t raise under mocks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| software/control/core/multi_point_worker.py | Adds info logs that include post-move measured stage z after each per-channel z-offset move (piezo and stage paths). |
| software/tests/control/test_MultiPointWorker_offsets.py | Adjusts the stage mock to provide numeric z_mm to support the new formatted logging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
_update_apply_channel_offset_enable_state force-unchecked the checkbox when laser AF turned off but never re-checked it when AF came back on, so a laser-AF off->on cycle silently dropped the user's opt-in — the visible checkbox no longer matched whether offsets were actually applied. Make the handler visibility-only: the checked state is now changed solely by the user, so it always reflects the apply_channel_offset flag. This is safe because offset application is already double-gated on reflection AF in the worker (_apply_channel_z_offset), so a retained opt-in stays inert while AF is off. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-channel z-offset move log rounded stage z to %.4f mm (0.1 µm), which sat right at the size of the deviations one wants to inspect — making measured movement look quantized. Bump to %.5f mm (10 nm) so actual moves can be checked below the 0.1 µm grid. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hongquanli
added a commit
that referenced
this pull request
Jul 4, 2026
Conflict resolution notes (software/control/core/multi_point_worker.py): this branch MOVED the shared capture mechanics (_image_callback, acquire_camera_image, _wait_for_outstanding_callback_images, _sleep) into MultiPointWorkerBase, while master MODIFIED them in place (acquisition watchdog #565, per-channel z-offset logging #573, error-vs-user-abort classification). Resolution keeps the refactored layout and ports master's edits into the base-class copies: - _abort_due_to_error and a no-op _run_state_beat live on the base (MultiPointWorker overrides the beat with the real watchdog one), so every request_abort_fn() error site in the moved methods now tags _abort_cause = "error" exactly as on master. - _abort_cause initialized in the base __init__; MultiPointWorker keeps run_state_writer/_run_state and _compute_end_reason. - Per-image _run_state_beat() call added in the base _image_callback, matching master's placement after the image counters. - Master's z-offset move logging auto-merged (methods not moved). Verified: record-zstack suites + master's watchdog/worker-reason tests (127 passed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an info-level log line reporting the actual z position after each per-channel
z-offset move during multi-point acquisition. The log is emitted from
_move_z_for_offset, so it covers both the apply path (_apply_channel_z_offset)and the reset/undo path (
_reset_channel_z_offset).Each log records the delta moved and where z landed:
[z-offset] moved +1.50 µm via stage; actual z = 3.2000 mm[z-offset] moved +1.50 µm via piezo; piezo at 101.50 µm, actual stage z = 3.2000 mmThe stage z is read back with
stage.get_pos()(the real, measured position) so it can beused to confirm each channel offset actually landed where expected.
Why
When diagnosing per-channel z-offset behavior (PR #551), it's useful to see the resulting
z after each offset move rather than only the requested delta.
Test plan
pytest tests/control/test_MultiPointWorker_offsets.py— 25 passed, 1 skippedblack --config pyproject.toml --check— cleanTest stub's
stage.get_pos()was given a numericz_mmso the new log f-string formatsunder the mock.
🤖 Generated with Claude Code