Skip to content

Add motor movement callback to SimMonitor#193

Open
cappel89 wants to merge 2 commits intomainfrom
add-sim-monitor-updates-with-motor-readback
Open

Add motor movement callback to SimMonitor#193
cappel89 wants to merge 2 commits intomainfrom
add-sim-monitor-updates-with-motor-readback

Conversation

@cappel89
Copy link
Copy Markdown
Member

@cappel89 cappel89 commented Apr 7, 2026

LIttle PR to fix a thread leak for the move method of SimPositioner, and have SimMonitor to register a callback on any change of readback from the ref_motor.

@cappel89 cappel89 requested a review from a team April 7, 2026 11:04
@cappel89 cappel89 self-assigned this Apr 7, 2026
Copilot AI review requested due to automatic review settings April 7, 2026 11:04
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 a motor-motion-driven update path for SimMonitor so its computed readback can refresh automatically when the configured reference motor moves.

Changes:

  • Add a motor subscription mechanism in SimMonitor (setup_readback_monitor / _update_readback) to trigger monitor readback updates on motor motion.
  • Extend SimulatedDataMonitor to register the motor callback when models/params are selected/updated.
  • Update the monitor readback test to construct a SimPositioner and assert that monitor subscriptions fire after motor motion.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
ophyd_devices/sim/sim_monitor.py Introduces motor subscription plumbing to trigger monitor updates on motor motion.
ophyd_devices/sim/sim_data.py Hooks motor callback registration into monitor sim model selection/parameter updates; refactors param-setting helper.
tests/test_simulation.py Adjusts monitor readback test to use a simulated motor and adds a callback-on-motion assertion.

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

Comment thread ophyd_devices/sim/sim_data.py
Comment thread ophyd_devices/sim/sim_monitor.py Outdated
Comment thread tests/test_simulation.py
Comment thread tests/test_simulation.py
Comment thread ophyd_devices/sim/sim_monitor.py
Comment thread ophyd_devices/sim/sim_data.py
Comment thread ophyd_devices/sim/sim_monitor.py Outdated
@cappel89 cappel89 force-pushed the add-sim-monitor-updates-with-motor-readback branch from fb18e92 to 2511557 Compare April 15, 2026 13:01
@cappel89 cappel89 marked this pull request as draft April 27, 2026 09:32
@cappel89 cappel89 force-pushed the add-sim-monitor-updates-with-motor-readback branch from 6265e76 to 4c14fa7 Compare April 27, 2026 09:42
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

❌ Patch coverage is 84.28571% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
ophyd_devices/sim/sim_positioner.py 73.33% 4 Missing ⚠️
ophyd_devices/sim/sim_data.py 89.28% 3 Missing ⚠️
ophyd_devices/sim/sim_monitor.py 85.71% 3 Missing ⚠️
ophyd_devices/utils/static_device_test.py 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@cappel89 cappel89 marked this pull request as ready for review April 27, 2026 10:50
@cappel89 cappel89 requested a review from a team April 27, 2026 10:50
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.

3 participants