Merge pull request #233 from DiamondLightSource/tidying

Tidying

Author: GDYendell

src/fastcs/attributes.py

@@ -41,11 +41,12 @@ def __init__(
         self.description = description
 
         # A callback to use when setting the datatype to a different value, for example
-        # changing the units on an int. This should be implemented in the backend.
+        # changing the units on an int.
         self._update_datatype_callbacks: list[Callable[[DataType[T]], None]] = []
 
-        # Name to be filled in by Controller when the Attribute is bound
-        self._name = None
+        # Path and name to be filled in by Controller it is bound to
+        self._name = ""
+        self._path = []
 
     @property
     def io_ref(self) -> AttributeIORefT:
@@ -68,6 +69,14 @@ def dtype(self) -> type[T]:
     def group(self) -> str | None:
         return self._group
 
+    @property
+    def name(self) -> str:
+        return self._name
+
+    @property
+    def path(self) -> list[str]:
+        return self._path
+
     def add_update_datatype_callback(
         self, callback: Callable[[DataType[T]], None]
     ) -> None:
@@ -82,16 +91,28 @@ def update_datatype(self, datatype: DataType[T]) -> None:
         for callback in self._update_datatype_callbacks:
             callback(datatype)
 
-    def set_name(self, name: list[str]):
+    def set_name(self, name: str):
         if self._name:
-            raise ValueError(
+            raise RuntimeError(
                 f"Attribute is already registered with a controller as {self._name}"
             )
 
         self._name = name
 
+    def set_path(self, path: list[str]):
+        if self._path:
+            raise RuntimeError(
+                f"Attribute is already registered with a controller at {self._path}"
+            )
+
+        self._path = path
+
     def __repr__(self):
-        return f"{self.__class__.__name__}({self._name}, {self._datatype})"
+        name = self.__class__.__name__
+        path = ".".join(self._path + [self._name]) or None
+        datatype = self._datatype.__class__.__name__
+
+        return f"{name}(path={path}, datatype={datatype}, io_ref={self._io_ref})"
 
 
 AttrIOUpdateCallback = Callable[["AttrR[T, Any]"], Awaitable[None]]

src/fastcs/control_system.py

@@ -200,7 +200,7 @@ def _build_controller_api(controller: BaseController, path: list[str]) -> Contro
         command_methods=command_methods,
         sub_apis={
             name: _build_controller_api(sub_controller, path + [name])
-            for name, sub_controller in controller.get_sub_controllers().items()
+            for name, sub_controller in controller.sub_controllers.items()
         },
         description=controller.description,
     )

src/fastcs/controller.py

@@ -67,7 +67,7 @@ def connect_attribute_ios(self) -> None:
             if isinstance(attr, AttrR):
                 attr.set_update_callback(io.update)
 
-        for controller in self.get_sub_controllers().values():
+        for controller in self.sub_controllers.values():
             controller.connect_attribute_ios()
 
     @property
@@ -80,6 +80,8 @@ def set_path(self, path: list[str]):
             raise ValueError(f"sub controller is already registered under {self.path}")
 
         self._path = path
+        for attribute in self.attributes.values():
+            attribute.set_path(path)
 
     def _bind_attrs(self) -> None:
         """Search for `Attributes` and `Methods` to bind them to this instance.
@@ -136,6 +138,7 @@ def add_attribute(self, name, attribute: Attribute):
             )
 
         attribute.set_name(name)
+        attribute.set_path(self.path)
         self.attributes[name] = attribute
         super().__setattr__(name, attribute)
 
@@ -158,13 +161,16 @@ def add_sub_controller(self, name: str, sub_controller: Controller):
         if isinstance(sub_controller.root_attribute, Attribute):
             self.attributes[name] = sub_controller.root_attribute
 
-    def get_sub_controllers(self) -> dict[str, Controller]:
+    @property
+    def sub_controllers(self) -> dict[str, Controller]:
         return self.__sub_controller_tree
 
     def __repr__(self):
-        return f"""\
-{type(self).__name__}({self.path}, {list(self.__sub_controller_tree.keys())})\
-"""
+        name = self.__class__.__name__
+        path = ".".join(self.path) or None
+        sub_controllers = list(self.sub_controllers.keys()) or None
+
+        return f"{name}(path={path}, sub_controllers={sub_controllers})"
 
     def __setattr__(self, name, value):
         if isinstance(value, Attribute):
@@ -179,9 +185,9 @@ class Controller(BaseController):
     """Top-level controller for a device.
 
     This is the primary class for implementing device support in FastCS. Instances of
-    this class can be loaded into a backend to access its ``Attribute``s. The backend
-    can then perform a specific function with the set of ``Attributes``, such as
-    generating a UI or creating parameters for a control system.
+    this class can be loaded into a FastCS to expose its ``Attribute``s to the transport
+    layer, which can then perform a specific function with the set of ``Attributes``,
+    such as generating a UI or creating parameters for a control system.
     """
 
     root_attribute: Attribute | None = None

src/fastcs/controller_api.py

@@ -49,18 +49,23 @@ def get_scan_and_initial_coros(
         initial_coros: list[Callable] = []
 
         for controller_api in self.walk_api():
-            _add_scan_method_tasks(scan_dict, controller_api)
+            _add_scan_method_tasks(scan_dict, initial_coros, controller_api)
             _add_attribute_update_tasks(scan_dict, initial_coros, controller_api)
 
         scan_coros = _get_periodic_scan_coros(scan_dict)
         return scan_coros, initial_coros
 
 
 def _add_scan_method_tasks(
-    scan_dict: dict[float, list[Callable]], controller_api: ControllerAPI
+    scan_dict: dict[float, list[Callable]],
+    initial_coros: list[Callable],
+    controller_api: ControllerAPI,
 ):
     for method in controller_api.scan_methods.values():
-        scan_dict[method.period].append(method.fn)
+        if method.period is ONCE:
+            initial_coros.append(method.fn)
+        else:
+            scan_dict[method.period].append(method.fn)
 
 
 def _add_attribute_update_tasks(

src/fastcs/datatypes.py

@@ -67,13 +67,16 @@ class _Numerical(DataType[T_Numerical]):
     min_alarm: T_Numerical | None = None
     max_alarm: T_Numerical | None = None
 
-    def validate(self, value: T_Numerical) -> T_Numerical:
-        super().validate(value)
-        if self.min is not None and value < self.min:
-            raise ValueError(f"Value {value} is less than minimum {self.min}")
-        if self.max is not None and value > self.max:
-            raise ValueError(f"Value {value} is greater than maximum {self.max}")
-        return value
+    def validate(self, value: Any) -> T_Numerical:
+        _value = super().validate(value)
+
+        if self.min is not None and _value < self.min:
+            raise ValueError(f"Value {_value} is less than minimum {self.min}")
+
+        if self.max is not None and _value > self.max:
+            raise ValueError(f"Value {_value} is greater than maximum {self.max}")
+
+        return _value
 
     @property
     def initial_value(self) -> T_Numerical:
@@ -99,11 +102,13 @@ class Float(_Numerical[float]):
     def dtype(self) -> type[float]:
         return float
 
-    def validate(self, value: float) -> float:
-        super().validate(value)
+    def validate(self, value: Any) -> float:
+        _value = super().validate(value)
+
         if self.prec is not None:
-            value = round(value, self.prec)
-        return value
+            _value = round(_value, self.prec)
+
+        return _value
 
 
 @dataclass(frozen=True)
@@ -177,21 +182,24 @@ def initial_value(self) -> np.ndarray:
         return np.zeros(self.shape, dtype=self.array_dtype)
 
     def validate(self, value: np.ndarray) -> np.ndarray:
-        super().validate(value)
-        if self.array_dtype != value.dtype:
+        _value = super().validate(value)
+
+        if self.array_dtype != _value.dtype:
             raise ValueError(
-                f"Value dtype {value.dtype} is not the same as the array dtype "
+                f"Value dtype {_value.dtype} is not the same as the array dtype "
                 f"{self.array_dtype}"
             )
-        if len(self.shape) != len(value.shape) or any(
+
+        if len(self.shape) != len(_value.shape) or any(
             shape1 > shape2
-            for shape1, shape2 in zip(value.shape, self.shape, strict=True)
+            for shape1, shape2 in zip(_value.shape, self.shape, strict=True)
         ):
             raise ValueError(
-                f"Value shape {value.shape} exceeeds the shape maximum shape "
+                f"Value shape {_value.shape} exceeeds the shape maximum shape "
                 f"{self.shape}"
             )
-        return value
+
+        return _value
 
 
 @dataclass(frozen=True)
@@ -207,12 +215,13 @@ def dtype(self) -> type[np.ndarray]:
     def initial_value(self) -> np.ndarray:
         return np.array([], dtype=self.structured_dtype)
 
-    def validate(self, value: np.ndarray) -> np.ndarray:
-        super().validate(value)
+    def validate(self, value: Any) -> np.ndarray:
+        _value = super().validate(value)
 
-        if self.structured_dtype != value.dtype:
+        if self.structured_dtype != _value.dtype:
             raise ValueError(
-                f"Value dtype {value.dtype.descr} is not the same as the structured "
+                f"Value dtype {_value.dtype.descr} is not the same as the structured "
                 f"dtype {self.structured_dtype}"
             )
-        return value
+
+        return _value

src/fastcs/transport/epics/ca/util.py

@@ -71,7 +71,7 @@ def record_metadata_from_datatype(
         case Waveform():
             if len(datatype.shape) != 1:
                 raise TypeError(
-                    f"Unsupported shape {datatype.shape}, the EPICS backend only "
+                    f"Unsupported shape {datatype.shape}, the EPICS transport only "
                     "supports to 1D arrays"
                 )
             arguments["length"] = datatype.shape[0]

src/fastcs/transport/epics/gui.py

@@ -100,6 +100,7 @@ def _get_attribute_component(
                     return None
                 return SignalRW(
                     name=name,
+                    description=attribute.description,
                     write_pv=pv,
                     write_widget=write_widget,
                     read_pv=pv + "_RBV",
@@ -109,12 +110,22 @@ def _get_attribute_component(
                 read_widget = self._get_read_widget(attribute.datatype)
                 if read_widget is None:
                     return None
-                return SignalR(name=name, read_pv=pv, read_widget=read_widget)
+                return SignalR(
+                    name=name,
+                    description=attribute.description,
+                    read_pv=pv,
+                    read_widget=read_widget,
+                )
             case AttrW():
                 write_widget = self._get_write_widget(attribute.datatype)
                 if write_widget is None:
                     return None
-                return SignalW(name=name, write_pv=pv, write_widget=write_widget)
+                return SignalW(
+                    name=name,
+                    description=attribute.description,
+                    write_pv=pv,
+                    write_widget=write_widget,
+                )
             case _:
                 raise FastCSError(f"Unsupported attribute type: {type(attribute)}")
 

tests/conftest.py

@@ -150,7 +150,7 @@ def run_ioc_as_subprocess(
         if not error_queue.empty():
             raise error_queue.get()
 
-        # close backend caches before the event loop
+        # close ca caches before the event loop
         purge_channel_caches()
 
         error_queue.close()
@@ -224,7 +224,7 @@ def test_controller(tango_system, register_device):
         if time.monotonic() - start_time > timeout:
             raise TimeoutError("Controller did not start in time")
 
-    # close backend caches before the event loop
+    # close ca caches before the event loop
     purge_channel_caches()
 
     # Stop buffer from getting full and blocking the subprocess

tests/test_controller.py

@@ -15,8 +15,8 @@ def test_controller_nesting():
 
     assert sub_controller.path == ["a"]
     assert sub_sub_controller.path == ["a", "b"]
-    assert controller.get_sub_controllers() == {"a": sub_controller}
-    assert sub_controller.get_sub_controllers() == {"b": sub_sub_controller}
+    assert controller.sub_controllers == {"a": sub_controller}
+    assert sub_controller.sub_controllers == {"b": sub_sub_controller}
 
     with pytest.raises(ValueError, match=r"existing sub controller"):
         controller.a = Controller()

tests/transport/epics/pva/test_p4p.py

@@ -71,7 +71,7 @@ async def test_scan_method(p4p_subprocess: tuple[str, Queue]):
     e_values = asyncio.Queue()
 
     # While the scan method will update every 0.1 seconds, it takes around that
-    # time for the p4p backends to update, broadcast, get.
+    # time for the p4p transport to update, broadcast, get.
     latency = 1e8
 
     e_monitor = ctxt.monitor(f"{pv_prefix}:Child1:E", e_values.put)