-
Notifications
You must be signed in to change notification settings - Fork 12
I16 insertion device #1778
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
I16 insertion device #1778
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1778 +/- ##
=======================================
Coverage 99.11% 99.12%
=======================================
Files 281 282 +1
Lines 10667 10683 +16
=======================================
+ Hits 10573 10589 +16
Misses 94 94 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Just a nit with the name. This is also a good opportunity to use the new DeviceManager instead of device_factory as we need to migrate across (so it no longer uses a global context and instead use something to manage the context). See dodal/beamlines/i03.py for an example
e.g
from dodal.device_manager import DeviceManager
...
devices = DeviceManager()
@devices.factory
def id() -> ...
src/dodal/beamlines/i16.py
Outdated
| @device_factory() | ||
| def id() -> UndulatorInMm: | ||
| return UndulatorInMm( | ||
| name="id", prefix=f"{BeamlinePrefix(BL).insertion_prefix}-MO-SERVC-01:" | ||
| ) |
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.
| @device_factory() | |
| def id() -> UndulatorInMm: | |
| return UndulatorInMm( | |
| name="id", prefix=f"{BeamlinePrefix(BL).insertion_prefix}-MO-SERVC-01:" | |
| ) | |
| @device_factory() | |
| def id() -> UndulatorInMm: | |
| return UndulatorInMm( | |
| prefix=f"{BeamlinePrefix(BL).insertion_prefix}-MO-SERVC-01:" | |
| ) |
Remove the name, let the factory auto populate it with the function name
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.
Sorry, also you should just do this for the prefix
| @device_factory() | |
| def id() -> UndulatorInMm: | |
| return UndulatorInMm( | |
| name="id", prefix=f"{BeamlinePrefix(BL).insertion_prefix}-MO-SERVC-01:" | |
| ) | |
| @device_factory() | |
| def id() -> UndulatorInMm: | |
| return UndulatorInMm( | |
| prefix=f"{PREFIX.insertion_prefix}-MO-SERVC-01:" | |
| ) |
oliwenmandiamond
left a comment
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.
Thanks, just need to remove the names and then all good
|
I have been looking into the energy calculations and think this should be merged as is and we can add that later on as a separate PR as there are some unanswered questions about how it's going to work. |
Fixes DiamondLightSource/sm-bluesky#126
This adds the undulator for I16, setting the gap in mm. It does not currently include the energy calculations or lookup table, these are performed in GDA at the moment and will be implemented in bluesky in a later change.
Instructions to reviewer on how to test:
run dodal connect i16 and check the id is in the list
Checks for reviewer
dodal connect ${BEAMLINE}