Add i22 beamline and devices

[DiamondJoseph]

Rebased and refactored i22 beamline
Requires #327, closes #305

Instructions to reviewer on how to test:

1. On machine day run on i22 control machine
2. Confirm devices are instantiated and operable

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

[DiamondJoseph]

Failing only because of the i24 failing test #329 : currently modus operandi seems to be merging regardless if the change is otherwise sensible and well behaved.

[Tom-Willemsen]

Perhaps this is a slightly wider discussion, but should we be naming the device or the scientific function of the device in dodal?

For the detectors we've called them waxs and saxs - i.e. the scientific function of the device (as opposed to, say, pilatus1 and pilatus2).

So should these tetramms be incident_beam_monitor and transmitted_beam_monitor, or user_tetramm for example? (I'm assuming that's what i0 and it mean, but could be wrong...)

[DiamondJoseph]

There's definitely a discussion to be had: I think it comes down to beamline scientist/support engineer preference on "how easy is this to type" vs. "how completely does this describe the function of the device" vs. "how completely does this describe the form of the device" (I think "how easy to type" is going to get a lot of traction...).

Do you have preference? I think incident_beam and transmitted_beam, with sufficient docstrings on the type and that it doesn't implement Movable makes it clear that it is a Monitor/Sensor.

[Tom-Willemsen]

Sure to be honest I don't have a particularly strong opinion on the actual chosen names, it just jumped out to me as inconsistent with the saxs/waxs detectors. Happy for you to spin this off into an issue or discussion or strawpoll on slack or something if you prefer...

[DiamondJoseph]

Made an issue, but also renamed for i22 to something I think is sensible. Will return to p38 when can discuss with p38 interested parties.

[Tom-Willemsen]

For your info I asked i22 scientists about what naming they'd generally prefer in a meeting this morning - their general preference is for scientific meaning rather than device name. E.g. they suggested i0 and it are preferred over tetramm1 etc.

[Tom-Willemsen]

What do we want to do about this commented-out code? Remove or uncomment?

[DiamondJoseph]

@coretl think this may be one for you- looks like the initial commit had line_2 commented out, then it was restored and action_0, action_1 were commented out.

If this is the relevant Mako driver, it doesn't have action, but the GigE camera has this and more.

[coretl]

The problem is that this is camera firmware dependent. I think we need to find a way to make Enum strings be non-exhaustive, maybe in the signal backend converter we subclass the supplied Enum to add extra entries if it they exist? Then this code can just be fixed_rate and line_1 and the others can be added at runtime as we don't care what they are

[DiamondJoseph]

Once this is merged, we should be able to reduce to just the values that are being used.
bluesky/ophyd-async#147

Getting the signal may return values outside of the enum, but putting the limited enum should be ok?

[Tom-Willemsen]

Getting the signal may return values outside of the enum

I guess the signal should probably be type-hinted as str then - or at least something that gives an indication this isn't a standard enum... Otherwise this is bound to trip people up eventually.

[codecov]

Codecov Report

Attention: Patch coverage is 81.25000% with 51 lines in your changes are missing coverage. Please review.

Project coverage is 92.38%. Comparing base (350455f) to head (7e68da7).
Report is 88 commits behind head on main.

Files	Patch %	Lines
src/dodal/devices/linkam3.py	66.07%	19 Missing ⚠️
src/dodal/devices/tetramm.py	82.72%	19 Missing ⚠️
src/dodal/devices/areadetector/adaravis.py	77.77%	12 Missing ⚠️
src/dodal/devices/areadetector/pilatus.py	94.11%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #332      +/-   ##
==========================================
- Coverage   93.30%   92.38%   -0.93%     
==========================================
  Files          89       94       +5     
  Lines        3315     3583     +268     
==========================================
+ Hits         3093     3310     +217     
- Misses        222      273      +51     

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

[Tom-Willemsen]

I'm ok with merging this now and getting further feedback from i22 beamline tests.

[Tom-Willemsen]

Q: we have panda-01 and panda_01 here, is the mismatch intentional/ok?

[DiamondJoseph]

It shouldn't matter, but to be consistent I've renamed both to panda1, which is consistent with the mx beamlines.

[Tom-Willemsen]

Suggested change

	# FIXME: Currently disconnected
	# def panda3(
	# wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False
	# ) -> PandA:
	# return device_instantiation(
	# PandA, "panda3", "-EA-PANDA-02:", wait_for_connection, fake_with_ophyd_sim
	# FIXME: Currently disconnected
	# def panda3(
	# wait_for_connection: bool = True, fake_with_ophyd_sim: bool = False
	# ) -> PandA:
	# return device_instantiation(
	# PandA, "panda3", "-EA-PANDA-03:", wait_for_connection, fake_with_ophyd_sim

I presume?

[Tom-Willemsen]

Getting the signal may return values outside of the enum

I guess the signal should probably be type-hinted as str then - or at least something that gives an indication this isn't a standard enum... Otherwise this is bound to trip people up eventually.

[abbiemery]

Closed as these devices have all been merged seperately.