Add i06-1 magnets

[oliwenmandiamond]

Fixes #2120

Instructions to reviewer on how to test:

1. Check implementations make sense
2. Check tests coverage is sufficient
3. Check doc strings for devices make sense
4. Check dodal connect i06-1

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

❌ Patch coverage is 99.73753% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.17%. Comparing base (c5a4fc3) to head (ccc8d74).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...c/dodal/devices/beamlines/i06_1/magnet/movement.py	98.98%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2115      +/-   ##
==========================================
+ Coverage   99.16%   99.17%   +0.01%     
==========================================
  Files         354      359       +5     
  Lines       13848    14211     +363     
==========================================
+ Hits        13732    14094     +362     
- Misses        116      117       +1     

☔ View full report in Codecov by Harness.
📢 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 gone through it roughly, I have a few questions

[Relm-Arrowny]

The core logic for movement strategies and mode boundary enforcement looks great! I still feel we should make MagnetAxis a pure StandardReadable so we do not bypass the controller's safety logic and eliminates the need to inject mag_within_boundary callbacks into MagnetAxis.

I know you might not be fully convinced t about keeping MagnetAxis read-only and I probably did not explain it well. If you're busy or would prefer, I'm happy to take a stab at the branch to refactor MagnetAxis, and the controller to show you what I had in mind!

[Relm-Arrowny]

Should: Could we make a ticket and add to this comment to revisit this, the negative first moving restriction is not in the manual and we tried single step move and asked Dirk about it and we believe it should be safe to move everything at once.

Suggested change

	# Do all decreasing of axes first as one step. This ensures we don't
	# quench the magnet.
	# Do all decreasing of axes first as one step. This ensures we don't
	# quench the magnet.
	# To do optimise magnet movement #issue.

[Relm-Arrowny]

could/maybe: There is no real reason to check the other inactive axis, since you can only get here if you are in the correct mode so we could just.

@dataclass
class UniaxialMovement(MovementStrategy):
    mode: MagnetMode
    limit: float

    def check_within_limits(
        self, current_readback: MagnetPosition, target: MagnetRequest
    ) -> None:
        axis = self.mode.axis_alias
        resolved_value = getattr(target.resolve_pos(current_readback), axis)

        if resolved_value is not None and abs(resolved_value) > self.limit:
            raise MagnetPositionError.axis_outside_limit(
                self.mode, self.limit, resolved_value, axis
            )

    def move_steps(
        self, current_readback: MagnetPosition, target: MagnetRequest
    ) -> list[MagnetRequest]:
        axis = self.mode.axis_alias
        return [MagnetRequest(**{axis: getattr(target, axis)})]

[oliwenmandiamond]

Same as above

[Relm-Arrowny]

Must: Missing checks to enforce positive quadrant boundaries . We should check x >= 0 and y >= 0 before allowing the move, we should also check H < 2.0T. If we do not do speed matching we must also do decrease first to stop it going beyond 2.0T during move.

[Relm-Arrowny]

Should: Axis_limit belong to MagnetAxis.

[oliwenmandiamond]

In epics it belongs to the power supply and is reflected with the PV. My goal is to model how it appears in epics / hardware so it refeclts it as much as possible. We shouldn't be providing two completely different PV's to a class. We should create two classes that resemble the PV structure and then combine them together with a composite device of the two classes.

Also as it is not used anywhere in the code, I will just remove it. If I did add it back, I would change this to be a power supply class which has the ramp rate class inside and then the additional pv signals such as the limit

[Relm-Arrowny]

Should: I still think there is no good reason to make MagnetAxis moveable, as we should never allow anyone to move an axis bypass the controller.

If we can do this:

yield from bps.scan(
    [detector], 
    scmc.cart, 
    MagnetRequest(x=0.0), 
    MagnetRequest(x=5.0), 
    num=100
)

Then I think every uniaxisal move should go via MagnetCartesianCoordinates,
otherwise an uniaxial device is probably better:

class MagnetUniaxialAxis(
    StandardReadable, StandardMovable[float], Flyable, Preparable
):
    def __init__(self, controller: "SuperConductingMagnetController", name: str = ""):
        self._controller = controller
   @AsyncStatus.wrap
    async def set(self, value: float):
        mode = await self._controller.mode.get_value()
        if mode not in (MagnetMode.UNIAXIAL_X, MagnetMode.UNIAXIAL_Y, MagnetMode.UNIAXIAL_Z):
            raise RuntimeError("no")
        axis = mode.axis_alias  
        await self._controller.set_within_boundary(MagnetRequest(**{axis: value}))

[Relm-Arrowny]

we should make these private and no longer need to pass mag_within_boundary into the axis if we accepte axis should just be device readable.

[Relm-Arrowny]

Should: flyable should be move to the controller or it's subclass so mode is check before we try to fly we do not want to try to fly x when in y mode:

@AsyncStatus.wrap
async def prepare(self, value: FlyMagnetInfo):
    mode = await self.mode.get_value()
    check what is our current mode
    do the prepare logic depend on mode.