More review comments

src/fastcs_eiger/eiger_controller.py

@@ -208,7 +208,7 @@ async def initialise(self) -> None:
                         raise NotImplementedError(
                             f"No subcontroller implemented for subsystem {subsystem}"
                         )
-                self.register_sub_controller(subsystem.upper(), controller)
+                self.register_sub_controller(subsystem.capitalize(), controller)
                 await controller.initialise()
         except HTTPRequestError:
             print("\nAn HTTP request failed while introspecting detector:\n")
@@ -274,6 +274,10 @@ async def initialise(self) -> None:
 
     @classmethod
     def _group(cls, parameter: EigerParameter):
+        if "/" in parameter.key:
+            group_parts = parameter.key.split("/")[:-1]
+            # e.g. "threshold/difference/mode" -> ThresholdDifference
+            return "".join(list(map(str.capitalize, group_parts)))
         return f"{parameter.subsystem.capitalize()}{parameter.mode.capitalize()}"
 
     @classmethod
@@ -336,22 +340,22 @@ async def update(self):
             return
 
         async with self._parameter_update_lock:
-            parameters = self._parameter_updates.copy()
+            keys_to_check = self._parameter_updates.copy()
             self._parameter_updates.clear()
 
         # Release lock while fetching parameters - this may be slow
         parameter_updates: list[Coroutine] = []
-        for parameter in parameters:
-            if parameter in IGNORED_KEYS:
+        for key in keys_to_check:
+            if key in IGNORED_KEYS:
                 continue
-            attr_name = _key_to_attribute_name(parameter)
+            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]
                     parameter_updates.append(updater.config_update(self, attr))
                 case _ as attr:
-                    print(f"Failed to handle update for {parameter}: {attr}")
+                    print(f"Failed to handle update for {key}: {attr}")
         await asyncio.gather(*parameter_updates)
 
 
@@ -362,16 +366,6 @@ class EigerDetectorController(EigerSubsystemController):
     trigger_mode = AttrRW(String())  # TODO: Include URI and validate type from API
     trigger_exposure = AttrRW(Float(), handler=LogicHandler())
 
-    @detector_command
-    async def trigger(self):
-        match self.trigger_mode.get(), self.trigger_exposure.get():
-            case ("inte", exposure) if exposure > 0.0:
-                await self.connection.put(command_uri("trigger"), exposure)
-            case ("ints" | "inte", _):
-                await self.connection.put(command_uri("trigger"))
-            case _:
-                raise RuntimeError("Can only do soft trigger in 'ints' or 'inte' mode")
-
     async def initialise(self) -> None:
         # Check current state of detector_state to see if initializing is required.
         state_val = await self.connection.get("detector/api/1.8.0/status/state")
@@ -388,6 +382,16 @@ async def initialize(self):
     async def arm(self):
         await self.connection.put(command_uri("arm"))
 
+    @detector_command
+    async def trigger(self):
+        match self.trigger_mode.get(), self.trigger_exposure.get():
+            case ("inte", exposure) if exposure > 0.0:
+                await self.connection.put(command_uri("trigger"), exposure)
+            case ("ints" | "inte", _):
+                await self.connection.put(command_uri("trigger"))
+            case _:
+                raise RuntimeError("Can only do soft trigger in 'ints' or 'inte' mode")
+
     @detector_command
     async def disarm(self):
         await self.connection.put(command_uri("disarm"))
@@ -400,14 +404,6 @@ async def abort(self):
     async def cancel(self):
         await self.connection.put(command_uri("cancel"))
 
-    @classmethod
-    def _group(cls, parameter: EigerParameter) -> str:
-        if "/" in parameter.key:
-            group_parts = parameter.key.split("/")[:-1]
-            # e.g. "threshold/difference/mode" -> ThresholdDifference
-            return "".join(list(map(str.capitalize, group_parts)))
-        return super()._group(parameter)
-
 
 class EigerMonitorController(EigerSubsystemController):
     _subsystem = "monitor"

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

@@ -94,15 +94,15 @@ async def test_introspection(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
 

tests/test_controller.py

@@ -75,9 +75,9 @@ async def test_eiger_controller_initialises(mocker: MockerFixture, mock_connecti
     connection.get = mock_connection.get
     await eiger_controller.initialise()
     assert list(eiger_controller.get_sub_controllers().keys()) == [
-        "DETECTOR",
-        "STREAM",
-        "MONITOR",
+        "Detector",
+        "Stream",
+        "Monitor",
     ]
     connection.get.assert_any_call("detector/api/1.8.0/status/state")
     connection.get.assert_any_call("stream/api/1.8.0/status/state")