-
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
Update Eiger API #111
base: main
Are you sure you want to change the base?
Update Eiger API #111
Conversation
GDYendell
commented
Aug 23, 2024
•
edited
Loading
edited
- Depends on Fix eiger stream #104
9c25907
to
4664a7f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #111 +/- ##
==========================================
+ Coverage 94.32% 94.74% +0.42%
==========================================
Files 34 34
Lines 1620 1751 +131
==========================================
+ Hits 1528 1659 +131
Misses 92 92 ☔ View full report in Codecov by Sentry. |
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 like you need to up the coverage, but looks good so far.
9118017
to
00b424c
Compare
I have just pushed this branch and it hasn't updated the PR or triggered the CI... |
c750000
to
4db4ceb
Compare
Fixed the CI 🔨 Eiger coverage is now 100% |
016d5de
to
262479b
Compare
Except for allowed values of element, which seem to be completely different and I don't have the energies to input for each element. Note some of these changes apply to the old firmware.
Currently the sim does not implement ntrigger correctly and always only does one trigger. This updates the logic to produce a series with nimage images, repeating ntrigger times, as the real detector does.
Arguably this is a bug, but it is what the real detector does.
262479b
to
76c6b76
Compare
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.
LGTM
Did you have any more comments on this @abbiemery ? |