Unify p38 and i22, swap out with env variables

[stan-dot]

Fixes #502

Instructions to reviewer on how to test:

1. Try on i22, try on i38
2. Confirm the devices are differnent yet experiments run correctly

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

Attention: Patch coverage is 95.45455% with 2 lines in your changes missing coverage. Please review.

Project coverage is 94.34%. Comparing base (b39f55c) to head (d0f2612).
Report is 2 commits behind head on main.

Files	Patch %	Lines
src/dodal/beamlines/i22.py	89.47%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #678      +/-   ##
==========================================
- Coverage   94.44%   94.34%   -0.10%     
==========================================
  Files         114      115       +1     
  Lines        4588     4561      -27     
==========================================
- Hits         4333     4303      -30     
- Misses        255      258       +3     

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

[stan-dot]

/workspaces/dodal/tests/preprocessors/test_filesystem_metadata.py:76:9 - error: Method "trigger" overrides class "Triggerable" in an incompatible manner
Return type mismatch: base method returns type "Status", override returns type "Status"
"Status" is incompatible with protocol "Status"
"exception" is an incompatible type
Type "(timeout: Unknown | None = None) -> (StatusTimeoutError | Exception | type[Exception] | None)" is incompatible with type "(timeout: float | None = 0) -> (BaseException | None)"
Function return type "StatusTimeoutError | Exception | type[Exception] | None" is incompatible with type "BaseException | None"
"bluesky.protocols.Status" is incompatible with "ophyd.status.Status"
"bluesky.protocols.Status" is incompatible with "ophyd.status.Status" (reportIncompatibleMethodOverride)
136 errors, 3 warnings, 0 informations
ERROR: InvocationError for command /venv/bin/pyright src tests (exited with code 1)

type checking fails due to dodal not catching up to ophyd-async changes

[stan-dot]

codecov for live beamline connectivity does not make that much sense, please let it merge

[stan-dot]

I need this for #673

[stan-dot]

from @DiamondJoseph

parameterised in such a way that you can either mock the IS_LAB call before the device_instantiation is called or otherwise export BEAMLINE=P38, then you should be able to traverse that alternate path

using https://github.com/DiamondLightSource/dodal/blob/502-automatically-keep-i22-and-p38-in-sync/tests/common/beamlines/test_device_instantiation.py

[stan-dot]

@coretl would an addition of a 'lab' flag be in the scope for the device instantiation factory decorator?

[stan-dot]

need to delete LAB_NAME from the CLI, and make it all optimized for testability

[DiamondJoseph]

split devices like saxs

[stan-dot]

???I do not understand

[callumforrester]

if IS_LAB:
    return this_device()
else:
    return that_device()

[stan-dot]

I do not agree that it's more readable, but I won't argue, changing now

[stan-dot]

done

[callumforrester]

You've done

if IS_LAB:
    return this_device()
return that_device()

Which is functionally the same, but harder to read.

[stan-dot]

Which is functionally the same, but harder to read.

that's a purely stylistic opinion, usefulness of else is a debated point among software engineers

[stan-dot]

at the core the still not-agreed-on-implementation detail is where to put the i22-p38 mapping?

when we read in the env in the CLI we need to check if the BEAMLINE belongs to the lab set and then load the dodal.beamlines.non-lab-beamline. a dictionary (this time lab: real) sounds like the most Pythonic way to do this and this can easily be hard-coded for the future.
request for comments @callumforrester , @DiamondJoseph

[DiamondJoseph]

when we read in the env in the CLI we need to check if the BEAMLINE belongs to the lab set ...

Which CLI, dodal's?

What I would expect is if you call dodal connect i22, you run i22.py and try and connect to the devices. If the env var $BEAMLINE is set to p38, that gets picked up by BL = get_beamline_name("i22") and mostly you connect to mocked devices. You never try and run dodal connect p38

[stan-dot]

ok fair rnough

[stan-dot]

@callumforrester please weigh in, me and @DiamondJoseph got stuck

update: this was confirmed as desired behavior:
What I would expect is if you call dodal connect i22, you run i22.py and try and connect to the devices. If the env var $BEAMLINE is set to p38, that gets picked up by BL = get_beamline_name("i22") and mostly you connect to mocked devices. You never try and run dodal connect p38

[stan-dot]

this might need a re-work once #483 is merged