-
Notifications
You must be signed in to change notification settings - Fork 8
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
add eiger timeouts availability #806
Conversation
6cfa033
to
5422a2a
Compare
5422a2a
to
74e712d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #806 +/- ##
=======================================
Coverage 95.63% 95.64%
=======================================
Files 128 128
Lines 5271 5279 +8
=======================================
+ Hits 5041 5049 +8
Misses 230 230 ☔ View full report in Codecov by Sentry. |
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, looks good. Just one comment
src/dodal/devices/eiger.py
Outdated
if beamline in AVAILABLE_TIMEOUTS: | ||
self.timeouts = AVAILABLE_TIMEOUTS[beamline] | ||
else: | ||
raise ValueError(f"Unknown beamline_name: {beamline}") |
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.
Could: I think it's better to use i03's timeout values as defaults rather than raising an error here. Then we could log a message to say that default values are being used
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.
Please could you also add a test for this line
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.
ok will do today
Fixes #256
Instructions to reviewer on how to test:
Checks for reviewer
dodal connect ${BEAMLINE}