Add load and save plans for spectroscopy detector #19
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 65 67 +2
=========================================
+ Hits 65 67 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/test_rig_bluesky/plans.py
Outdated
| description=f"Sum of {roi.name} channel", | ||
| ) | ||
| ] | ||
| # ) |
There was a problem hiding this comment.
The NDAttribute setup should stay in code. We don't want to have to edit the xml by hand to update it.
There was a problem hiding this comment.
If the ROIs are consistent, would the xml not also be consistent? I understand at the moment it's only saved for red, but I can easily re-generate the .yaml once #18 has been merged
src/test_rig_bluesky/plans.py
Outdated
| yield from load_settings(spectroscopy_detector, yaml_directory, yaml_filename) | ||
|
|
||
| for motor in [sample_stage.x, sample_stage.y]: | ||
| yield from bps.mv( |
There was a problem hiding this comment.
These motor settings should be included
There was a problem hiding this comment.
If we want to include motor settings that will be a separate settings yaml - not a problem but will involve expanding the issue to encompass saving/loading baseline states of devices in general, rather than just the spectroscopy detector
There was a problem hiding this comment.
Yeah that seems reasonable
There was a problem hiding this comment.
I might suggest handling the panda separately, since ophyd-async does
There was a problem hiding this comment.
Do you mean handle the motors separately?
There was a problem hiding this comment.
No I mean the panda, but looking more closely ophyd-async only special cases it on load, not save https://github.com/bluesky/ophyd-async/blob/main/src/ophyd_async/plan_stubs/__init__.py.
src/test_rig_bluesky/plans.py
Outdated
| yield from load_settings(spectroscopy_detector, yaml_directory, yaml_filename) | ||
|
|
||
| for motor in [sample_stage.x, sample_stage.y]: | ||
| yield from bps.mv( |
There was a problem hiding this comment.
I might suggest handling the panda separately, since ophyd-async does
tests/unit_tests/test_plans.py
Outdated
| callback_on_mock_put(detector.driver.acquire, on_acquire) | ||
|
|
||
|
|
||
| def test_save_settings(RE: RunEngine): |
There was a problem hiding this comment.
Could: Assert that the file appears in /tmp
tests/unit_tests/test_plans.py
Outdated
| ) | ||
|
|
||
|
|
||
| def test_load_settings(RE: RunEngine): |
There was a problem hiding this comment.
Could: Assert that mock signal values now match the settings
| roistat.enable_callbacks: Enable | ||
| roistat.nd_array_address: 0 | ||
| roistat.nd_array_port: D2.CAM | ||
| roistat.nd_attributes_file: <Attributes><Attribute name="<ophyd_async.core._signal.SignalRW |
There was a problem hiding this comment.
Ah, this doesn't look right. Presumably this doesn't resolve itself when loading and it sends this as the actual value to the PV?
There was a problem hiding this comment.
Ah, presumably that's because of how I tried to pull the ROI names out - is there a way to interrogate these signals easily? I seem to be having trouble trying to read the signals in a local RE...
There was a problem hiding this comment.
I am not sure but I don't think it is a problem worth solving because we don't want this xml in the save file anyway. Or at least we don't want to load it.
| def save_settings( | ||
| device: Device, | ||
| design_name: str, | ||
| design_directory: str = os.path.abspath("./src/test_rig_bluesky/"), |
There was a problem hiding this comment.
I'm still not thrilled about letting the user configure this, makes it harder for us to swap it out for a database in the future. I won't block merge over it though.
There was a problem hiding this comment.
I added it back in to allow the settings for testing to be saved in the testing directory. I'm sure there's a better way to do it but I'm not sure what that way is
callumforrester
left a comment
callumforrester
left a comment
There was a problem hiding this comment.
Blocking pending conflict resolution and commit tidying
src/test_rig_bluesky/plans.py
Outdated
| wait=True, | ||
| ) | ||
| channel_name = yield from bps.rd(roistatn.name_) | ||
| print("channel_name") |
EmsArnold
force-pushed
the
load_save_plans
branch
from
d5ec1fc to
643f2f5
Compare
EmsArnold
force-pushed
the
load_save_plans
branch
from
643f2f5 to
85b23d6
Compare
Adds load and save plans for spectroscopy detector, with unit tests.