AI review comments

Author: Tom-Willemsen

src/fastcs_secop/_controllers.py

@@ -95,33 +95,44 @@ async def initialise(self) -> None:
     @command()
     async def execute(self) -> None:
         """Execute the command."""
-        prefix = f"do {self._module_name}:{self._command_name}"
-        response_prefix = f"done {self._module_name}:{self._command_name}"
-
-        if self.args is not None:
-            if self.raw_args:
-                cmd = f"{prefix} {self.args.get()}\n"
-            else:
-                cmd = f"{prefix} {encode(self.args.get(), self._datainfo['argument'])}\n"
-        else:
-            cmd = f"{prefix}\n"
-
-        logger.debug("Sending command: '%s'", cmd)
-        response = await self._connection.send_query(cmd)
-        logger.debug("Response: '%s'", response)
-
-        response = response.strip()
-        if not response.startswith(response_prefix):
-            logger.warning("command '%s' failed (response='%s')", prefix, response)
-            return
-
-        response = response[len(response_prefix) :].strip()
-
-        if self.result is not None:
-            if self.raw_result:
-                await self.result.update(orjson.dumps(orjson.loads(response)[0]).decode())
+        try:
+            prefix = f"do {self._module_name}:{self._command_name}"
+            response_prefix = f"done {self._module_name}:{self._command_name}"
+
+            if self.args is not None:
+                if self.raw_args:
+                    cmd = f"{prefix} {self.args.get()}\n"
+                else:
+                    cmd = f"{prefix} {encode(self.args.get(), self._datainfo['argument'])}\n"
             else:
-                await self.result.update(decode(response, self._datainfo["result"], self.result))
+                cmd = f"{prefix}\n"
+
+            logger.debug("Sending command: '%s'", cmd)
+            response = await self._connection.send_query(cmd)
+            logger.debug("Response: '%s'", response)
+
+            response = response.strip()
+            if not response.startswith(response_prefix):
+                logger.warning("command '%s' failed (response='%s')", prefix, response)
+                return
+
+            response = response[len(response_prefix) :].strip()
+
+            if self.result is not None:
+                if self.raw_result:
+                    await self.result.update(orjson.dumps(orjson.loads(response)[0]).decode())
+                else:
+                    await self.result.update(
+                        decode(response, self._datainfo["result"], self.result)
+                    )
+        except Exception as e:
+            logger.error(
+                "command %s:%s failed: %s: %s",
+                self._module_name,
+                self._command_name,
+                e.__class__.__name__,
+                e,
+            )
 
 
 class SecopModuleController(Controller):
@@ -318,7 +329,7 @@ async def check_idn(self) -> None:
                 f"Not a SECoP device?"
             )
 
-        print(f"Connected to SECoP device with IDN='{identification}'.")
+        logger.info("Connected to SECoP device with IDN='%s'.", identification)
 
     async def initialise(self) -> None:
         """Set up FastCS for this SECoP node.

src/fastcs_secop/_io.py

@@ -151,12 +151,12 @@ def encode(value: T, datainfo: dict[str, Any]) -> str:
             assert isinstance(value, enum.Enum)
             return orjson.dumps(value.value).decode()
         case "scaled":
-            return orjson.dumps(round(value / datainfo["scale"])).decode()
+            val = round(value / datainfo["scale"])
+            assert isinstance(val, int)
+            return orjson.dumps(val).decode()
         case "blob":
             assert isinstance(value, np.ndarray)
-            return orjson.dumps(
-                base64.b64encode("".join(chr(c) for c in value).encode("utf-8")).decode()
-            ).decode()
+            return orjson.dumps(base64.b64encode(value.tobytes()).decode()).decode()
         case "array":
             return orjson.dumps(value, option=orjson.OPT_SERIALIZE_NUMPY).decode()
         case "tuple":
@@ -217,8 +217,8 @@ async def send(self, attr: AttrW[T, SecopAttributeIORef], value: T) -> None:
         except ConnectionError:
             # Reconnect will be attempted in a periodic scan task
             pass
-        except Exception:
-            logger.exception("Exception during send()")
+        except Exception as e:
+            logger.error("Exception during send() for %s: %s: %s", attr, e.__class__.__name__, e)
 
 
 class SecopRawAttributeIO(AttributeIO[str, SecopRawAttributeIORef]):

src/fastcs_secop/_util.py

@@ -159,7 +159,7 @@ def secop_datainfo_to_fastcs_dtype(datainfo: dict[str, Any], raw: bool = False)
         case "bool":
             return Bool()
         case "enum":
-            enum_type = enum.Enum("enum_type", datainfo["members"])
+            enum_type = enum.Enum("GeneratedSecopEnum", datainfo["members"])
             return Enum(enum_type)
         case "string":
             return String()

tests/test_against_emulator.py

@@ -45,7 +45,6 @@ async def controller():
             ip="127.0.0.1",
             port=57677,
         ),
-        quirks={},
     )
 
     for _ in range(100):

tests/test_controller.py

@@ -142,7 +142,7 @@ async def test_secop_module_controller_initialise():
     assert "skipped_accessible" not in controller.attributes
 
 
-async def test_command_controller_execute_fails():
+async def test_command_controller_execute_invalid_response():
     connection = AsyncMock()
     controller = SecopCommandController(
         connection=connection,
@@ -157,6 +157,21 @@ async def test_command_controller_execute_fails():
     await controller.execute()  # No exception thrown
 
 
+async def test_command_controller_execute_fails():
+    connection = AsyncMock()
+    controller = SecopCommandController(
+        connection=connection,
+        command_name="some_command",
+        module_name="some_module",
+        datainfo={},
+        quirks=SecopQuirks(),
+    )
+    await controller.initialise()
+
+    connection.send_query.side_effect = Exception
+    await controller.execute()  # No exception thrown
+
+
 async def test_command_controller_execute_no_args_no_return():
     connection = AsyncMock()
     controller = SecopCommandController(