Update electron analyser to use single detector rather than mutlple

[oliwenmandiamond]

With the release of Bluesky 1.15.0 https://github.com/bluesky/bluesky/releases, we can now remove dynamically creating new electron analyser detectors (1 per region) to force the run engine to save new configuration. Details found here and now fixed bluesky/bluesky#1899. Instead, we can just use a single detector now and the run_engine will correctly save the configuration for each new stream for each region.

Related sm-bluesky change DiamondLightSource/sm-bluesky#245.

Instructions to reviewer on how to test:

1. Check tests make sense
2. Check tests pass

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}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.11%. Comparing base (0beeb31) to head (58f7670).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2028   +/-   ##
=======================================
  Coverage   99.11%   99.11%           
=======================================
  Files         327      327           
  Lines       12814    12814           
=======================================
  Hits        12701    12701           
  Misses        113      113           

☔ 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.

[Relm-Arrowny]

I really like how little change in bluesky make this so much cleaner. Some minor point otherwise, it almost looked cool.

[Relm-Arrowny]

Should we catch staging empty data?

Suggested change

	pass
	if self.data is None:
	raise RuntimeError(
	"Bah bah bah"
	)

[oliwenmandiamond]

I think the answer is no. So we have two ways to use the electron analyser.

We use an analyserscan plan. This takes the sequence data that holds a list of regions as an argument and configures the analyser with it. At every point, we do an additional loop for each region configured via the sequence data on the analyser (will cover Specs and VGScienta case). This is also why the sequence needs to be attached to the analyser and this is the only way to get a reference to it when inside the scan engine at each step using a custom per_step in a normal scan:

>>> bc.plans.analyserscan(bc.devices.analyser, load_sequence("/path/to/data"), ...) 
# load_sequence returns a data structure which is then serialisable with blueapi and passed to the plan to configure the analyser with

We use a standard scan. The user can configure the detector first by setting a single region. (Will cover Mbs case)

>>> r = load_sequence("/path/to/data").get("My Region")

or

>>> r = load_region("path/to/data")

And then configure before scan

>>> bc.plans.move(bc.devices.analyser, r)
>>> bc.plans.scan([bc.devices.analyser], ...)

So if we try to do scan 2, then it will raise an error as we didn't configure a sequence. So the check we need to do is inside analyserscan plan which is done here.

@Villtord thoughts?

[Relm-Arrowny]

I think both would benefit by having the earlier fail. as it fails on stage which happens before we do anything else in a scan?

[Villtord]

Can we convert single region that is configured via bc.plans.move(bc.devices.analyser, r) into a some kind of default sequence internally? perhaps it could be a more unified approach, would be great if we can have one logic
If we do that then we can use this suggestion of Raymond.

[Villtord]

Is it actually true that stage is called later than analysercan checks if sequence is none? I thought analyserscan first checks sequence and then pass it on normal nd_scan which makes staging .

[Relm-Arrowny]

analysercan only call prepare and scan and I think the check comes in when per_step is call during a scan, that normally happen after stage and run is called. e.g. scan

[Villtord]

agreed

[Relm-Arrowny]

Not sure this is a wrapper, Stateholder, dataclass maybe?

[oliwenmandiamond]

So this is the debate I've kinda had with myself, do I make it Bluesky like and wrap it in an object to properly interact with it in plans, or do i just make it an internal state that I set inside the plan

and in plans instead of doing

yield from bps.prepare(analyser.sequence, sequence)

I just do

analyser.sequence = sequence

I guess an argument to just remove this wrapper/state holder/dataclass or whatever it is that the user should never try to prepare the device themselves because it can only be used with analyserscan plans which take the sequence object as an argument anyway to setup the analyser.

I'm kinda leaning towards just removing the wrapper entirely and just do option 2

[Relm-Arrowny]

I probably missing something can't we just make analyser prepareable and do both of what you trying to do here?

yield from bps.prepare(analyser, sequence)
and
analyser.sequence = sequence

something like this?

async def prepare(self, value:):
    self.sequence = value

async def stage(self) -> None:
    if self.sequence is None:
        raise bah
    await self._controller.disarm()

async def unstage(self) -> None:
    await self._controller.disarm()
    self._sequence = None

With this the other comment is moot, it will fail as soon as we try to run a scan without having regions defined.

[Villtord]

If your last code is for analyser object then it won't work as analyser can't cycle regions through the sequence and start scan for each region - currently it can only do so for 1 region hence the need for custom scan plan - i.e. anayserscan
Another question is maybe we can also convert single region into a sequence such that even single region can be used in analyserscan plan?

[Relm-Arrowny]

As mention in person, if you want the keep detector region state you can ignore the stage comment otherwise looks good to me.