Adapter rework for Zebra

[abbiemery]

This is a possible change to the way adapters work within tickit. This attempts unifies the adapters in a composed way by seperating out the IO and the device specific handling logic. The idea is that you use an AdapterContainer which has both an adapter (the part that handles the messages) and the io which does the rest.

The AdapterContainer acts as the previous adapters did and contains the run_forever method. Now though, this calls a setup method in the io in which the io takes the specific adapter that has been provided. The adapter and io are failry coupled given the io needs a specific adapter type to work, eg. class HttpIo(AdapterIo[HttpAdapter]): . However i was hoping this change provides a clearer divide in developer and user implemented code.

In the http example:

class HttpIo(AdapterIo[HttpAdapter]):
class MyDeviceSpecificAdapter(HttpAdapter):

myAdapterContainer.io = HttpIo
myAdapterContainer.adapter = MyDeviceSpecificAdapter

As before users would need to write specific adapters for their devices but this should simplifiy their lives a bit as all they now need is the device and device specific methods. For example the http adapter on an iobox:

class IoBoxHttpAdapter(HttpAdapter):
    """An adapter for an IoBox that allows reads and writes via REST calls"""

    io_box: IoBoxDevice

    def __init__(self, io_box: IoBoxDevice) -> None:
        self.io_box = io_box

    @HttpEndpoint.put("/memory/{address}", interrupt=True)
    async def write_to_address(self, request: web.Request) -> web.Response:
        address = request.match_info["address"]
        new_value = (await request.json())["value"]
        self.io_box.write(address, new_value)
        return web.json_response({address: new_value})

    @HttpEndpoint.get("/memory/{address}")
    async def read_from_address(self, request: web.Request) -> web.Response:
        address = request.match_info["address"]
        value = self.io_box.read(address)
        return web.json_response({address: value})

Then the component config:

@pydantic.v1.dataclasses.dataclass
class ExampleHttpDevice(ComponentConfig):
    """Example HTTP device."""

    host: str = "localhost"
    port: int = 8080

    def __call__(self) -> Component:  # noqa: D102
        device = IoBoxDevice()
        adapters = [
            AdapterContainer(
                IoBoxHttpAdapter(device),
                HttpIo(
                    self.host,
                    self.port,
                ),
            )
        ]
        return DeviceSimulation(
            name=self.name,
            device=device,
            adapters=adapters,
        )

Most importantly, and the motivation for this change, is there is no longer a limitation on what an adapter may apply to. This allows us to write an adapter which inspect the components in a system simulation. I put a quick example one I made in the example folder which used a tcpio and commandadapter.

class BaseSystemSimulationAdapter:
    """A common base adapter for system simulation adapters.

    They should be able to use any interpreter available.
    """

    _components: Dict[ComponentID, Component]
    _wiring: Union[Wiring, InverseWiring]

    def setup_adapter(
        self,
        components: Dict[ComponentID, Component],
        wiring: Union[Wiring, InverseWiring],
    ) -> None:
        self._components = components
        self._wiring = wiring


class SystemSimulationAdapter(BaseSystemSimulationAdapter, CommandInterpreter):
    """Network adapter for a generic system simulation.

    Network adapter for a generic system simulation using a command interpreter. This
    Can be used to query the system simulation component for a list of the
    ComponentID's for the components in the system and given a specific ID, the details
    of that component.
    """

    _byte_format: ByteFormat = ByteFormat(b"%b\r\n")

    @RegexCommand(r"ids", False, "utf-8")
    async def get_component_ids(self) -> bytes:
        """Returns a list of ids for all the components in the system simulation."""
        return str(self._components.keys()).encode("utf-8")

    ...

I'd really appreciate opinions on the idea before I commit to it and rewrite half the test suite. I'm particularly not certain on:

1. The naming. It doesn't quite fit for me but i'm lacking the oversight and vocabulary to find something that feels right.
2. The epics adapter, I don't think i really divided the io and adapter code well here, it all felt rather intertwined.
3. The wrapper: I haven't figured out how to change these up yet, so they don't work. We COULD keep them wrapping or we could investigate the path of using a new specification(like regex) and maybe chain them there.

[codecov]

Codecov Report

Merging #174 (b47a673) into master (bd673a8) will decrease coverage by 0.58%.
The diff coverage is 93.82%.

@@            Coverage Diff             @@
##           master     #174      +/-   ##
==========================================
- Coverage   95.21%   94.64%   -0.58%     
==========================================
  Files          43       41       -2     
  Lines        1295     1306      +11     
==========================================
+ Hits         1233     1236       +3     
- Misses         62       70       +8     

Files Changed	Coverage Δ
...rc/tickit/adapters/specifications/regex_command.py	96.15% <ø> (ø)
src/tickit/adapters/utils.py	100.00% <ø> (ø)
src/tickit/adapters/tcp.py	82.60% <65.00%> (ø)
src/tickit/core/components/system_simulation.py	94.11% <75.00%> (-3.61%)	⬇️
src/tickit/adapters/io/zeromq_push_io.py	84.93% <90.00%> (ø)
src/tickit/adapters/zmq.py	90.90% <90.90%> (ø)
src/tickit/core/adapter.py	91.30% <93.33%> (-0.37%)	⬇️
src/tickit/adapters/io/tcp_io.py	97.67% <97.67%> (ø)
src/tickit/adapters/epics.py	76.78% <100.00%> (ø)
src/tickit/adapters/http.py	100.00% <100.00%> (ø)
... and 7 more

[tpoliaw]

• The naming

  adapters sound like they should be adapting something but as far as I can make out, they provide IO for external processes to modify the state of devices. Should the DeviceSimulation/Component types have a mutators/modfiers/io list instead? Adapter makes sense to be the shim between the device and the interface the io type requires, ie, an Adapter adapts a Device to allow it to be controlled by an IO source to mutate the Component?

• The epics adapter,

  I think this still applies to all the adapters to a certain extent. It feels like there should be a way to make the Io components do more work and have the adapters purely be a mapping from end point to device method.

• The wrapper

  If they weren't working before and no one was using them, deleting them for now and revisiting them when a clearer use case comes up makes sense.

I'm not sure I understand the need for AdapterContainers - why can't the io instances have the adapters passed in at the same time as their configuration? At the moment the whole class seems to be a single function. Where container.run_forever(interrupt) is called now, it could call io.setup(interrupt) and have the same behaviour? Or is that what they used to do?

Given this is such a big PR, I've not really looked that closely at the code as much as the restructuring of the adapters. I've not looked at the tests at all.

If this works as it is and unblocks the work on the zebra components, I'm tempted to say commit it as it is and deal with any fallout later. It could drag on for weeks otherwise.

[DiamondJoseph]

Does the adapter need access to the loaded db records?

[abbiemery]

At the moment this is the method which actually links the records on the ioc and the device attributes, so yes. It could benefit from renaming.

[DiamondJoseph]

should this be called if not self.db_file?

[abbiemery]

No, the confusing naming is perhaps the adapter.on_db_load() which links the records with device fields, or the callables to get them. You can also make your own records here and assign them, which is what happens in a world without db files.

[DiamondJoseph]

How does repeating a field name on a Pydantic dataclass work? Is this just to be explicit in the docs?

[abbiemery]

I just copied the other one and it works so honestly I don't know