refactor: fix typos, update documentation, and improve code readability

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

ophyd_devices/devices/panda_box/README.md

@@ -25,7 +25,7 @@ class PandaState(StrEnum):
 There are a couple of methods which are tagged as USER_ACCESS methods, and thereby also available on the proxy devices. 
 These methods include:
  - `send_raw(cmd: Union[str, list[str]]) -> Any` : Send raw commands or lists of commands to the PandaBox hardware.
- - `add_status_callback(status: StatusBase, success: list[PandaState], failure: list[PandaState], check_directly: bool = True) -> str` : Register a callback to resolve status objects based on PandaBox events. PandaBox events are defined in the `PandaState` enum, which includes states like READY, START, FRAME, END, and DISARMED. These states correspond to different stages of that data acquisition of the PCAP module of the PandaBox. 
+ - `add_status_callback(status: StatusBase, success: list[PandaState], failure: list[PandaState], check_directly: bool = True) -> str` : Register a callback to resolve status objects based on PandaBox events. PandaBox events are defined in the `PandaState` enum, which includes states like READY, START, FRAME, END, and DISARMED. These states correspond to different stages of the data acquisition of the PCAP module of the PandaBox. 
 - `remove_status_callback(cb_id: str) -> None` : Remove a registered status callback using its unique callback ID (str) which is returned by *add_status_callback*.
 - `add_data_callback(callback: Callable[[LITERAL_PANDA_DATA], None], data_type: PandaState = PandaState.FRAME.value) -> str` : Register a callback for processing PandaBox data. The callback function is called when data of the specified type (READY, START, FRAME, END, DATA) is received from the PandaBox. The default data type is FRAME, which corresponds to actual frame data from the PCAP module. These data frames can be inspected in pandablocks.response module.
 - `remove_data_callback(cb_id: str) -> None` : Remove a registered data callback using its unique callback ID (str) which is returned by *add_data_callback*.
@@ -33,7 +33,7 @@ These methods include:
 
 ### Other useful methods
 
-- `convert_frame_data(frame_data: FrameData, signal_name_key_mapping: dict[str, str] | None = None) -> dict[str, Any]` : Convert FrameData from PandaBox into a dictionary format compatible with Ophyd signals. Optionally map PandaBox signal names to custom signal names.
+- `convert_frame_data(frame_data: FrameData) -> dict[str, Any]` : Convert FrameData from PandaBox into a dictionary format compatible with Ophyd signals, using the device's configured signal aliases.
 - `_get_signal_names_allowed_for_capture() -> list[str]` : Get a list of all signal keys that can be configured for capture on the PandaBox.
 - `_get_signal_names_configured_for_capture() -> list[str]` : Get a list of all signal keys that are currently configured for capture on the PandaBox.
 
@@ -52,4 +52,4 @@ Saves the current layout from the PandaBox at the specified host to a local file
 ``` bash
 python ./utility_scripts.py --host panda-box-host.psi.ch --load-layout ./my_layout.ini
 ```
-**IMPORTANT**: Loads the layout from the local file `my_layout.ini` to the PandaBox at the specified host. Please note that loading a layout will overwrite the current configuration on the PandaBox. The UI will partly update, but the WEB server needs to be restarted manually to reflect these changes properly. We expect beamlines to prepare and test layouts beforehands and not use the PandaBox web interface in operation. All dynamic configuration should be done through the ophyd device hooks either directly in the device integration or temporarily through custom scan implementations.
+**IMPORTANT**: Loads the layout from the local file `my_layout.ini` to the PandaBox at the specified host. Please note that loading a layout will overwrite the current configuration on the PandaBox. The UI will partly update, but the WEB server needs to be restarted manually to reflect these changes properly. We expect beamlines to prepare and test layouts beforehand and not use the PandaBox web interface in operation. All dynamic configuration should be done through the ophyd device hooks either directly in the device integration or temporarily through custom scan implementations.

ophyd_devices/devices/panda_box/panda_box.py

@@ -428,18 +428,22 @@ class PandaBox(PSIDeviceBase):
             with BlockingClient(self.host) as client:
                 for data in client.data(scaled=False):
                     if isinstance(data, ReadyData):
+                        logger.info("PandaBox is ready for data acquisition.")
                         self._run_status_callbacks(PandaState.READY)
                         self._run_data_callbacks(data, PandaState.READY)
 
                     elif isinstance(data, StartData):
+                        logger.info("PandaBox has started data acquisition.")
                         self._run_status_callbacks(PandaState.START)
                         self._run_data_callbacks(data, PandaState.START)
 
                     elif isinstance(data, FrameData):
+                        logger.info("PandaBox has received a frame of data.")
                         self._run_status_callbacks(PandaState.FRAME)
                         self._run_data_callbacks(data, PandaState.FRAME)
 
                     elif isinstance(data, EndData):
+                        logger.info("PandaBox has ended data acquisition.")
                         self._run_status_callbacks(PandaState.END)
                         self._run_data_callbacks(data, PandaState.END)
                         break  # Exit data readout loop
@@ -543,6 +547,11 @@ class PandaBox(PSIDeviceBase):
         """
         # Test connection by sending WHO command which should respond with PandaBox ID
         super().on_connected()
+        if self.data_thread.is_alive():
+            logger.warning(
+                "Data thread is already running. On Connected probably called multiple times."
+            )
+            return
         self.data_thread.start()
         self.add_data_callback(data_type=PandaState.FRAME, callback=self._receive_frame_data)
 
@@ -593,7 +602,7 @@ class PandaBox(PSIDeviceBase):
         """
         On pre_scan hook for the PandaBox. We use this hook to arm the PCAP module for data acquisition.
         This logic makes sure that the data readout loop is started and that we received the READY event
-        from the device. Only then can the PCAP module aquire data.
+        from the device. Only then can the PCAP module acquire data.
         """
         status = StatusBase(obj=self)
         status.add_callback(self._pre_scan_status_callback)
@@ -620,14 +629,14 @@ class PandaBox(PSIDeviceBase):
         return [key.split(" ")[0].strip("!") for key in ret if key.strip(".")]
 
     def _get_signal_names_configured_for_capture(self) -> list[str]:
-        """Utility method to get a list of all signal keys thar ARE CURRENTLY CONFIGURED for capture on the PandaBox."""
+        """Utility method to get a list of all signal keys that ARE CURRENTLY CONFIGURED for capture on the PandaBox."""
         ret = self.send_raw("*CAPTURE?")
         signal_names = []
         for value in ret:
             if value.strip("."):  # Ignore empty values "."
                 string_parts = value.strip("!").split(" ")
                 base_name = string_parts[0]  # Get base name without capture config
-                _ = [signal_names.append(f"{base_name}.{key}") for key in string_parts[1:]]
+                signal_names.extend(f"{base_name}.{key}" for key in string_parts[1:])
         return signal_names
 
     def convert_frame_data(self, frame_data: FrameData) -> dict[str, Any]:

ophyd_devices/devices/panda_box/utility_scripts.py

@@ -47,9 +47,6 @@ def main() -> None:
     elif args.load_layout is not None:
         load_layout_from_file_to_panda(host=args.host, file_path=args.load_layout)
 
-    else:
-        parser.print_help()
-
 
 if __name__ == "__main__":
     main()

ophyd_devices/devices/panda_box/utils.py

@@ -46,5 +46,9 @@ def get_pcap_capture_fields():
     out = []
     for block in PANDA_AVAIL_PCAP_BLOCKS:
         for field in PANDA_AVAIL_PCAP_CAPTURE_FIELDS:
+            # Consider this mapping, and alsock
+            # block_name = f"{block}.{field}"
+            # block_name = block.replace(".", "_")
+            # out.append(block_name) TODO - If applied Adapt 'convert_frame_data' method in panda_box.py to handle this mapping
             out.append(f"{block}.{field}")
     return out

tests/test_panda.py

@@ -50,6 +50,7 @@ def test_panda_wait_for_connection(panda_box):
 def test_panda_on_connected(panda_box):
     """Test that on_connected sets the connected flag."""
     with mock.patch.object(panda_box, "data_thread") as mock_data_thread:
+        mock_data_thread.is_alive.return_value = False
         panda_box.on_connected()
         mock_data_thread.start.assert_called_once()
         assert len(panda_box._data_callbacks) == 1
@@ -61,6 +62,12 @@ def test_panda_on_connected(panda_box):
         panda_box.remove_data_callback(cb_id)
         assert len(panda_box._data_callbacks) == 0, "Data callback was not removed"
 
+        # Call on_connected again, should add the callback again
+        mock_data_thread.reset_mock()
+        mock_data_thread.is_alive.return_value = True
+        panda_box.on_connected()
+        mock_data_thread.start.assert_not_called()
+
 
 def test_panda_add_status_callback(panda_box):
     """Test that add_status_callback adds proper status callbacks, and resolves them correctly."""
@@ -119,6 +126,7 @@ def test_panda_receive_frame_data(panda_box, _signal_aliases):
     fdata = FrameData(data)
     # Use on_connected to set up data callback
     with mock.patch.object(panda_box, "data_thread") as mock_data_thread:
+        mock_data_thread.is_alive.return_value = False
         panda_box.on_connected()  # This will set up the data callback
         mock_data_thread.start.assert_called_once()