Skip to content

control/microcontroller: convert to using Logger and remove some unused/commented code - #5

Merged
ianohara merged 3 commits into
masterfrom
ian-microcontroller-logging
Oct 25, 2024
Merged

control/microcontroller: convert to using Logger and remove some unused/commented code#5
ianohara merged 3 commits into
masterfrom
ian-microcontroller-logging

Conversation

@ianohara

Copy link
Copy Markdown
Collaborator

See title.

Tested by: First by running python main_hcs.py --simulation and python main_hcs.py --simulation --verbose. I will also run this on the system itself.

@ianohara
ianohara force-pushed the ian-unused-imports-and-logging branch from 3443da1 to 7df7369 Compare October 25, 2024 21:14
@ianohara
ianohara force-pushed the ian-microcontroller-logging branch from 35cdd64 to bd6ea51 Compare October 25, 2024 21:16
@ianohara
ianohara changed the base branch from ian-unused-imports-and-logging to master October 25, 2024 21:16
@ianohara
ianohara merged commit 12a64f9 into master Oct 25, 2024
@ianohara
ianohara deleted the ian-microcontroller-logging branch December 27, 2024 17:47
hongquanli added a commit that referenced this pull request Dec 30, 2025
- Add comment clarifying SET_LIM uses limit codes instead of axis IDs
- Add comment noting MERGIN typo is intentional to match firmware
- Replace defensive padding/truncation with assertion for MSG_LENGTH

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
hongquanli added a commit that referenced this pull request May 18, 2026
…tbeat-skip, etc.

Addresses six issues from the copilot-pull-request-reviewer bot review:

#1 (cephla.py): Call acknowledge_aborted_command() after catching
CommandAborted (and after the inner resend failure if it was also a
CommandAborted), so the next send_command doesn't log the spurious
"Last command aborted and not cleared before new command sent!"
warning. The inner ack is gated on isinstance(e2, CommandAborted) to
avoid the "ack with nothing to ack" path on TimeoutError.

#2 (cephla.py): Drop the redundant `target_usteps = ...` recompute
after _home_wheel. config and target_pos haven't changed and
_target_pos_to_usteps doesn't depend on current_pos.

#3 (cephla.py): Fix _home_wheel docstring — wheel is driven to
config.min_index (typically slot 1), not "slot 0".

#5 (firmware/serial_communication.cpp): Skip the
`mcu_cmd_execution_status = COMPLETED_WITHOUT_ERRORS` reset when
processing a HEARTBEAT. The keepalive has no result to report, and
resetting would clobber a pending CMD_EXECUTION_ERROR from the
previous command if the broadcast hasn't fired yet. Eliminates the
narrow race where heartbeat traffic interleaves a failure broadcast.

#6 (firmware/stage_commands.cpp): Split mark_move_failed() into two
helpers — mark_move_failed() (for paths that already set
mcu_cmd_execution_in_progress = true) and report_move_error() (for
early-return paths that didn't). The !enabled branch in
dispatch_filterwheel_move now uses report_move_error() so it doesn't
spuriously unwind in_progress for an unrelated motion in flight on
another axis. Invariant: only the function that claimed in_progress
gets to clear it.

#7 (test_filter_wheel.py): Add `getattr(mc, move_rel_attr).assert_not_called()`
to both parametrized CommandAborted/TimeoutError tests, so the
absolute-MOVETO recovery path is enforced — fall-back to relative
MOVE would now be caught.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant