-
Notifications
You must be signed in to change notification settings - Fork 0
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
Some reformatting of eiger_detector.py #19
base: master
Are you sure you want to change the base?
Conversation
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've had a quick look and this seems to generally improve things.
@ajgdls could you give a review as well?
Have made that change, the 4 mbbi records in eiger_options_config no longer cast the mbbi index to string unnecessarily. |
Sorry maybe I wasn't clear. We do need the cast from int to string because the detector only accepts string (I think?). With eiger-fastcs, this might be less important, though DAQ have requested that we still use mbb records because they automatically get treated as enums in ophyd. |
I can look over it again but I believe it doesn't affect what gets sent to the detector, just how python is internally handling the tracking of the state. I believe the int gets cast to a string which is handled by set_value, which casts it back to an int when get_option is called and is replaced by the string at that index in the EigerOption options list. |
replace unnecessary getattrs with normal accessors
1c53c08
to
33fb5a0
Compare
I've checked through this and I can't see why the string values need to be stored, they are indeed converted to integer indexes and then the "real" string value written down to the hardware. |
@jsouter have you been able to test this with a detector? It might be worth adding a couple of debug statements that could be turned on to show what messages are going to the hardware (if that debugging is not already in there somewhere). |
I've only tested this against the tickit-devices sim I believe, while that appears to work I can give it a try with a detector soon. |
Small changes, namely removing unnecessary use of getattr/setattr when the attr name is given, replacing some format strings with f strings and naming references to the param tree subtrees to shorten some of the horribly long lines.