-
Notifications
You must be signed in to change notification settings - Fork 6
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
Added back the shutter selector to the voltmeters display. #323
Conversation
1e7a477
to
18a004c
Compare
Ready to be tested at the beamline. |
for idx in range(self.ui.shutter_combobox.count()): | ||
self.ui.shutter_combobox.removeItem(idx) | ||
# Add the shutters to the shutter combobox | ||
shutters = registry.findall("shutters", allow_none=True) |
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 would like explicitly remove 'front end shutter' from the list because it happens very often that ppl accidentally choose front end shutter.
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 agree that the FES should not be available.
Instead of hard-coding the name "front_end_shutter", though, I used the existing allow_close
and allow_open
attributes on the shutter devices. The shutter will only be present if both are not False
. A corollary is that shutters without those attributes will still be allowed.
src/firefly/tests/test_voltmeters.py
Outdated
assert checkbox.isEnabled() | ||
# Check that shutters were added to the combobox | ||
combobox_items = [combobox.itemText(idx) for idx in range(combobox.count())] | ||
assert "front_end_shutter" in combobox_items |
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.
If we decided to remove front end shutter, the test should be modified too
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.
Done.
Things to do before merging:
write docsupdate iconfig_testing.toml