Skip to content

Update id logic to use standard movable - #2161

Merged
oliwenmandiamond merged 31 commits into
mainfrom
update_id_logic_to_use_standard_movable
Aug 18, 2026
Merged

Update id logic to use standard movable#2161
oliwenmandiamond merged 31 commits into
mainfrom
update_id_logic_to_use_standard_movable

Conversation

@oliwenmandiamond

@oliwenmandiamond oliwenmandiamond commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #2158

This fixes the insertion device logic to be compatible with StandardMovable, specifically the UndulatorGap. This has also restructured the ID logic so that we now have these new files:

  • apple2_undulator_phases.py
  • apple2_undulator_gap.py
  • apple2_undulator_base.py
  • apple2_undulaltor_motors.py
  • apple2_controller.py

Other notable change is MotorStringSetpoint which allows us to still use Motor which have setpoints as str still take floats using a derived signal, allowing the ID logic to not break when using MotorMovableLogic (soon to be MotorFlyableMoveLogic).

This has made the logic much more modular and easier to understand. Please note I have not altered the actual logic or the tests so this hasn't changed the ID behaviour. This was necessary because Motor has changed to use StandardMovable (and in ophyd-async 0.21.1 StandardFlyable) which made the current API conflicting as it was overriding methods it shouldn't of. This will make the migration to ophyd-async 0.21.1 much easier to manage. I've also added more documentation to classes to make it easier for someone else to maintain in the future.

Instructions to reviewer on how to test:

  1. Check refactor makes sense
  2. Tests pass
  3. dodal connect works

Checks for reviewer

  • Would the PR title make sense to a scientist on a set of release notes
  • If a new device has been added does it follow the standards
  • If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
  • Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.19%. Comparing base (25167f2) to head (48a313c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2161   +/-   ##
=======================================
  Coverage   99.19%   99.19%           
=======================================
  Files         361      364    +3     
  Lines       14339    14378   +39     
=======================================
+ Hits        14223    14262   +39     
  Misses        116      116           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@oliwenmandiamond
oliwenmandiamond marked this pull request as ready for review August 6, 2026 09:59
@oliwenmandiamond
oliwenmandiamond requested a review from a team as a code owner August 6, 2026 09:59

@Relm-Arrowny Relm-Arrowny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look good thanks for doing all the work.

Comment thread src/dodal/devices/insertion_device/apple2_undulator_base.py
Comment thread src/dodal/devices/insertion_device/apple2_undulator_gap.py Outdated
Comment thread src/dodal/devices/insertion_device/apple2_undulator_phase_axes.py Outdated
Comment thread src/dodal/devices/insertion_device/apple2_undulator_phase_axes.py Outdated
Comment thread src/dodal/devices/insertion_device/apple2_undulator_phase_axes.py
Comment thread src/dodal/devices/insertion_device/apple2_undulator_gap.py Outdated
Comment thread src/dodal/devices/insertion_device/apple2_undulator_base.py Outdated
Comment thread src/dodal/devices/insertion_device/apple2_controller.py Outdated
Co-authored-by: Raymond Fan <raymond.fan@diamond.ac.uk>

@Relm-Arrowny Relm-Arrowny left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks LGTM, love the pr note.

@oliwenmandiamond
oliwenmandiamond merged commit faf8c44 into main Aug 18, 2026
18 of 19 checks passed
@oliwenmandiamond
oliwenmandiamond deleted the update_id_logic_to_use_standard_movable branch August 18, 2026 08: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.

Update insertion device devices to user StandardMovable and StandardFlyer

2 participants