Skip to content

Conversation

@Villtord
Copy link
Contributor

@Villtord Villtord commented Jul 31, 2025

Fixes #1424

Instructions to reviewer on how to test:

  1. dodal connect i05, i05-1
  2. Confirm no errors in tests

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
Copy link

codecov bot commented Jul 31, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.75%. Comparing base (d8902b2) to head (c6be37c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1418      +/-   ##
==========================================
+ Coverage   98.74%   98.75%   +0.01%     
==========================================
  Files         243      243              
  Lines        8769     8859      +90     
==========================================
+ Hits         8659     8749      +90     
  Misses        110      110              

☔ 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.

@Villtord Villtord changed the base branch from main to create-i05-shared-beamline-module July 31, 2025 15:27
Base automatically changed from create-i05-shared-beamline-module to main August 1, 2025 12:55
@Villtord Villtord self-assigned this Aug 4, 2025
@Villtord
Copy link
Contributor Author

Villtord commented Sep 10, 2025

Blocked by https://jira.diamond.ac.uk/browse/I05-731
EDIT: successful "dodal connect" is blocked by requested epics change

@Villtord Villtord marked this pull request as ready for review September 11, 2025 12:29
@Villtord Villtord requested a review from a team as a code owner September 11, 2025 12:29
Copy link
Contributor

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

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

Thanks. Mostly style suggestions. The only one I think might be a mistake is the enum names

@oliwenmandiamond
Copy link
Contributor

I'm really dislike defining devices in shared and then just redefining them on the beamline branch as we have config in multiple places for the same thing e.g

from dodal.beamlines.beamline_utils.i05_shared import m3mj6_switching_mirror
...
@device_factory()
def m3mj6() -> XYZPiezoSwitchingMirror:
    return m3mj6_switching_mirror()

It's not needed because blueapi supports loading in multiple beamlines e.g i05 and i05_shared. It's only kinda needed for dodal connect which this change addresses #1532 and needs discussion on standard of shared beamlines

@Villtord
Copy link
Contributor Author

@oliwenmandiamond thanks I'm on your side with that, but atm I think we just stick to what we have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create some common optics mirrors classes and populate I05 optics with them

4 participants