Converting MX Beamlines to device manager #1788

adaudon · 2025-12-16T11:07:35Z

Instructions to reviewer on how to test:

Do thing x
Confirm thing y happens

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 · 2025-12-18T11:17:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.09%. Comparing base (00af332) to head (783ba6d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1788      +/-   ##
==========================================
- Coverage   99.12%   99.09%   -0.03%     
==========================================
  Files         283      283              
  Lines       10736    10745       +9     
==========================================
+ Hits        10642    10648       +6     
- Misses         94       97       +3

☔ View full report in Codecov by Sentry.
📢 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.

…amondLightSource/dodal into 1765-Convert_MX_to_new_device_manager

tpoliaw · 2026-01-05T14:58:08Z

src/dodal/beamlines/i04.py

-    """
    return OAVToRedisForwarder(
        f"{PREFIX.beamline_prefix}-DI-OAV-01:",
        oav_roi=oav(),


I think this should be updated to pass oav in to avoid multiple instances

Yes, I agree, and we should do the same for the full screen one. e.g.:

@devices.factory() def oav_to_redis_forwarder(oav: OAVBeamCentrePV, oav_full_screen:OAVBeamCentrePV) -> OAVToRedisForwarder: return OAVToRedisForwarder( f"{PREFIX.beamline_prefix}-DI-OAV-01:", oav_roi=oav, oav_fs=oav_full_screen, redis_host=RedisConstants.REDIS_HOST, redis_password=RedisConstants.REDIS_PASSWORD, redis_db=RedisConstants.MURKO_REDIS_DB, )

src/dodal/beamlines/i23.py

…amondLightSource/dodal into 1765-Convert_MX_to_new_device_manager

DominicOram

Great, thank you. A couple more comments

DominicOram · 2026-01-08T13:25:06Z

src/dodal/beamlines/i02_2.py

    """Get the i02-2 synchrotron device, instantiate it if it hasn't already been.
    If this is called when already instantiated in i02-2, it will return the existing object.
    """


Should: We can remove this comment, and the one below, as we have done elsewhere

DominicOram · 2026-01-08T13:29:31Z

src/dodal/beamlines/i04.py

-    )
+@devices.v1_init(EigerDetector, prefix="BL03I-EA-EIGER-01:", wait=False)
+def eiger(eiger: EigerDetector) -> EigerDetector:
+    return eiger


Must: You need to set the detector_id here, like we did for i03 (

dodal/src/dodal/beamlines/i03.py

Line 162 in 8b911e5

eiger.detector_id = 78

). You probably also need to set the beamline in the same way, though it's not as important

Should it be BL04I instead of BL03I?

Yes, good spot. In both here and in i03.py they should be:

@devices.v1_init( EigerDetector, prefix=f"{PREFIX.beamline_prefix}-EA-EIGER-01:", wait=False )

DominicOram · 2026-01-08T13:35:29Z

src/dodal/beamlines/i04.py

-    """
    return OAVToRedisForwarder(
        f"{PREFIX.beamline_prefix}-DI-OAV-01:",
        oav_roi=oav(),


Yes, I agree, and we should do the same for the full screen one. e.g.:

@devices.factory() def oav_to_redis_forwarder(oav: OAVBeamCentrePV, oav_full_screen:OAVBeamCentrePV) -> OAVToRedisForwarder: return OAVToRedisForwarder( f"{PREFIX.beamline_prefix}-DI-OAV-01:", oav_roi=oav, oav_fs=oav_full_screen, redis_host=RedisConstants.REDIS_HOST, redis_password=RedisConstants.REDIS_PASSWORD, redis_db=RedisConstants.MURKO_REDIS_DB, )

DominicOram · 2026-01-08T13:37:52Z

src/dodal/beamlines/i04.py



-@device_factory(
-    skip=BL == "s04",


Should: Now we're removing s04 from here can we also remove it from the dictionary in dodal/tests/conftest.py

DominicOram · 2026-01-08T13:40:59Z

src/dodal/beamlines/i24.py

-@device_factory()
+@devices.factory()
 def attenuator() -> EnumFilterAttenuator:
    """Get a read-only attenuator device for i24, instantiate it if it hasn't already


Should: These comments, and others in this file, should be removed as they have been elsewhere

DominicOram · 2026-01-08T13:44:16Z

src/dodal/beamlines/i24.py

    """
    return Aperture(
        f"{PREFIX.beamline_prefix}-AL-APTR-01:",
    )


Could: Whilst we're here the formatting of a lot of these device instantiations is a bit odd. If we remove the trailing comma the save on format will make this one line. Same with many other devices in this file.

DominicOram · 2026-01-08T13:45:14Z

src/dodal/beamlines/i24.py

 #         wait=wait_for_connection,
 #         fake=fake_with_ophyd_sim,
 #         post_create=set_params,
 #     )


Should: This commented out code is now entirely wrong. I think we should delete it all.

adaudon force-pushed the 1765-Convert_MX_to_new_device_manager branch 2 times, most recently from 5c175c7 to 4a05f61 Compare December 18, 2025 11:14

Tamoor Shahid added 6 commits December 18, 2025 14:21

Partially Converted easy changes in I04 to DeviceManager

e6eebf6

Changed I04 Eiger to use DeviceManager

b01fcee

Converted I02-1 to Device Manager

9b12b3b

Converted I02-2 to Device Manager

a07a000

Converted I23 to Device Manager

35b4680

Converted I24 to Device Manager

662aa54

adaudon force-pushed the 1765-Convert_MX_to_new_device_manager branch from f65166e to 662aa54 Compare December 18, 2025 14:21

Converted i04 to Device Manager

9bfbbc4

adaudon force-pushed the 1765-Convert_MX_to_new_device_manager branch from f98b994 to 9bfbbc4 Compare December 18, 2025 16:16

adaudon and others added 3 commits January 5, 2026 14:13

Merge branch 'main' into 1765-Convert_MX_to_new_device_manager

4698a69

Removed unused imports

d07eb00

Merge branch '1765-Convert_MX_to_new_device_manager' of github.com:Di…

e2c0241

…amondLightSource/dodal into 1765-Convert_MX_to_new_device_manager

adaudon marked this pull request as ready for review January 5, 2026 14:24

adaudon requested a review from a team as a code owner January 5, 2026 14:24

tpoliaw reviewed Jan 5, 2026

View reviewed changes

adaudon and others added 3 commits January 6, 2026 13:58

Merge branch 'main' into 1765-Convert_MX_to_new_device_manager

11d1439

Added fixture changes to path_provider

55d03fe

Merge branch '1765-Convert_MX_to_new_device_manager' of github.com:Di…

783ba6d

…amondLightSource/dodal into 1765-Convert_MX_to_new_device_manager

DominicOram requested changes Jan 8, 2026

View reviewed changes

Converting MX Beamlines to device manager #1788

Are you sure you want to change the base?

Converting MX Beamlines to device manager #1788

Conversation

adaudon commented Dec 16, 2025

Instructions to reviewer on how to test:

Checks for reviewer

Uh oh!

codecov bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DominicOram left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Dec 18, 2025 •

edited

Loading