From bafba1639f89629444583af4a2570a9c585e55ac Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 16:05:54 +0000 Subject: [PATCH 01/10] feat: add initial start and stop writing top level commands --- src/fastcs_odin/odin_controller.py | 41 ++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/fastcs_odin/odin_controller.py b/src/fastcs_odin/odin_controller.py index 24aaaf7..8c468c1 100644 --- a/src/fastcs_odin/odin_controller.py +++ b/src/fastcs_odin/odin_controller.py @@ -1,7 +1,13 @@ +import asyncio +import re +from collections.abc import Sequence +from functools import cached_property + from fastcs.attributes import AttrR from fastcs.connections.ip_connection import IPConnectionSettings from fastcs.controller import Controller from fastcs.datatypes import Bool, Float, Int, String +from fastcs.wrappers import command from fastcs_odin.eiger_fan import EigerFanAdapterController from fastcs_odin.frame_processor import FrameProcessorAdapterController @@ -11,6 +17,7 @@ from fastcs_odin.odin_adapter_controller import ( OdinAdapterController, StatusSummaryUpdater, + _filter_sub_controllers, ) from fastcs_odin.util import AdapterType, OdinParameter, create_odin_parameters @@ -36,6 +43,40 @@ def __init__(self, settings: IPConnectionSettings) -> None: self.connection = HTTPConnection(settings.ip, settings.port) + def _collect_commands( + self, + path_filter: Sequence[str | tuple[str, ...] | re.Pattern], + command_name: str, + ): + commands = [] + + controllers = list(_filter_sub_controllers(self, path_filter)) + + for controller in controllers: + if cmd := getattr(controller, command_name, None): + commands.append(cmd) + return commands + + @cached_property + def _start_writing_commands(self): + return self._collect_commands(("FP", re.compile("FP*"), "HDF"), "start_writing") + + @cached_property + def _stop_writing_commands(self): + return self._collect_commands(("FP", re.compile("FP*"), "HDF"), "stop_writing") + + @command() + async def start_writing(self) -> None: + await asyncio.gather( + *(start_writing() for start_writing in self._start_writing_commands) + ) + + @command() + async def stop_writing(self) -> None: + await asyncio.gather( + *(stop_writing() for stop_writing in self._stop_writing_commands) + ) + async def initialise(self) -> None: self.connection.open() From c15c94ce0e8542d69c5bb61d8dab70efbf0988e1 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 16:43:14 +0000 Subject: [PATCH 02/10] chore: move commands down to FP level and raise exception if command not found --- src/fastcs_odin/frame_processor.py | 45 +++++++++++++++++++++++++++++- src/fastcs_odin/odin_controller.py | 41 --------------------------- 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/fastcs_odin/frame_processor.py b/src/fastcs_odin/frame_processor.py index 32761d9..551fcf2 100644 --- a/src/fastcs_odin/frame_processor.py +++ b/src/fastcs_odin/frame_processor.py @@ -1,15 +1,18 @@ +import asyncio import re from collections.abc import Sequence -from functools import partial +from functools import cached_property, partial from fastcs.attributes import AttrR from fastcs.cs_methods import Command from fastcs.datatypes import Bool, Int +from fastcs.wrappers import command from pydantic import ValidationError from fastcs_odin.odin_adapter_controller import ( OdinAdapterController, StatusSummaryUpdater, + _filter_sub_controllers, ) from fastcs_odin.odin_data import OdinDataAdapterController, OdinDataController from fastcs_odin.util import AllowedCommandsResponse, OdinParameter, partition @@ -78,6 +81,46 @@ class FrameProcessorAdapterController(OdinDataAdapterController): _subcontroller_label = "FP" _subcontroller_cls = FrameProcessorController + def _collect_commands( + self, + path_filter: Sequence[str | tuple[str, ...] | re.Pattern], + command_name: str, + ): + commands = [] + + controllers = list(_filter_sub_controllers(self, path_filter)) + + for controller in controllers: + try: + cmd = getattr(controller, command_name) + commands.append(cmd) + except AttributeError as err: + raise AttributeError( + f"Sub controller {controller} " + f"does not have command '{command_name}'." + ) from err + return commands + + @cached_property + def _start_writing_commands(self): + return self._collect_commands((re.compile("FP*"), "HDF"), "hi") + + @cached_property + def _stop_writing_commands(self): + return self._collect_commands((re.compile("FP*"), "HDF"), "stop_writing") + + @command() + async def start_writing(self) -> None: + await asyncio.gather( + *(start_writing() for start_writing in self._start_writing_commands) + ) + + @command() + async def stop_writing(self) -> None: + await asyncio.gather( + *(stop_writing() for stop_writing in self._stop_writing_commands) + ) + class FrameProcessorPluginController(OdinAdapterController): """SubController for a plugin in a frameProcessor application.""" diff --git a/src/fastcs_odin/odin_controller.py b/src/fastcs_odin/odin_controller.py index 8c468c1..24aaaf7 100644 --- a/src/fastcs_odin/odin_controller.py +++ b/src/fastcs_odin/odin_controller.py @@ -1,13 +1,7 @@ -import asyncio -import re -from collections.abc import Sequence -from functools import cached_property - from fastcs.attributes import AttrR from fastcs.connections.ip_connection import IPConnectionSettings from fastcs.controller import Controller from fastcs.datatypes import Bool, Float, Int, String -from fastcs.wrappers import command from fastcs_odin.eiger_fan import EigerFanAdapterController from fastcs_odin.frame_processor import FrameProcessorAdapterController @@ -17,7 +11,6 @@ from fastcs_odin.odin_adapter_controller import ( OdinAdapterController, StatusSummaryUpdater, - _filter_sub_controllers, ) from fastcs_odin.util import AdapterType, OdinParameter, create_odin_parameters @@ -43,40 +36,6 @@ def __init__(self, settings: IPConnectionSettings) -> None: self.connection = HTTPConnection(settings.ip, settings.port) - def _collect_commands( - self, - path_filter: Sequence[str | tuple[str, ...] | re.Pattern], - command_name: str, - ): - commands = [] - - controllers = list(_filter_sub_controllers(self, path_filter)) - - for controller in controllers: - if cmd := getattr(controller, command_name, None): - commands.append(cmd) - return commands - - @cached_property - def _start_writing_commands(self): - return self._collect_commands(("FP", re.compile("FP*"), "HDF"), "start_writing") - - @cached_property - def _stop_writing_commands(self): - return self._collect_commands(("FP", re.compile("FP*"), "HDF"), "stop_writing") - - @command() - async def start_writing(self) -> None: - await asyncio.gather( - *(start_writing() for start_writing in self._start_writing_commands) - ) - - @command() - async def stop_writing(self) -> None: - await asyncio.gather( - *(stop_writing() for stop_writing in self._stop_writing_commands) - ) - async def initialise(self) -> None: self.connection.open() From ea9a3e4ce4a062319e59c87cf000b0d8a6f3d792 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 17:32:34 +0000 Subject: [PATCH 03/10] refactor: add exception handling for attribute not found in StatusSummaryUpdater --- src/fastcs_odin/odin_adapter_controller.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/fastcs_odin/odin_adapter_controller.py b/src/fastcs_odin/odin_adapter_controller.py index 2737ea4..28f3e26 100644 --- a/src/fastcs_odin/odin_adapter_controller.py +++ b/src/fastcs_odin/odin_adapter_controller.py @@ -97,13 +97,19 @@ class StatusSummaryUpdater(AttrHandlerR, Generic[In, Out]): async def initialise(self, controller): self.controller = controller - self.attributes: Sequence[AttrR[In]] = [ - attr - for sub_controller in _filter_sub_controllers( - self.controller, self.path_filter - ) - if isinstance(attr := sub_controller.attributes[self.attribute_name], AttrR) - ] + self.attributes: Sequence[AttrR[In]] = [] + for sub_controller in _filter_sub_controllers( + self.controller, self.path_filter + ): + try: + attr = sub_controller.attributes[self.attribute_name] + except KeyError as err: + raise KeyError( + f"Sub controller {sub_controller} " + f"does not have attribute '{self.attribute_name}'." + ) from err + if isinstance(attr, AttrR): + self.attributes.append(attr) async def update(self, attr: AttrR): values = [attribute.get() for attribute in self.attributes] From f9c8145efe83478189baa3e6b44b0eb998247370 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 17:33:00 +0000 Subject: [PATCH 04/10] tests: add test for attribute not found exception --- tests/test_controllers.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/test_controllers.py b/tests/test_controllers.py index a4880a9..55a5ee8 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -390,7 +390,7 @@ async def test_status_summary_updater(mocker: MockerFixture): @pytest.mark.asyncio @pytest.mark.parametrize("mock_sub_controller", ("FP", ("FP",), re.compile("FP"))) -async def test_status_summary_updater_raise_exception( +async def test_status_summary_updater_raise_exception_if_controller_not_found( mock_sub_controller, mocker: MockerFixture ): controller = mocker.MagicMock() @@ -401,6 +401,20 @@ async def test_status_summary_updater_raise_exception( await handler.initialise(controller) +@pytest.mark.asyncio +async def test_status_summary_updater_raise_exception_if_attribute_not_found( + mocker: MockerFixture, +): + controller = mocker.MagicMock() + controller.get_sub_controllers.return_value = { + "OD": OdinController(mocker.MagicMock()) + } + + handler = StatusSummaryUpdater(["OD"], "some_attribute", any) + with pytest.raises(KeyError, match="does not have attribute"): + await handler.initialise(controller) + + @pytest.mark.asyncio async def test_config_fan_sender(mocker: MockerFixture): controller = mocker.MagicMock() From 048d83fb53faa89bc79911aad330a3abef1bdaf5 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 7 Nov 2025 17:42:01 +0000 Subject: [PATCH 05/10] chore: amend path_filter for start_writing --- src/fastcs_odin/frame_processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastcs_odin/frame_processor.py b/src/fastcs_odin/frame_processor.py index 551fcf2..5a8de74 100644 --- a/src/fastcs_odin/frame_processor.py +++ b/src/fastcs_odin/frame_processor.py @@ -103,7 +103,7 @@ def _collect_commands( @cached_property def _start_writing_commands(self): - return self._collect_commands((re.compile("FP*"), "HDF"), "hi") + return self._collect_commands((re.compile("FP*"), "HDF"), "start_writing") @cached_property def _stop_writing_commands(self): From 72c76598e8b0d3d5b18aebbec84d46cf8a362e2d Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 10 Nov 2025 12:01:43 +0000 Subject: [PATCH 06/10] tests: add tests for top level FP commands --- tests/test_controllers.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/test_controllers.py b/tests/test_controllers.py index 55a5ee8..dc08e85 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -468,3 +468,40 @@ async def test_frame_reciever_controllers(): assert invalid_decoder_parameter not in decoder_controller.parameters # index, status, decoder parts removed from path assert decoder_controller.parameters[0]._path == ["packets_dropped"] + + +@pytest.mark.asyncio +async def test_frame_processor_start_and_stop_writing(mocker: MockerFixture): + fpac = FrameProcessorAdapterController( + mocker.AsyncMock(), mocker.AsyncMock(), "api/0.1" + ) + fpc = FrameProcessorController(mocker.AsyncMock(), mocker.AsyncMock(), "api/0.1") + await fpc._create_plugin_sub_controllers(["hdf"]) + + # Mock the commands to check calls + hdf = fpc.get_sub_controllers()["HDF"] + hdf.start_writing = mocker.AsyncMock() # type: ignore + hdf.stop_writing = mocker.AsyncMock() # type: ignore + + fpac.register_sub_controller("FP0", fpc) + + # Top level FP commands should collect and call lower level commands + await fpac.start_writing() + await fpac.stop_writing() + assert len(hdf.start_writing.mock_calls) == 1 # type: ignore + assert len(hdf.stop_writing.mock_calls) == 1 # type: ignore + + +@pytest.mark.asyncio +async def test_top_frame_processor_commands_raise_exception(mocker: MockerFixture): + mock_connection = mocker.AsyncMock() + fpac = FrameProcessorAdapterController( + mock_connection, mocker.AsyncMock(), "api/0.1" + ) + + fpc = FrameProcessorController(mocker.AsyncMock(), mocker.AsyncMock(), "api/0.1") + await fpc._create_plugin_sub_controllers(["hdf"]) + fpac.register_sub_controller("FP0", fpc) + + with pytest.raises(AttributeError, match="does not have"): + await fpac.start_writing() From 17ed6ad2e2e8b447789b360d6cb51ea358a965f2 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 10 Nov 2025 13:08:05 +0000 Subject: [PATCH 07/10] chore: clean typo and unused line --- tests/test_controllers.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_controllers.py b/tests/test_controllers.py index dc08e85..25e4bcc 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -402,7 +402,7 @@ async def test_status_summary_updater_raise_exception_if_controller_not_found( @pytest.mark.asyncio -async def test_status_summary_updater_raise_exception_if_attribute_not_found( +async def test_status_summary_updater_raises_exception_if_attribute_not_found( mocker: MockerFixture, ): controller = mocker.MagicMock() @@ -493,10 +493,11 @@ async def test_frame_processor_start_and_stop_writing(mocker: MockerFixture): @pytest.mark.asyncio -async def test_top_frame_processor_commands_raise_exception(mocker: MockerFixture): - mock_connection = mocker.AsyncMock() +async def test_top_level_frame_processor_commands_raise_exception( + mocker: MockerFixture, +): fpac = FrameProcessorAdapterController( - mock_connection, mocker.AsyncMock(), "api/0.1" + mocker.AsyncMock(), mocker.AsyncMock(), "api/0.1" ) fpc = FrameProcessorController(mocker.AsyncMock(), mocker.AsyncMock(), "api/0.1") From 78e77b9fcba6787117cb0742784c5f19392ed7a9 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Tue, 18 Nov 2025 15:29:33 +0000 Subject: [PATCH 08/10] refactor: reimplement check for attribute not found --- .../io/status_summary_attribute_io.py | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/fastcs_odin/io/status_summary_attribute_io.py b/src/fastcs_odin/io/status_summary_attribute_io.py index 23dded3..178aa13 100644 --- a/src/fastcs_odin/io/status_summary_attribute_io.py +++ b/src/fastcs_odin/io/status_summary_attribute_io.py @@ -58,16 +58,19 @@ def initialise_summary_attributes(controller): for attribute in controller.attributes.values(): if isinstance(attribute.io_ref, StatusSummaryAttributeIORef): - attributes = [ - attr - for sub_controller in _filter_sub_controllers( - controller, attribute.io_ref.path_filter - ) - if isinstance( - attr := sub_controller.attributes[attribute.io_ref.attribute_name], - AttrR, - ) - ] + attributes: Sequence[AttrR] = [] + for sub_controller in _filter_sub_controllers( + controller, attribute.io_ref.path_filter + ): + try: + attr = sub_controller.attributes[attribute.io_ref.attribute_name] + except KeyError as err: + raise KeyError( + f"Sub controller {sub_controller} " + f"does not have attribute '{attribute.io_ref.attribute_name}'." + ) from err + if isinstance(attr, AttrR): + attributes.append(attr) attribute.io_ref.set_attributes(attributes) From 8524c828c0b0073406919d377cbad062fed47d68 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Tue, 18 Nov 2025 15:29:51 +0000 Subject: [PATCH 09/10] tests: reimplement test for attribute not found error --- tests/test_controllers.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_controllers.py b/tests/test_controllers.py index ceb9d3f..1dcd4b6 100644 --- a/tests/test_controllers.py +++ b/tests/test_controllers.py @@ -486,3 +486,17 @@ async def test_top_level_frame_processor_commands_raise_exception( with pytest.raises(AttributeError, match="does not have"): await fpac.start_writing() + + +@pytest.mark.asyncio +async def test_status_summary_updater_raises_exception_if_attribute_not_found(): + controller = Controller() + sub_controller = Controller() + + controller.add_sub_controller("OD", sub_controller) + + controller.writing = AttrR( + Bool(), StatusSummaryAttributeIORef(["OD"], "some_attribute", any) + ) + with pytest.raises(KeyError, match=r"Sub controller .* does not have attribute"): + initialise_summary_attributes(controller) From 441feb2d1ac217509db6ca49e23bc0e5c683ded9 Mon Sep 17 00:00:00 2001 From: Gary Yendell Date: Thu, 20 Nov 2025 12:06:53 +0000 Subject: [PATCH 10/10] Update error message --- src/fastcs_odin/frame_processor.py | 4 ++-- src/fastcs_odin/io/status_summary_attribute_io.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/fastcs_odin/frame_processor.py b/src/fastcs_odin/frame_processor.py index ef12b99..108ed46 100644 --- a/src/fastcs_odin/frame_processor.py +++ b/src/fastcs_odin/frame_processor.py @@ -119,8 +119,8 @@ def _collect_commands( commands.append(cmd) except AttributeError as err: raise AttributeError( - f"Sub controller {controller} " - f"does not have command '{command_name}'." + f"Sub controller {controller} does not have command " + f"'{command_name}' required by {self} command fan out {path_filter}" ) from err return commands diff --git a/src/fastcs_odin/io/status_summary_attribute_io.py b/src/fastcs_odin/io/status_summary_attribute_io.py index 178aa13..fa7e249 100644 --- a/src/fastcs_odin/io/status_summary_attribute_io.py +++ b/src/fastcs_odin/io/status_summary_attribute_io.py @@ -66,8 +66,9 @@ def initialise_summary_attributes(controller): attr = sub_controller.attributes[attribute.io_ref.attribute_name] except KeyError as err: raise KeyError( - f"Sub controller {sub_controller} " - f"does not have attribute '{attribute.io_ref.attribute_name}'." + f"Sub controller {sub_controller} does not have attribute " + f"'{attribute.io_ref.attribute_name}' required by {controller} " + f"status summary {attribute.io_ref.path_filter}" ) from err if isinstance(attr, AttrR): attributes.append(attr)