Update spectroscopy plan to configure ROIs and motion settings #14

GDYendell · 2025-08-06T12:56:09Z

No description provided.

callumforrester

Looks good, mostly concurrency issues, I also think some more unit testing would be useful

callumforrester · 2025-08-06T15:19:24Z

tests/system_tests/test_plans_system.py

    assert events["FINISHED"][0]["scanDimensions"] == [5]
+
+
+def test_spectroscopy_re():


Could: My only objection to putting tests like this back in is that I can't run them from my laptop over eduroam/vpn, which is very convenient. However if you think they add more value than take I won't block over this.

I wrote this for two reasons:

Blueapi was not working - maybe that is less likely to be the case in future?

Passing the scanspec into the TaskRequest didn't seem to work. Do you have an example
of that?

Can have a look at passing the scanspec into the request, for this we could use something like pytest-requires and then you could have @pytest.mark.requires("beamline network") or similar

I suspect the reason the spec didn't work is because there is no type bound in the plan signature:

def spectroscopy( spectroscopy_detector: AravisDetector = spectroscopy_detector, sample_stage: XYZStage = sample_stage, - spec: Spec | None = None, + spec: Spec[Movable] | None = None, exposure_time: float = 0.1, metadata: dict[str, Any] | None = None, ) -> MsgGenerator[None]:

Specifying a bluesky type tells blueapi to resolve it as a special case (device or devices). I also made a PR on top of this PR that just skips tests like this if you're not on a beamline machine, up to you. https://github.com/DiamondLightSource/test-rig-bluesky/pull/17/files

I have pulled in the PR.

So if I add Spec[Movable] I can do something like this?

_sample_stage = sample_stage() scan_spec = Line(_sample_stage.x, 2, 5, 30) * Line(_sample_stage.y, 0, 5, 50) events = bluesky_plan_runner.run( TaskRequest( name="demo_spectroscopy", params={"spec": scan_spec.serialize()}, instrument_session=latest_commissioning_instrument_session, ), timeout=10, )

I can't test currently because rabbit mq is not running.

Yes, I believe that should work

callumforrester · 2025-08-06T15:23:05Z

src/test_rig_bluesky/plans.py

+        yield from bps.abs_set(roistatn.name_, roi.name)
+        yield from bps.abs_set(roistatn.min_x, roi.start_x)
+        yield from bps.abs_set(roistatn.min_y, roi.start_y)
+        yield from bps.abs_set(roistatn.size_x, roi.size)
+        yield from bps.abs_set(roistatn.size_y, roi.size)
+        yield from bps.abs_set(roistatn.use, True)


Must: These won't block until put complete so could lead to race conditions, you could instead:

Suggested change

yield from bps.abs_set(roistatn.name_, roi.name)

yield from bps.abs_set(roistatn.min_x, roi.start_x)

yield from bps.abs_set(roistatn.min_y, roi.start_y)

yield from bps.abs_set(roistatn.size_x, roi.size)

yield from bps.abs_set(roistatn.size_y, roi.size)

yield from bps.abs_set(roistatn.use, True)

yield from bps.abs_set(roistatn.name_, roi.name, group="setup")

yield from bps.abs_set(roistatn.min_x, roi.start_x, group="setup")

yield from bps.abs_set(roistatn.min_y, roi.start_y, group="setup")

yield from bps.abs_set(roistatn.size_x, roi.size, group="setup")

yield from bps.abs_set(roistatn.size_y, roi.size, group="setup")

yield from bps.abs_set(roistatn.use, True, group="setup")

# This line blocks until all puts are complete

yield from bps.wait(group="setup")

OR

Suggested change

yield from bps.abs_set(roistatn.name_, roi.name)

yield from bps.abs_set(roistatn.min_x, roi.start_x)

yield from bps.abs_set(roistatn.min_y, roi.start_y)

yield from bps.abs_set(roistatn.size_x, roi.size)

yield from bps.abs_set(roistatn.size_y, roi.size)

yield from bps.abs_set(roistatn.use, True)

bps.mv(roistatn.name_, roi.name,

bps.abs_set(roistatn.min_x, roi.start_x,

bps.abs_set(roistatn.min_y, roi.start_y,

bps.abs_set(roistatn.size_x, roi.size,

bps.abs_set(roistatn.size_y, roi.size,

bps.abs_set(roistatn.use, True,

wait=True,

)

callumforrester · 2025-08-06T15:23:23Z

src/test_rig_bluesky/plans.py

        spectroscopy_detector, TriggerInfo(livetime=exposure_time), wait=True
    )
+    # TODO: This would be nicer if NDArrayBaseIO had a PortName signal
+    yield from bps.abs_set(spectroscopy_detector.fileio.nd_array_port, "D2.roistat")


Must: This should block to avoid race conditions

Suggested change

yield from bps.abs_set(spectroscopy_detector.fileio.nd_array_port, "D2.roistat")

yield from bps.abs_set(spectroscopy_detector.fileio.nd_array_port, "D2.roistat", wait=True)

callumforrester · 2025-08-06T15:23:48Z

src/test_rig_bluesky/plans.py

+        yield from bps.abs_set(motor.acceleration_time, 0.001)
+        yield from bps.abs_set(motor.velocity, 100)


Must: See above for suggestions on making these concurrent and blocking

src/test_rig_bluesky/plans.py

GDYendell · 2025-08-07T15:21:57Z

~~Has something to with path provider changed upstream? It is failing on main with the new dependencies.~~

What do we have to change here to make the unittests work after removing the path provider?

callumforrester · 2025-08-08T09:13:21Z

@GDYendell #15

callumforrester · 2025-08-11T11:23:16Z

src/test_rig_bluesky/plans.py

+        yield from bps.mv(
+            bps.abs_set(roistatn.name_, roi.name),
+            bps.abs_set(roistatn.min_x, roi.start_x),
+            bps.abs_set(roistatn.min_y, roi.start_y),
+            bps.abs_set(roistatn.size_x, roi.size),
+            bps.abs_set(roistatn.size_y, roi.size),
+            bps.abs_set(roistatn.use, True),
+            wait=True,
+        )


Does this work? I thought it had to be:

Suggested change

yield from bps.mv(

bps.abs_set(roistatn.name_, roi.name),

bps.abs_set(roistatn.min_x, roi.start_x),

bps.abs_set(roistatn.min_y, roi.start_y),

bps.abs_set(roistatn.size_x, roi.size),

bps.abs_set(roistatn.size_y, roi.size),

bps.abs_set(roistatn.use, True),

wait=True,

)

yield from bps.mv(

roistatn.name_, roi.name,

roistatn.min_x, roi.start_x,

roistatn.min_y, roi.start_y,

roistatn.size_x, roi.size,

roistatn.size_y, roi.size,

roistatn.use, True,

wait=True,

)

Ah. No I didn't try running after this change. I have updated it, but I still haven't tested it because I don't want to clash with anyone.

I have now tested this, but only up to running spec_scan as we don't have GPFS currently.

Add rule to skip tests that commuicate directly with hardware via EPICS unless the tests are run from one of a list of approved hosts that is known to have access to the b01-1 control system

codecov · 2025-08-26T10:57:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (dd6f783) to head (6de0f4a).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main       #14   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           42        65   +23     
=========================================
+ Hits            42        65   +23

☔ 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 · 2025-08-26T11:02:49Z

Unit tests passing. System tests might not be.

callumforrester

LGTM! Thanks

GDYendell requested review from callumforrester and douglaswinter August 6, 2025 12:56

callumforrester suggested changes Aug 6, 2025

View reviewed changes

GDYendell force-pushed the spectroscopy-rois branch from f6619ff to f322789 Compare August 8, 2025 13:01

callumforrester reviewed Aug 11, 2025

View reviewed changes

GDYendell force-pushed the spectroscopy-rois branch 2 times, most recently from d073c9e to a8b65fd Compare August 14, 2025 15:13

GDYendell force-pushed the spectroscopy-rois branch from a8b65fd to b38e924 Compare August 26, 2025 09:56

GDYendell and others added 2 commits August 26, 2025 10:55

Update spectroscopy plan to configure ROIs and motion settings

99b664a

Skip RE tests when not on beamline network

6de0f4a

Add rule to skip tests that commuicate directly with hardware via EPICS unless the tests are run from one of a list of approved hosts that is known to have access to the b01-1 control system

GDYendell force-pushed the spectroscopy-rois branch from b38e924 to 6de0f4a Compare August 26, 2025 10:56

callumforrester approved these changes Aug 26, 2025

View reviewed changes

GDYendell merged commit 7ef2a03 into main Aug 26, 2025
8 checks passed

		assert events["FINISHED"][0]["scanDimensions"] == [5]


		def test_spectroscopy_re():

-        yield from bps.abs_set(roistatn.name_, roi.name)
-        yield from bps.abs_set(roistatn.min_x, roi.start_x)
-        yield from bps.abs_set(roistatn.min_y, roi.start_y)
-        yield from bps.abs_set(roistatn.size_x, roi.size)
-        yield from bps.abs_set(roistatn.size_y, roi.size)
-        yield from bps.abs_set(roistatn.use, True)
+        yield from bps.abs_set(roistatn.name_, roi.name, group="setup")
+        yield from bps.abs_set(roistatn.min_x, roi.start_x, group="setup")
+        yield from bps.abs_set(roistatn.min_y, roi.start_y, group="setup")
+        yield from bps.abs_set(roistatn.size_x, roi.size, group="setup")
+        yield from bps.abs_set(roistatn.size_y, roi.size, group="setup")
+        yield from bps.abs_set(roistatn.use, True, group="setup")
+        # This line blocks until all puts are complete
+        yield from bps.wait(group="setup")

	yield from bps.abs_set(spectroscopy_detector.fileio.nd_array_port, "D2.roistat")
	yield from bps.abs_set(spectroscopy_detector.fileio.nd_array_port, "D2.roistat", wait=True)

		yield from bps.abs_set(motor.acceleration_time, 0.001)
		yield from bps.abs_set(motor.velocity, 100)

Update spectroscopy plan to configure ROIs and motion settings #14

Update spectroscopy plan to configure ROIs and motion settings #14

Uh oh!

Conversation

GDYendell commented Aug 6, 2025

Uh oh!

callumforrester left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GDYendell Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

callumforrester Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GDYendell commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

callumforrester commented Aug 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

GDYendell commented Aug 26, 2025

Uh oh!

callumforrester left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GDYendell Aug 6, 2025 •

edited

Loading

callumforrester Aug 15, 2025 •

edited

Loading

GDYendell commented Aug 7, 2025 •

edited

Loading

codecov bot commented Aug 26, 2025 •

edited

Loading