-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check stepper alarm status after move #816
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds functionality to check the stepper motor controller’s alarm status after a move. It introduces a new ST10AlarmCode enum, updates signal handling to check the alarm state before sending a move-end message, and adds a utility method for converting responses to integers.
- Added a new ST10AlarmCode IntFlag enum to encapsulate alarm bit flags.
- Modified the move end handling to call _on_move_end which checks the alarm state.
- Introduced the _request_int method to simplify numeric conversions from device responses.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
tests/hardware/plugins/stepper_motor/test_st10_controller.py | Updated tests to reflect the new _on_move_end handler and added tests for the alarm_code property. |
frog/hardware/plugins/stepper_motor/st10_controller.py | Introduced ST10AlarmCode enum, _on_move_end logic for alarm checking, and a new _request_int method for integer conversion. |
Comments suppressed due to low confidence (1)
frog/hardware/plugins/stepper_motor/st10_controller.py:532
- [nitpick] Consider including the requested variable name (e.g., '{name}') in the exception message to improve debugging clarity. For example, change the error message to 'Non-integer response received for {name}: {resp}'.
return int(resp, base)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems sensible, and looks good overall! Just made a couple of suggestions, and wondering whether we're missing some tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No test for ST10Controller._request_int
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added one now as well as one for _on_move_end
Co-authored-by: Dan Cummins <45606273+dc2917@users.noreply.github.com>
Description
The ST10 controller has an "alarm status" that can be queried to find out if an error has occurred. This includes drive faults, low voltage etc. as well as if the motor has hit a limit switch (which was the original motivation for looking at this -- currently we just hardcode the position of the limits in FROG so we don't hit them). Jon has said that he might not put limit switches on the new UNIRAS rig, in which case we could just turn them off, but in any case it seems useful to know whether something's gone wrong with the motor, especially if you're 10,000 ft up.
When an error occurs, we should get a notification because the motor will stop so it makes sense to query the motor at this point.
Closes #703.
Type of change
Key checklist
pre-commit run -a
)pytest
)mkdocs build -s
)pyinstaller
-built executable works (if relevant)Further checks