Add ref to update and send signatures to simplify type checking

fix some test pyright errors

Author: James Souter

src/fastcs/attribute_io.py

@@ -12,12 +12,15 @@ def __init_subclass__(cls) -> None:
         args = get_args(cast(Any, cls).__orig_bases__[0])
         cls.ref_type = args[0]
 
-    async def update(self, attr: AttrR[T, AttributeIORefT]) -> None:
+    async def update(
+        self, attr: AttrR[T, AttributeIORefT], ref: AttributeIORefT
+    ) -> None:
         raise NotImplementedError()
 
     async def send(
         self,
         attr: AttrRW[T, AttributeIORefT],
+        ref: AttributeIORefT,
         value,  # TODO, type this
     ) -> None:
         raise NotImplementedError()

src/fastcs/attribute_io_ref.py

@@ -7,4 +7,4 @@ class AttributeIORef:
     update_period: float | None = None
 
 
-AttributeIORefT = TypeVar("AttributeIORefT", bound=AttributeIORef, covariant=True)
+AttributeIORefT = TypeVar("AttributeIORefT", bound=AttributeIORef)

src/fastcs/attributes.py

@@ -1,15 +1,15 @@
 from __future__ import annotations
 
 import asyncio
-from collections.abc import Callable, Coroutine
-from typing import Generic, Self
+from collections.abc import Callable
+from typing import Generic
 
 from typing_extensions import TypeVar
 
 import fastcs
 from fastcs.attribute_io_ref import AttributeIORef
 
-from .datatypes import ATTRIBUTE_TYPES, AttrCallback, DataType, T
+from .datatypes import ATTRIBUTE_TYPES, AttrSetCallback, AttrUpdateCallback, DataType, T
 
 # TODO rename this: typevar with default
 AttributeIORefTD = TypeVar(
@@ -97,8 +97,8 @@ def __init__(
         self._value: T = (
             datatype.initial_value if initial_value is None else initial_value
         )
-        self._on_set_callbacks: list[AttrCallback[T]] | None = None
-        self._on_update_callbacks: list[AttrCallback[T]] | None = None
+        self._on_set_callbacks: list[AttrSetCallback[T]] | None = None
+        self._on_update_callbacks: list[AttrUpdateCallback] | None = None
 
     def get(self) -> T:
         return self._value
@@ -109,21 +109,19 @@ async def set(self, value: T) -> None:
         if self._on_set_callbacks is not None:
             await asyncio.gather(*[cb(self._value) for cb in self._on_set_callbacks])
 
-    def add_set_callback(self, callback: AttrCallback[T]) -> None:
+    def add_set_callback(self, callback: AttrSetCallback[T]) -> None:
         if self._on_set_callbacks is None:
             self._on_set_callbacks = []
         self._on_set_callbacks.append(callback)
 
-    def add_update_callback(
-        self, callback: Callable[[Self], Coroutine[None, None, None]]
-    ):
+    def add_update_callback(self, callback: AttrUpdateCallback):
         if self._on_update_callbacks is None:
             self._on_update_callbacks = []
         self._on_update_callbacks.append(callback)
 
     async def update(self):
         if self._on_update_callbacks is not None:
-            await asyncio.gather(*[cb(self) for cb in self._on_update_callbacks])
+            await asyncio.gather(*[cb() for cb in self._on_update_callbacks])
 
 
 class AttrW(Attribute[T, AttributeIORefTD]):
@@ -142,8 +140,8 @@ def __init__(
             group,
             description=description,
         )
-        self._process_callbacks: list[AttrCallback[T]] | None = None
-        self._write_display_callbacks: list[AttrCallback[T]] | None = None
+        self._process_callbacks: list[AttrSetCallback[T]] | None = None
+        self._write_display_callbacks: list[AttrSetCallback[T]] | None = None
 
     async def process(self, value: T) -> None:
         await self.process_without_display_update(value)
@@ -159,15 +157,15 @@ async def update_display_without_process(self, value: T) -> None:
         if self._write_display_callbacks:
             await asyncio.gather(*[cb(value) for cb in self._write_display_callbacks])
 
-    def add_process_callback(self, callback: AttrCallback[T]) -> None:
+    def add_process_callback(self, callback: AttrSetCallback[T]) -> None:
         if self._process_callbacks is None:
             self._process_callbacks = []
         self._process_callbacks.append(callback)
 
     def has_process_callback(self) -> bool:
         return bool(self._process_callbacks)
 
-    def add_write_display_callback(self, callback: AttrCallback[T]) -> None:
+    def add_write_display_callback(self, callback: AttrSetCallback[T]) -> None:
         if self._write_display_callbacks is None:
             self._write_display_callbacks = []
         self._write_display_callbacks.append(callback)

src/fastcs/controller.py

@@ -72,26 +72,29 @@ def _create_send_callback(self, io, attr):
 
             async def send_callback(value):
                 await attr.update_display_without_process(value)
-
                 if isinstance(attr, AttrRW):
                     await attr.set(value)
-
         else:
 
             async def send_callback(value):
-                await io.send(attr, value)
+                await io.send(attr, attr.io_ref, value)
+                # TODO, should we just then call the above send_callback here?
 
         return send_callback
 
     def _create_update_callback(self, io, attr):
         if io is None or attr.io_ref is None:
 
-            async def update_callback(attr):
+            async def error_callback():
                 raise RuntimeError("No AttributeIO registered to handle update")
 
-            return update_callback
+            return error_callback
         else:
-            return io.update
+
+            async def update_callback():
+                await io.update(attr, attr.io_ref)
+
+            return update_callback
 
     @property
     def path(self) -> list[str]:

src/fastcs/datatypes.py

@@ -24,7 +24,8 @@
 ATTRIBUTE_TYPES: tuple[type] = T.__constraints__  # type: ignore
 
 
-AttrCallback = Callable[[T], Awaitable[None]]
+AttrSetCallback = Callable[[T], Awaitable[None]]
+AttrUpdateCallback = Callable[[], Awaitable[None]]
 
 
 @dataclass(frozen=True)

src/fastcs/demo/controllers.py

@@ -39,16 +39,16 @@ def __init__(self, connection: IPConnection, suffix: str):
         self.suffix = suffix
 
     async def send(
-        self, attr: AttrW[T, TemperatureControllerAttributeIORef], value: T
+        self, attr: AttrW, ref: TemperatureControllerAttributeIORef, value: T
     ) -> None:
         await self._connection.send_command(
-            f"{attr.io_ref.name}{self.suffix}={attr.dtype(value)}\r\n"
+            f"{ref.name}{self.suffix}={attr.dtype(value)}\r\n"
         )
 
-    async def update(self, attr: AttrR[T, TemperatureControllerAttributeIORef]) -> None:
-        response = await self._connection.send_query(
-            f"{attr.io_ref.name}{self.suffix}?\r\n"
-        )
+    async def update(
+        self, attr: AttrR, ref: TemperatureControllerAttributeIORef
+    ) -> None:
+        response = await self._connection.send_query(f"{ref.name}{self.suffix}?\r\n")
         response = response.strip("\r\n")
 
         await attr.set(attr.dtype(response))

tests/assertable_controller.py

@@ -24,11 +24,11 @@ class MyTestAttributeIORef(AttributeIORef):
 
 
 class MyTestAttributeIO(AttributeIO[MyTestAttributeIORef, T]):
-    async def update(self, attr: AttrR[T]):
-        print(f"{self.controller} update {attr}")
+    async def update(self, attr: AttrR[T], ref: MyTestAttributeIORef):
+        print(f"update {attr}")
 
-    async def send(self, attr: AttrW[T], value: T):
-        print(f"{self.controller}: {attr} = {value}")
+    async def send(self, attr: AttrW[T], ref: MyTestAttributeIORef, value: T):
+        print(f"sending {attr} = {value}")
 
 
 test_attribute_io = MyTestAttributeIO()  # instance

tests/test_attribute.py

@@ -1,6 +1,6 @@
 from dataclasses import dataclass
 from functools import partial
-from typing import Generic
+from typing import Generic, TypeVar
 
 import pytest
 from pytest_mock import MockerFixture
@@ -13,7 +13,9 @@
     AttrW,
 )
 from fastcs.controller import Controller
-from fastcs.datatypes import Float, Int, String, T
+from fastcs.datatypes import Float, Int, String
+
+NumberT = TypeVar("NumberT", int, float)
 
 
 @pytest.mark.asyncio
@@ -68,8 +70,8 @@ class MyAttributeIORef(AttributeIORef):
         cool: int
 
     class MyAttributeIO(AttributeIO[MyAttributeIORef, int]):
-        async def update(self, attr: AttrR[Int, MyAttributeIORef]):
-            print("I am updating", self.ref_type, attr.io_ref.cool)
+        async def update(self, attr: AttrR, ref: MyAttributeIORef):
+            print("I am updating", self.ref_type, ref.cool)
 
     class MyController(Controller):
         my_attr = AttrR(Int(), io_ref=MyAttributeIORef(cool=5))
@@ -114,38 +116,49 @@ async def test_dynamic_attribute_io_specification():
     ]
 
     @dataclass
-    class DemoParameterAttributeIORef(AttributeIORef, Generic[T]):
+    class DemoParameterAttributeIORef(AttributeIORef, Generic[NumberT]):
         name: str
-        min: T | None = None
-        max: T | None = None
+        # TODO, this is weird, we should just use the attributes's min and max fields
+        min: NumberT | None = None
+        max: NumberT | None = None
         read_only: bool = False
 
-    class DemoParameterAttributeIO(AttributeIO[DemoParameterAttributeIORef, T]):
-        async def update(self, attr: AttrR[T]):
+    class DemoParameterAttributeIO(AttributeIO[DemoParameterAttributeIORef, NumberT]):
+        async def update(
+            self,
+            attr: AttrR[NumberT],
+            ref: DemoParameterAttributeIORef,
+        ):
             # OK, so this doesn't really work when we have min and maxes...
             await attr.set(attr.get() + 1)
 
-        async def send(self, attr: AttrW[T], value) -> None:
+        async def send(
+            self,
+            attr: AttrW[NumberT, DemoParameterAttributeIORef],
+            ref: DemoParameterAttributeIORef,
+            value: NumberT,
+        ) -> None:
             if (
-                attr.io_ref.read_only
+                ref.read_only
             ):  # TODO, this isn't necessary as we can not call process on this anyway
-                raise RuntimeError(
-                    f"Could not set read only attribute {attr.io_ref.name}"
-                )
+                raise RuntimeError(f"Could not set read only attribute {ref.name}")
 
-            if (io_min := attr.io_ref.min) is not None and value < io_min:
+            if (io_min := ref.min) is not None and value < io_min:
                 raise RuntimeError(
-                    f"Could not set {attr.io_ref.name} to {value}, "
-                    f"min is {attr.io_ref.min}"
+                    f"Could not set {ref.name} to {value}, min is {ref.min}"
                 )
 
-            if (io_max := attr.io_ref.max) is not None and value > io_max:
+            if (io_max := ref.max) is not None and value > io_max:
                 raise RuntimeError(
-                    f"Could not set {attr.io_ref.name} to {value}, "
-                    f"max is {attr.io_ref.max}"
+                    f"Could not set {ref.name} to {value}, max is {ref.max}"
                 )
+            # TODO: we should always end send with a update_display_without_process...
 
     class DemoParameterController(Controller):
+        ro_int_parameter: AttrR
+        int_parameter: AttrRW
+        float_parameter: AttrRW  # hint to satisfy pyright
+
         async def initialise(self):
             dtype_mapping = {"int": Int(), "float": Float()}
             for parameter_response in example_introspection_response:

tests/test_backend.py

@@ -7,7 +7,7 @@
 from fastcs.backend import Backend, build_controller_api
 from fastcs.controller import Controller
 from fastcs.cs_methods import Command
-from fastcs.datatypes import Int, T
+from fastcs.datatypes import Int
 from fastcs.exceptions import FastCSError
 from fastcs.wrappers import command, scan
 
@@ -98,13 +98,13 @@ async def test_wrapper():
 def test_update_periods():
     @dataclass
     class AttributeIORefTimesCalled(AttributeIORef):
-        update_period: float | None
+        update_period: float | None = None
         _times_called = 0
 
-    class AttributeIOTimesCalled(AttributeIO[AttributeIORefTimesCalled, T]):
-        async def update(self, attr):
-            attr.io_ref._times_called += 1
-            await attr.set(attr.io_ref._times_called)
+    class AttributeIOTimesCalled(AttributeIO[AttributeIORefTimesCalled, int]):
+        async def update(self, attr: AttrR[int], ref: AttributeIORefTimesCalled):
+            ref._times_called += 1
+            await attr.set(ref._times_called)
 
     class MyController(Controller):
         update_once = AttrR(Int(), io_ref=AttributeIORefTimesCalled(update_period=ONCE))