Skip to content

Add roistat plugin to ViSR spectroscopy detector#1390

Merged
GDYendell merged 1 commit intomainfrom
visr-roistat
Aug 22, 2025
Merged

Add roistat plugin to ViSR spectroscopy detector#1390
GDYendell merged 1 commit intomainfrom
visr-roistat

Conversation

@GDYendell
Copy link
Contributor

@GDYendell GDYendell commented Jul 25, 2025

Instructions to reviewer on how to test:

  1. On b01-1-ws001 do
In [1]: from bluesky import RunEngine

In [2]: RE = RunEngine()

In [3]: from dodal.beamlines.b01_1 import spectroscopy_detector

In [4]: s = spectroscopy_detector(connect_immediately=True)

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
  • Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

@GDYendell GDYendell force-pushed the visr-roistat branch 2 times, most recently from dc00b44 to c3cbe40 Compare August 8, 2025 13:20
@GDYendell GDYendell marked this pull request as ready for review August 8, 2025 13:21
@GDYendell GDYendell requested a review from a team as a code owner August 8, 2025 13:21
@@ -1,4 +1,5 @@
from ophyd_async.epics.adaravis import AravisDetector
from ophyd_async.epics.adcore import NDROIStatIO
from ophyd_async.fastcs.panda import HDFPanda
Copy link
Contributor

@rtuck99 rtuck99 Aug 8, 2025

Choose a reason for hiding this comment

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

Since this adds a dependency on a new device to dodal, you should bump the ophyd-async dependency version in pyproject.toml

Copy link
Contributor

@rtuck99 rtuck99 left a comment

Choose a reason for hiding this comment

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

Approved, however I note that if I try to do dodal connect b01_1 on a b01-1-ws001 I get PATH_PROVIDER is not defined, so I assume there is some additional setup required?

Ideally it would be good if dodal connect works without modificatons, other beamlines set a default in the beamline module.

@callumforrester
Copy link
Contributor

callumforrester commented Aug 11, 2025

@rtuck99 This may have been an error on my part or it may require #1129, I will investigate.

Edit: It would seem, and I did not realise this, that b01-1 is the first beamline we've got fully using numtracker, so we're encountering all the teething problems. Blueapi's numtracker client refuses to work with anything but it's own path provider, #1129 is an issue for resolving this, so I will see if I can push it forward.

@GDYendell
Copy link
Contributor Author

Updated on main, which has bumped the ophyd-async dependency, so I think this is good to merge now.

@codecov
Copy link

codecov bot commented Aug 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.73%. Comparing base (aac8fda) to head (10e09d2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1390   +/-   ##
=======================================
  Coverage   98.73%   98.73%           
=======================================
  Files         232      232           
  Lines        8592     8594    +2     
=======================================
+ Hits         8483     8485    +2     
  Misses        109      109           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@GDYendell GDYendell merged commit 46a11a6 into main Aug 22, 2025
11 checks passed
@GDYendell GDYendell deleted the visr-roistat branch August 22, 2025 11:23
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.

3 participants