Hyperion supervisor implementation

[rtuck99]

Fixes:

• Hyperion blueapi supervisor service #1365

Requires:

• Preparatory work for Hyperion supervisor: Hyperion planrunner refactor #1531
• Ability to launch blueapi and run Hyperion plans #1389
• Changes to support hyperion-supervisor dodal#1804

Instructions to reviewer on how to test:

1. Do thing x
2. Confirm thing y happens

Checks for reviewer

• Would the PR title make sense to a user on a set of release notes

[codecov]

Codecov Report

❌ Patch coverage is 99.15966% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.45%. Comparing base (6e45f38) to head (d0e35d7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1530      +/-   ##
==========================================
+ Coverage   92.29%   92.45%   +0.15%     
==========================================
  Files         144      146       +2     
  Lines        8167     8272     +105     
==========================================
+ Hits         7538     7648     +110     
+ Misses        629      624       -5     

Components	Coverage Δ
i24 SSX	78.56% <ø> (ø)
hyperion	98.18% <99.15%> (+0.28%)	⬆️
other	98.00% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jacob720]

Looks good, a couple of comments but happy to merge if you disagree

[jacob720]

Could : Update this docstring to include the watchdog_port

[jacob720]

It strikes me that currently if we add a plan that agamemnon can request, we'd have to make changes in 3 places:

• SupvervisorRunner.decode_and_execute
• InProcessRunner.decode_and_execute
• create_parameters_from_agamemnon

Could we push some of this logic into new classes that extend or contain the parameter models?

Something like

class ExecutablePlan:
    @abstractmethod
    def execute_plan(self, context: BlueskyContext) -> MsgGenerator: ...

    @abstractmethod
    def get_task_request(self, instrument_session) -> TaskRequest: ...


class TaskRequestAndExecuteFromLoadCentreCollect(ExecutablePlan):
    def __init__(self, parameters: LoadCentreCollect):
        self.parameters = parameters

    def get_task_request(self, instrument_session):
        return TaskRequest(
            name="load_centre_collect",
            params={"parameters": self.parameters},
            instrument_session=instrument_session,
        )

    def execute_plan(self, context: BlueskyContext):
        devices: Any = create_devices(context)
        yield from self.execute_plan(
            partial(load_centre_collect_full, devices, self.parameters)
        )
        return self.parameters.visit


class TaskRequestFromUDCDefaultState(ExecutablePlan):
    def get_task_request(self, instrument_session):
        return TaskRequest(
            name="load_centre_collect",
            params={},
            instrument_session=instrument_session,
        )

    def execute_plan(self, context: BlueskyContext):
        udc_default_devices: UDCDefaultDevices = device_composite_from_context(
            context, UDCDefaultDevices
        )
        yield from move_to_udc_default_state(udc_default_devices)

etc.
Then we could get rid of the switch statements in SupvervisorRunner.decode_and_execute and InProcessRunner.decode_and_execute and instead have

def decode_and_execute(
    self, current_visit: str | None, parameter_list: Sequence[ExecutablePlan]
) -> MsgGenerator:
    for parameters in parameter_list:
        current_visit = self._run_task_remotely(parameters.get_task_request(instrument_session)) or current_visit
    return current_visit

or

def decode_and_execute(
    self, current_visit: str | None, parameter_list: Sequence[ExecutablePlan]
  ) -> MsgGenerator:
    for parameters in parameter_list:
        current_visit = parameters.execute_plan(self.context) or current_visit
    return current_visit

I might have misunderstood something and there may be good reasons to not do this but I thought it could be worth discussing

[rtuck99]

InProcessRunner will ultimately go away, once we are running happily in blueapi, we can delete it. The abstraction has been created so that we can temporarily switch between both setups. It is annoying that we have to have code that is very similar in both runner implementations, unfortunately there isn't a nice way around it with one method being a generator function and the other not.

It's probably unlikely we will add more plans for agamemnon to run. What is more likely is commissioning plans but these wouldn't be called by hyperion-supervisor.

[jacob720]

That makes sense, thanks for explaining