Update to use new attribute API and pyright (#57)

* Update to use new attribute API and pyright

* fix parameter updates in EigerSubsystemController.update

implement pyright review comments

* Fix handling of string[] values

* Update FastCS dependency to 0.8.0

.copier-answers.yml

@@ -15,4 +15,4 @@ github_org: DiamondLightSource
 package_name: fastcs_eiger
 pypi: true
 repo_name: fastcs-eiger
-type_checker: mypy
+type_checker: pyright

pyproject.toml

@@ -12,7 +12,7 @@ classifiers = [
 description = "Eiger control system integration with FastCS"
 dependencies = [
     "aiohttp",
-    "fastcs~=0.7.0",
+    "fastcs~=0.8.0",
     "numpy",
     "pillow",
     "typer",
@@ -27,11 +27,11 @@ dev = [
     "tickit-devices @ git+https://github.com/dls-controls/tickit-devices.git@eiger-stream2",
     "black",
     "copier",
-    "mypy",
     "myst-parser",
     "pipdeptree",
     "pre-commit",
     "pydata-sphinx-theme>=0.12",
+    "pyright",
     "pytest",
     "pytest-asyncio",
     "pytest-cov",
@@ -57,8 +57,9 @@ name = "Gary Yendell"
 [tool.setuptools_scm]
 version_file = "src/fastcs_eiger/_version.py"
 
-[tool.mypy]
-ignore_missing_imports = true # Ignore missing stubs in imported modules
+[tool.pyright]
+typeCheckingMode = "standard"
+reportMissingImports = false # Ignore missing stubs in imported modules
 
 [tool.pytest.ini_options]
 # Run pytest with all our checkers, and don't spam us with massive tracebacks on error
@@ -91,12 +92,12 @@ passenv = *
 allowlist_externals =
     pytest
     pre-commit
-    mypy
+    pyright
     sphinx-build
     sphinx-autobuild
 commands =
     pre-commit: pre-commit run --all-files --show-diff-on-failure {posargs}
-    type-checking: mypy src tests {posargs}
+    type-checking: pyright src tests {posargs}
     tests: pytest --cov=fastcs_eiger --cov-report term --cov-report xml:cov.xml {posargs}
     docs: sphinx-{posargs:build -EW --keep-going} -T docs build/html
 """

src/fastcs_eiger/__main__.py

@@ -2,9 +2,12 @@
 from typing import Optional
 
 import typer
-from fastcs.backends.asyncio_backend import AsyncioBackend
-from fastcs.backends.epics.backend import EpicsBackend
-from fastcs.backends.epics.gui import EpicsGUIOptions
+from fastcs.launch import FastCS
+from fastcs.transport.epics.options import (
+    EpicsGUIOptions,
+    EpicsIOCOptions,
+    EpicsOptions,
+)
 
 from fastcs_eiger import __version__
 from fastcs_eiger.eiger_controller import EigerController
@@ -52,19 +55,16 @@ def ioc(
 
     controller = EigerController(ip, port)
 
-    backend = EpicsBackend(controller, pv_prefix)
-    backend.create_gui(
-        EpicsGUIOptions(output_path=ui_path / "eiger.bob", title=f"Eiger - {pv_prefix}")
+    options = EpicsOptions(
+        ioc=EpicsIOCOptions(pv_prefix=pv_prefix),
+        gui=EpicsGUIOptions(
+            output_path=ui_path / "eiger.bob", title=f"Eiger - {pv_prefix}"
+        ),
     )
-    backend.run()
-
-
-@app.command()
-def asyncio(ip: str = EigerIp, port: int = EigerPort):
-    controller = EigerController(ip, port)
-
-    backend = AsyncioBackend(controller)
-    backend.run_interactive_session()
+    launcher = FastCS(controller, options)
+    launcher.create_docs()
+    launcher.create_gui()
+    launcher.run()
 
 
 # test with: python -m fastcs_eiger

src/fastcs_eiger/eiger_controller.py

@@ -5,8 +5,8 @@
 from typing import Any, Literal
 
 import numpy as np
-from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW
-from fastcs.controller import Controller, SubController
+from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW, Handler
+from fastcs.controller import BaseController, Controller, SubController
 from fastcs.datatypes import Bool, Float, Int, String
 from fastcs.wrappers import command, scan
 from PIL import Image
@@ -62,10 +62,10 @@ class EigerHandler:
     """
 
     uri: str
-    update_period: float = 0.2
+    update_period: float | None = 0.2
 
     async def put(
-        self, controller: "EigerSubsystemController", _: AttrW, value: Any
+        self, controller: "EigerSubsystemController", attr: AttrW, value: Any
     ) -> None:
         parameters_to_update = await controller.connection.put(self.uri, value)
         if not parameters_to_update:
@@ -86,10 +86,15 @@ async def put(
 
         await controller.queue_update(parameters_to_update)
 
-    async def update(self, controller: "EigerController", attr: AttrR) -> None:
+    async def update(self, controller: "EigerSubsystemController", attr: AttrR) -> None:
         try:
             response = await controller.connection.get(self.uri)
-            await attr.set(response["value"])
+            value = response["value"]
+            if isinstance(value, list) and all(
+                isinstance(s, str) for s in value
+            ):  # error is a list of strings
+                value = ", ".join(value)
+            await attr.set(value)
         except Exception as e:
             print(f"Failed to get {self.uri}:\n{e.__class__.__name__} {e}")
 
@@ -116,15 +121,16 @@ async def config_update(
 
 
 @dataclass
-class LogicHandler:
+class LogicHandler(Handler):
     """
     Handler for FastCS Attribute Creation
 
     Dataclass that is called using the AttrR, AttrRW function.
     Used for dynamically created attributes that are added for additional logic
     """
 
-    async def put(self, _: "EigerController", attr: AttrW, value: Any) -> None:
+    async def put(self, controller: BaseController, attr: AttrW, value: Any) -> None:
+        assert isinstance(attr, AttrR)  # AttrW does not implement set
         await attr.set(value)
 
 
@@ -227,13 +233,21 @@ async def initialise(self) -> None:
             print("\nAn HTTP request failed while introspecting detector:\n")
             raise
 
+    def get_subsystem_controllers(self) -> list["EigerSubsystemController"]:
+        return [
+            controller
+            for controller in self.get_sub_controllers().values()
+            if isinstance(controller, EigerSubsystemController)
+        ]
+
     @scan(0.1)
     async def update(self):
         """Periodically check for parameters that need updating from the detector."""
+        subsystem_controllers = self.get_subsystem_controllers()
         await self.stale_parameters.set(
-            any(c.stale_parameters.get() for c in self.get_sub_controllers().values())
+            any(c.stale_parameters.get() for c in subsystem_controllers)
         )
-        controller_updates = [c.update() for c in self.get_sub_controllers().values()]
+        controller_updates = [c.update() for c in subsystem_controllers]
         await asyncio.gather(*controller_updates)
 
 
@@ -283,7 +297,7 @@ async def initialise(self) -> None:
         attributes = self._create_attributes(parameters)
 
         for name, attribute in attributes.items():
-            setattr(self, name, attribute)
+            self.attributes[name] = attribute
 
     @classmethod
     def _group(cls, parameter: EigerParameter):
@@ -314,18 +328,19 @@ def _create_attributes(cls, parameters: list[EigerParameter]):
                     datatype = String()
                 case _:
                     print(f"Failed to handle {parameter}")
+                    continue
 
             group = cls._group(parameter)
             match parameter.response["access_mode"]:
                 case "r":
                     attributes[parameter.attribute_name] = AttrR(
-                        datatype,
+                        datatype,  # type: ignore
                         handler=EIGER_HANDLERS[parameter.mode](parameter.uri),
                         group=group,
                     )
                 case "rw":
                     attributes[parameter.attribute_name] = AttrRW(
-                        datatype,
+                        datatype,  # type: ignore
                         handler=EIGER_HANDLERS[parameter.mode](parameter.uri),
                         group=group,
                         allowed_values=parameter.response.get("allowed_values", None),
@@ -362,10 +377,8 @@ async def update(self):
             if key in IGNORED_KEYS:
                 continue
             attr_name = _key_to_attribute_name(key)
-            match getattr(self, attr_name, None):
-                # TODO: mypy doesn't understand AttrR as a type for some reason:
-                # `error: Expected type in class pattern; found "Any"  [misc]`
-                case AttrR(updater=EigerConfigHandler() as updater) as attr:  # type: ignore [misc]
+            match self.attributes.get(attr_name, None):
+                case AttrR(updater=EigerConfigHandler() as updater) as attr:
                     parameter_updates.append(updater.config_update(self, attr))
                 case _ as attr:
                     print(f"Failed to handle update for {key}: {attr}")

tests/conftest.py

@@ -40,7 +40,7 @@ def sim_eiger_controller(request):
     )
 
     # Wait until ready
-    while True:
+    while proc.stdout is not None:
         line = proc.stdout.readline()
         if "Starting HTTP server..." in line:
             break

tests/system/parameters.json

@@ -1,5 +1,5 @@
 {
-    "Detector": {
+    "detector": {
         "humidity": {
             "subsystem": "detector",
             "mode": "status",
@@ -770,7 +770,7 @@
             }
         }
     },
-    "Stream": {
+    "stream": {
         "dropped": {
             "subsystem": "stream",
             "mode": "status",
@@ -863,7 +863,7 @@
             }
         }
     },
-    "Monitor": {
+    "monitor": {
         "buffer_free": {
             "subsystem": "monitor",
             "mode": "status",

tests/system/test_introspection.py

@@ -4,7 +4,7 @@
 from typing import Any
 
 import pytest
-from fastcs.attributes import Attribute, AttrR
+from fastcs.attributes import Attribute, AttrR, AttrRW
 from fastcs.datatypes import Float
 
 from fastcs_eiger.eiger_controller import (
@@ -15,6 +15,7 @@
     EigerMonitorController,
     EigerParameter,
     EigerStreamController,
+    EigerSubsystemController,
 )
 
 HERE = Path(__file__).parent
@@ -42,14 +43,14 @@ async def test_attribute_creation(sim_eiger_controller: EigerController):
     await controller.initialise()
     serialised_parameters: dict[str, dict[str, Any]] = {}
     subsystem_parameters = {}
-    for subsystem_name, subcontroller in controller.get_sub_controllers().items():
-        serialised_parameters[subsystem_name] = {}
+    for subcontroller in controller.get_subsystem_controllers():
+        serialised_parameters[subcontroller._subsystem] = {}
         subsystem_parameters[
-            subsystem_name
+            subcontroller._subsystem
         ] = await subcontroller._introspect_detector_subsystem()
-        for param in subsystem_parameters[subsystem_name]:
-            serialised_parameters[subsystem_name][param.key] = _serialise_parameter(
-                param
+        for param in subsystem_parameters[subcontroller._subsystem]:
+            serialised_parameters[subcontroller._subsystem][param.key] = (
+                _serialise_parameter(param)
             )
 
     expected_file = HERE / "parameters.json"
@@ -61,15 +62,15 @@ async def test_attribute_creation(sim_eiger_controller: EigerController):
     assert serialised_parameters == expected_parameters, "Detector API does not match"
 
     detector_attributes = EigerDetectorController._create_attributes(
-        subsystem_parameters["Detector"]
+        subsystem_parameters["detector"]
     )
     assert len(detector_attributes) == 76
     monitor_attributes = EigerMonitorController._create_attributes(
-        subsystem_parameters["Monitor"]
+        subsystem_parameters["monitor"]
     )
     assert len(monitor_attributes) == 7
     stream_attributes = EigerStreamController._create_attributes(
-        subsystem_parameters["Stream"]
+        subsystem_parameters["stream"]
     )
     assert len(stream_attributes) == 8
 
@@ -95,6 +96,7 @@ async def test_controller_groups_and_parameters(sim_eiger_controller: EigerContr
 
     for subsystem in MISSING_KEYS:
         subcontroller = controller.get_sub_controllers()[subsystem.title()]
+        assert isinstance(subcontroller, EigerSubsystemController)
         parameters = await subcontroller._introspect_detector_subsystem()
         if subsystem == "detector":
             # ignored keys should not get added to the controller
@@ -107,9 +109,9 @@ async def test_controller_groups_and_parameters(sim_eiger_controller: EigerContr
                     if attr_name == "threshold_energy":
                         continue
                     assert attr.group and "Threshold" in attr.group
-
-            attr = subcontroller.threshold_1_energy
+            attr: AttrRW = subcontroller.attributes["threshold_1_energy"]  # type: ignore
             sender = attr.sender
+            assert sender is not None
             await sender.put(subcontroller, attr, 100.0)
             # set parameters to update based on response to put request
             assert subcontroller._parameter_updates == {
@@ -123,8 +125,9 @@ async def test_controller_groups_and_parameters(sim_eiger_controller: EigerContr
             subcontroller._parameter_updates.clear()
 
             # make sure API inconsistency for threshold/difference/mode is addressed
-            attr = subcontroller.threshold_difference_mode
+            attr: AttrRW = subcontroller.attributes["threshold_difference_mode"]  # type: ignore
             sender = attr.sender
+            assert sender is not None
             await sender.put(subcontroller, attr, "enabled")
             assert subcontroller._parameter_updates == {"threshold/difference/mode"}
 

tests/test_controller.py

@@ -4,7 +4,11 @@
 import pytest
 from pytest_mock import MockerFixture
 
-from fastcs_eiger.eiger_controller import EigerController, EigerHandler
+from fastcs_eiger.eiger_controller import (
+    EigerController,
+    EigerDetectorController,
+    EigerHandler,
+)
 
 _lock = asyncio.Lock()
 
@@ -71,7 +75,7 @@ async def test_stale_parameter_propagates_to_top_controller(mocker: MockerFixtur
     await eiger_controller.initialise()
 
     detector_controller = eiger_controller.get_sub_controllers()["Detector"]
-
+    assert isinstance(detector_controller, EigerDetectorController)
     # queueing update sets subcontroller to stale
     assert detector_controller.stale_parameters.get() is False
     await detector_controller.queue_update(["dummy_attribute"])