-
Notifications
You must be signed in to change notification settings - Fork 1
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
Create DS subcontroller for fp dataset parameters #37
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #37 +/- ##
==========================================
+ Coverage 82.30% 82.92% +0.61%
==========================================
Files 9 9
Lines 390 404 +14
==========================================
+ Hits 321 335 +14
Misses 69 69 ☔ View full report in Codecov by Sentry. |
src/odin_fastcs/odin_data.py
Outdated
for parameter in self._parameters: | ||
short_path = parameter.uri[3:] | ||
if short_path[0] == "compressed_size": | ||
short_path[0] = "compressed" # slightly shorter PV names |
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.
I think we should fix this by renaming the dataset to size
(in the fp config) as it is in the API (https://media.dectris.com/SIMPLON_APIReference_v1p8.pdf page 29) rather than hacking it here.
With DiamondLightSource/FastCS#56 it at least won't crash and it isn't the end of the world if these PVs aren't created, for now.
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.
Okay, have undone that change/made the necessary test changes
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.
Looks good. Could you fixup the extra commit into the original so that it is just one commit and then we can merge.
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 one comment.
e75a6ff
to
a9cb407
Compare
a9cb407
to
2832712
Compare
FP plugin controllers create a subcontroller named "DS" if any "dataset" parameters exist. Also shortens "compressed_size" to "compression" in the path for slightly shorter PV names, some more shortening may be necessary for longer device names.