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

Hyperion 1169 Attempted fix for failure re-arming the detector after beam dump #410

Merged

Conversation

rtuck99
Copy link
Contributor

@rtuck99 rtuck99 commented Apr 3, 2024

Fixes DiamondLightSource/mx-bluesky#282

Instructions to reviewer on how to test:

  1. To reproduce the issue without the fix, set dodal.devices.eiger.TEST_1169_INJECT = True, and dodal.devices.eiger.TEST_1169_FIX = False, this will cause the stage to be re-attempted, it should fail before it reaches the cam acquire and fan wait
  2. To test whether the fix works, leave the values at their defaults - it should now successfully re-arm the detector

If the tests pass we can tidy the code.

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

Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.79%. Comparing base (ccb4e81) to head (2956593).

❗ Current head 2956593 differs from pull request most recent head fa73693. Consider uploading reports for the commit fa73693 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     DiamondLightSource/hyperion#410   +/-   ##
=======================================
  Coverage   92.78%   92.79%           
=======================================
  Files          99       99           
  Lines        3769     3773    +4     
=======================================
+ Hits         3497     3501    +4     
  Misses        272      272           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Looks sensible, let's go down to test together at some point. Could you fix the mypy error?

Base automatically changed from hyperion_1167_detector_not_cleaned_after_smargon_fail to main April 9, 2024 15:43
@rtuck99 rtuck99 force-pushed the hyperion_1169_error_in_arming_the_detector_after_beam_dump branch from 8881620 to 0714aa7 Compare April 12, 2024 12:43
@DominicOram
Copy link
Contributor

Merge this. @rtuck99 and @DominicOram to test tomorrow.

@rtuck99 rtuck99 merged commit 97fd3d7 into main May 15, 2024
9 of 11 checks passed
@rtuck99 rtuck99 deleted the hyperion_1169_error_in_arming_the_detector_after_beam_dump branch May 15, 2024 15:32
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.

Error in arming the detector after beam dump
3 participants