Skip to content
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 ability to enable dev shm streaming on eiger #560

Merged
merged 5 commits into from
May 22, 2024

Conversation

DominicOram
Copy link
Contributor

See DiamondLightSource/hyperion#1372

Instructions to reviewer on how to test:

  1. Confirm tests pass

Checks for reviewer

  • Would the PR title make sense to a scientist on a set of release notes
  • If a new device has been added does it follow the standards
  • If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly

Copy link
Collaborator

@olliesilvester olliesilvester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit, but looks good

@@ -144,6 +144,9 @@ def stop(self, *args):
self.disarm_detector()
stop_status &= self.disable_roi_mode()
stop_status.wait(self.GENERAL_STATUS_TIMEOUT)
# See https://github.com/DiamondLightSource/hyperion/issues/1395
LOGGER.info("Turning off Eiger dev/shm streaming")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: If we have enable_dev_shm set to off, I think we will get a logging message during change_dev_shm, and in Hyperion when XRC finishes, and again here - all saying that dev shm is being disabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that given the consequence of leaving it on accidentally is very bad it's actually good to be very specific in the logs that we're turning it off even if that doesn't actually matter

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, makes sense

@DominicOram DominicOram merged commit c1e5d88 into main May 22, 2024
11 checks passed
@DominicOram DominicOram deleted the hyperion_1372_enable_dev_shm branch May 22, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants