From ba2c274cd66643e78c04bea09116e398717be089 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 20 Mar 2025 09:51:07 +0000 Subject: [PATCH 01/17] Add blocking flag to command pvs to wait for callback to set PACT false --- src/fastcs/transport/epics/ca/ioc.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index ac9ca00d7..abf5fad1a 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -238,8 +238,7 @@ async def wrapped_method(_: Any): await method() record = builder.Action( - f"{pv_prefix}:{pv_name}", - on_update=wrapped_method, + f"{pv_prefix}:{pv_name}", on_update=wrapped_method, blocking=True ) _add_attr_pvi_info(record, pv_prefix, attr_name, "x") From 3debec02856e35d213f986b4de0c296f518e90e1 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 20 Mar 2025 10:01:35 +0000 Subject: [PATCH 02/17] Fix assertions in unit tests --- tests/transport/epics/ca/test_softioc.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/transport/epics/ca/test_softioc.py b/tests/transport/epics/ca/test_softioc.py index 9c758bfea..b27703d49 100644 --- a/tests/transport/epics/ca/test_softioc.py +++ b/tests/transport/epics/ca/test_softioc.py @@ -288,7 +288,9 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): epics_controller_api.attributes["write_bool"].datatype ), ) - ioc_builder.Action.assert_any_call(f"{DEVICE}:Go", on_update=mocker.ANY) + ioc_builder.Action.assert_any_call( + f"{DEVICE}:Go", on_update=mocker.ANY, blocking=True + ) # Check info tags are added add_pvi_info.assert_called_once_with(f"{DEVICE}:PVI") @@ -474,8 +476,7 @@ def test_long_pv_names_discarded(mocker: MockerFixture): short_command_pv_name = "command_short_name".title().replace("_", "") ioc_builder.Action.assert_called_once_with( - f"{DEVICE}:{short_command_pv_name}", - on_update=mocker.ANY, + f"{DEVICE}:{short_command_pv_name}", on_update=mocker.ANY, blocking=True ) with pytest.raises(AssertionError): long_command_pv_name = long_command_name.title().replace("_", "") From 3d9d613f5cca90c9d6753104a66c76cd1ad601c5 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 20 Mar 2025 10:14:45 +0000 Subject: [PATCH 03/17] Change setuptools version to check deprecation warning --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d36507ce6..3cb035df0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [build-system] -requires = ["setuptools>=64", "setuptools_scm[toml]>=8"] +requires = ["setuptools>=70.1", "setuptools_scm[toml]>=8"] build-backend = "setuptools.build_meta" [project] From f76d924200974ab7c77c905895e41cfd31c252f4 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 21 Mar 2025 12:48:31 +0000 Subject: [PATCH 04/17] Pass command mode flag from Command --- src/fastcs/cs_methods.py | 22 +++++++++++++++++-- src/fastcs/transport/epics/ca/ioc.py | 12 +++++----- .../transport/epics/pva/_pv_handlers.py | 15 ++++++++----- src/fastcs/transport/epics/pva/ioc.py | 2 +- 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/fastcs/cs_methods.py b/src/fastcs/cs_methods.py index 5bcd9a267..65e1740df 100644 --- a/src/fastcs/cs_methods.py +++ b/src/fastcs/cs_methods.py @@ -1,3 +1,4 @@ +import enum from asyncio import iscoroutinefunction from collections.abc import Callable, Coroutine from inspect import Signature, getdoc, signature @@ -74,6 +75,16 @@ def group(self): return self._group +class CommandMode(enum.Enum): + #: Command value becomes `True` in the transport once + #: the command starts, becomes `False` after it finishes. + HIGH_AFTER_START = "HIGH_AFTER_START" + + #: Command value becomes `True` in the transport only after + #: the command has finished. Becomes `False` immediately after. + HIGH_AFTER_FINISH = "HIGH_AFTER_FINISH" + + class Command(Method[BaseController]): """A `Controller` `Method` that performs a single action when called. @@ -82,7 +93,14 @@ class Command(Method[BaseController]): Calling an instance of this class will call the bound `Controller` method. """ - def __init__(self, fn: CommandCallback, *, group: str | None = None): + def __init__( + self, + fn: CommandCallback, + *, + group: str | None = None, + mode: CommandMode = CommandMode.HIGH_AFTER_START, + ): + self.mode = mode super().__init__(fn, group=group) def _validate(self, fn: CommandCallback) -> None: @@ -92,7 +110,7 @@ def _validate(self, fn: CommandCallback) -> None: raise FastCSException(f"Command method cannot have arguments: {fn}") async def __call__(self): - return await self._fn() + return await self._fn(), self.mode class Scan(Method[BaseController]): diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index abf5fad1a..0f1724734 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -8,6 +8,7 @@ from fastcs.attributes import AttrR, AttrRW, AttrW from fastcs.controller_api import ControllerAPI +from fastcs.cs_methods import CommandMode from fastcs.datatypes import DataType, T from fastcs.transport.epics.ca.util import ( builder_callable_from_attribute, @@ -224,21 +225,20 @@ def _create_and_link_command_pvs( method.enabled = False else: _create_and_link_command_pv( - _pv_prefix, - pv_name, - attr_name, - method.fn, + _pv_prefix, pv_name, attr_name, method.fn, method.mode ) def _create_and_link_command_pv( - pv_prefix: str, pv_name: str, attr_name: str, method: Callable + pv_prefix: str, pv_name: str, attr_name: str, method: Callable, mode: CommandMode ) -> None: async def wrapped_method(_: Any): await method() record = builder.Action( - f"{pv_prefix}:{pv_name}", on_update=wrapped_method, blocking=True + f"{pv_prefix}:{pv_name}", + on_update=wrapped_method, + blocking=True if mode == CommandMode.HIGH_AFTER_FINISH else False, ) _add_attr_pvi_info(record, pv_prefix, attr_name, "x") diff --git a/src/fastcs/transport/epics/pva/_pv_handlers.py b/src/fastcs/transport/epics/pva/_pv_handlers.py index 801d4e63e..6749ad1fb 100644 --- a/src/fastcs/transport/epics/pva/_pv_handlers.py +++ b/src/fastcs/transport/epics/pva/_pv_handlers.py @@ -7,7 +7,7 @@ from p4p.server.asyncio import SharedPV from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW -from fastcs.cs_methods import CommandCallback +from fastcs.cs_methods import CommandCallback, CommandMode from fastcs.datatypes import Table from .types import ( @@ -51,8 +51,9 @@ async def put(self, pv: SharedPV, op: ServerOperation): class CommandPvHandler: - def __init__(self, command: CommandCallback): + def __init__(self, command: CommandCallback, mode: CommandMode): self._command = command + self._mode = mode self._task_in_progress = False async def _run_command(self) -> dict: @@ -81,11 +82,13 @@ async def put(self, pv: SharedPV, op: ServerOperation): "Maybe the command should spawn an asyncio task?" ) - # Flip to true once command task starts pv.post({"value": True, **p4p_timestamp_now(), **p4p_alarm_states()}) - op.done() + if self._mode == CommandMode.HIGH_AFTER_START: + op.done() alarm_states = await self._run_command() pv.post({"value": False, **p4p_timestamp_now(), **alarm_states}) + if self._mode == CommandMode.HIGH_AFTER_FINISH: + op.done() else: raise RuntimeError("Commands should only take the value `True`.") @@ -123,7 +126,7 @@ async def on_update(value): return shared_pv -def make_command_pv(command: CommandCallback) -> SharedPV: +def make_command_pv(command: CommandCallback, mode: CommandMode) -> SharedPV: type_ = NTScalar.buildType("?", display=True, control=True) initial = Value(type_, {"value": False, **p4p_alarm_states()}) @@ -133,7 +136,7 @@ def _wrap(value: dict): shared_pv = SharedPV( initial=initial, - handler=CommandPvHandler(command), + handler=CommandPvHandler(command, mode), wrap=_wrap, ) diff --git a/src/fastcs/transport/epics/pva/ioc.py b/src/fastcs/transport/epics/pva/ioc.py index 923dc618e..eb637922a 100644 --- a/src/fastcs/transport/epics/pva/ioc.py +++ b/src/fastcs/transport/epics/pva/ioc.py @@ -55,7 +55,7 @@ async def parse_attributes( for attr_name, method in controller_api.command_methods.items(): pv_name = get_pv_name(pv_prefix, attr_name) - command_pv = make_command_pv(method.fn) + command_pv = make_command_pv(method.fn, method.mode) provider.add(pv_name, command_pv) pvi_tree.add_signal(pv_name, "x") From 49310cc75841b698a884d55f92ea08da54da1e9b Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 21 Mar 2025 13:04:38 +0000 Subject: [PATCH 05/17] Add flag to command wrapper --- src/fastcs/cs_methods.py | 9 +++++++-- src/fastcs/wrappers.py | 5 +++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/fastcs/cs_methods.py b/src/fastcs/cs_methods.py index 65e1740df..8461e4c08 100644 --- a/src/fastcs/cs_methods.py +++ b/src/fastcs/cs_methods.py @@ -168,8 +168,13 @@ class UnboundCommand(Method[Controller_T]): """ def __init__( - self, fn: UnboundCommandCallback[Controller_T], *, group: str | None = None + self, + fn: UnboundCommandCallback[Controller_T], + *, + group: str | None = None, + mode: CommandMode = CommandMode.HIGH_AFTER_START, ) -> None: + self.mode = mode super().__init__(fn, group=group) def _validate(self, fn: UnboundCommandCallback[Controller_T]) -> None: @@ -179,7 +184,7 @@ def _validate(self, fn: UnboundCommandCallback[Controller_T]) -> None: raise FastCSException("Command method cannot have arguments") def bind(self, controller: Controller_T) -> Command: - return Command(MethodType(self.fn, controller)) + return Command(MethodType(self.fn, controller), mode=self.mode) def __call__(self): raise method_not_bound_error diff --git a/src/fastcs/wrappers.py b/src/fastcs/wrappers.py index 4462a559d..538b0952f 100644 --- a/src/fastcs/wrappers.py +++ b/src/fastcs/wrappers.py @@ -1,6 +1,7 @@ from collections.abc import Callable from .cs_methods import ( + CommandMode, Controller_T, UnboundCommand, UnboundCommandCallback, @@ -32,7 +33,7 @@ def put(fn: UnboundPutCallback[Controller_T]) -> UnboundPut[Controller_T]: def command( - *, group: str | None = None + *, group: str | None = None, mode: CommandMode = CommandMode.HIGH_AFTER_START ) -> Callable[[UnboundCommandCallback[Controller_T]], UnboundCommand[Controller_T]]: """Decorator to tag a `Controller` method to be turned into a `Command`. @@ -44,6 +45,6 @@ def command( def wrapper( fn: UnboundCommandCallback[Controller_T], ) -> UnboundCommand[Controller_T]: - return UnboundCommand(fn, group=group) + return UnboundCommand(fn, group=group, mode=mode) return wrapper From aef08c56b7e78fe50a3bb8a6f1fe49627fe73413 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 21 Mar 2025 13:11:40 +0000 Subject: [PATCH 06/17] Amend tests to follow default behaviour of HIGH_AFTER_START --- tests/transport/epics/ca/test_softioc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/transport/epics/ca/test_softioc.py b/tests/transport/epics/ca/test_softioc.py index b27703d49..2c639c9ce 100644 --- a/tests/transport/epics/ca/test_softioc.py +++ b/tests/transport/epics/ca/test_softioc.py @@ -289,7 +289,7 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): ), ) ioc_builder.Action.assert_any_call( - f"{DEVICE}:Go", on_update=mocker.ANY, blocking=True + f"{DEVICE}:Go", on_update=mocker.ANY, blocking=False ) # Check info tags are added @@ -476,7 +476,7 @@ def test_long_pv_names_discarded(mocker: MockerFixture): short_command_pv_name = "command_short_name".title().replace("_", "") ioc_builder.Action.assert_called_once_with( - f"{DEVICE}:{short_command_pv_name}", on_update=mocker.ANY, blocking=True + f"{DEVICE}:{short_command_pv_name}", on_update=mocker.ANY, blocking=False ) with pytest.raises(AssertionError): long_command_pv_name = long_command_name.title().replace("_", "") From d4cf2a3cf745fdb64f99f0209699db9af8c01855 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 21 Mar 2025 13:17:10 +0000 Subject: [PATCH 07/17] Remove returning mode on call dunder --- src/fastcs/cs_methods.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastcs/cs_methods.py b/src/fastcs/cs_methods.py index 8461e4c08..bfe3a6bd6 100644 --- a/src/fastcs/cs_methods.py +++ b/src/fastcs/cs_methods.py @@ -110,7 +110,7 @@ def _validate(self, fn: CommandCallback) -> None: raise FastCSException(f"Command method cannot have arguments: {fn}") async def __call__(self): - return await self._fn(), self.mode + return await self._fn() class Scan(Method[BaseController]): From 5750ad3656a0e01f5e8ee049e5ece9425dc96bf7 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 24 Mar 2025 12:02:22 +0000 Subject: [PATCH 08/17] Revert commits addressing flag blocking --- pyproject.toml | 2 +- src/fastcs/cs_methods.py | 29 ++----------------- src/fastcs/transport/epics/ca/ioc.py | 12 ++++---- .../transport/epics/pva/_pv_handlers.py | 15 ++++------ src/fastcs/transport/epics/pva/ioc.py | 2 +- src/fastcs/wrappers.py | 5 ++-- tests/transport/epics/ca/test_softioc.py | 4 +-- 7 files changed, 21 insertions(+), 48 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3cb035df0..d36507ce6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,5 @@ [build-system] -requires = ["setuptools>=70.1", "setuptools_scm[toml]>=8"] +requires = ["setuptools>=64", "setuptools_scm[toml]>=8"] build-backend = "setuptools.build_meta" [project] diff --git a/src/fastcs/cs_methods.py b/src/fastcs/cs_methods.py index bfe3a6bd6..5bcd9a267 100644 --- a/src/fastcs/cs_methods.py +++ b/src/fastcs/cs_methods.py @@ -1,4 +1,3 @@ -import enum from asyncio import iscoroutinefunction from collections.abc import Callable, Coroutine from inspect import Signature, getdoc, signature @@ -75,16 +74,6 @@ def group(self): return self._group -class CommandMode(enum.Enum): - #: Command value becomes `True` in the transport once - #: the command starts, becomes `False` after it finishes. - HIGH_AFTER_START = "HIGH_AFTER_START" - - #: Command value becomes `True` in the transport only after - #: the command has finished. Becomes `False` immediately after. - HIGH_AFTER_FINISH = "HIGH_AFTER_FINISH" - - class Command(Method[BaseController]): """A `Controller` `Method` that performs a single action when called. @@ -93,14 +82,7 @@ class Command(Method[BaseController]): Calling an instance of this class will call the bound `Controller` method. """ - def __init__( - self, - fn: CommandCallback, - *, - group: str | None = None, - mode: CommandMode = CommandMode.HIGH_AFTER_START, - ): - self.mode = mode + def __init__(self, fn: CommandCallback, *, group: str | None = None): super().__init__(fn, group=group) def _validate(self, fn: CommandCallback) -> None: @@ -168,13 +150,8 @@ class UnboundCommand(Method[Controller_T]): """ def __init__( - self, - fn: UnboundCommandCallback[Controller_T], - *, - group: str | None = None, - mode: CommandMode = CommandMode.HIGH_AFTER_START, + self, fn: UnboundCommandCallback[Controller_T], *, group: str | None = None ) -> None: - self.mode = mode super().__init__(fn, group=group) def _validate(self, fn: UnboundCommandCallback[Controller_T]) -> None: @@ -184,7 +161,7 @@ def _validate(self, fn: UnboundCommandCallback[Controller_T]) -> None: raise FastCSException("Command method cannot have arguments") def bind(self, controller: Controller_T) -> Command: - return Command(MethodType(self.fn, controller), mode=self.mode) + return Command(MethodType(self.fn, controller)) def __call__(self): raise method_not_bound_error diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index 0f1724734..abf5fad1a 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -8,7 +8,6 @@ from fastcs.attributes import AttrR, AttrRW, AttrW from fastcs.controller_api import ControllerAPI -from fastcs.cs_methods import CommandMode from fastcs.datatypes import DataType, T from fastcs.transport.epics.ca.util import ( builder_callable_from_attribute, @@ -225,20 +224,21 @@ def _create_and_link_command_pvs( method.enabled = False else: _create_and_link_command_pv( - _pv_prefix, pv_name, attr_name, method.fn, method.mode + _pv_prefix, + pv_name, + attr_name, + method.fn, ) def _create_and_link_command_pv( - pv_prefix: str, pv_name: str, attr_name: str, method: Callable, mode: CommandMode + pv_prefix: str, pv_name: str, attr_name: str, method: Callable ) -> None: async def wrapped_method(_: Any): await method() record = builder.Action( - f"{pv_prefix}:{pv_name}", - on_update=wrapped_method, - blocking=True if mode == CommandMode.HIGH_AFTER_FINISH else False, + f"{pv_prefix}:{pv_name}", on_update=wrapped_method, blocking=True ) _add_attr_pvi_info(record, pv_prefix, attr_name, "x") diff --git a/src/fastcs/transport/epics/pva/_pv_handlers.py b/src/fastcs/transport/epics/pva/_pv_handlers.py index 6749ad1fb..801d4e63e 100644 --- a/src/fastcs/transport/epics/pva/_pv_handlers.py +++ b/src/fastcs/transport/epics/pva/_pv_handlers.py @@ -7,7 +7,7 @@ from p4p.server.asyncio import SharedPV from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW -from fastcs.cs_methods import CommandCallback, CommandMode +from fastcs.cs_methods import CommandCallback from fastcs.datatypes import Table from .types import ( @@ -51,9 +51,8 @@ async def put(self, pv: SharedPV, op: ServerOperation): class CommandPvHandler: - def __init__(self, command: CommandCallback, mode: CommandMode): + def __init__(self, command: CommandCallback): self._command = command - self._mode = mode self._task_in_progress = False async def _run_command(self) -> dict: @@ -82,13 +81,11 @@ async def put(self, pv: SharedPV, op: ServerOperation): "Maybe the command should spawn an asyncio task?" ) + # Flip to true once command task starts pv.post({"value": True, **p4p_timestamp_now(), **p4p_alarm_states()}) - if self._mode == CommandMode.HIGH_AFTER_START: - op.done() + op.done() alarm_states = await self._run_command() pv.post({"value": False, **p4p_timestamp_now(), **alarm_states}) - if self._mode == CommandMode.HIGH_AFTER_FINISH: - op.done() else: raise RuntimeError("Commands should only take the value `True`.") @@ -126,7 +123,7 @@ async def on_update(value): return shared_pv -def make_command_pv(command: CommandCallback, mode: CommandMode) -> SharedPV: +def make_command_pv(command: CommandCallback) -> SharedPV: type_ = NTScalar.buildType("?", display=True, control=True) initial = Value(type_, {"value": False, **p4p_alarm_states()}) @@ -136,7 +133,7 @@ def _wrap(value: dict): shared_pv = SharedPV( initial=initial, - handler=CommandPvHandler(command, mode), + handler=CommandPvHandler(command), wrap=_wrap, ) diff --git a/src/fastcs/transport/epics/pva/ioc.py b/src/fastcs/transport/epics/pva/ioc.py index eb637922a..923dc618e 100644 --- a/src/fastcs/transport/epics/pva/ioc.py +++ b/src/fastcs/transport/epics/pva/ioc.py @@ -55,7 +55,7 @@ async def parse_attributes( for attr_name, method in controller_api.command_methods.items(): pv_name = get_pv_name(pv_prefix, attr_name) - command_pv = make_command_pv(method.fn, method.mode) + command_pv = make_command_pv(method.fn) provider.add(pv_name, command_pv) pvi_tree.add_signal(pv_name, "x") diff --git a/src/fastcs/wrappers.py b/src/fastcs/wrappers.py index 538b0952f..4462a559d 100644 --- a/src/fastcs/wrappers.py +++ b/src/fastcs/wrappers.py @@ -1,7 +1,6 @@ from collections.abc import Callable from .cs_methods import ( - CommandMode, Controller_T, UnboundCommand, UnboundCommandCallback, @@ -33,7 +32,7 @@ def put(fn: UnboundPutCallback[Controller_T]) -> UnboundPut[Controller_T]: def command( - *, group: str | None = None, mode: CommandMode = CommandMode.HIGH_AFTER_START + *, group: str | None = None ) -> Callable[[UnboundCommandCallback[Controller_T]], UnboundCommand[Controller_T]]: """Decorator to tag a `Controller` method to be turned into a `Command`. @@ -45,6 +44,6 @@ def command( def wrapper( fn: UnboundCommandCallback[Controller_T], ) -> UnboundCommand[Controller_T]: - return UnboundCommand(fn, group=group, mode=mode) + return UnboundCommand(fn, group=group) return wrapper diff --git a/tests/transport/epics/ca/test_softioc.py b/tests/transport/epics/ca/test_softioc.py index 2c639c9ce..b27703d49 100644 --- a/tests/transport/epics/ca/test_softioc.py +++ b/tests/transport/epics/ca/test_softioc.py @@ -289,7 +289,7 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): ), ) ioc_builder.Action.assert_any_call( - f"{DEVICE}:Go", on_update=mocker.ANY, blocking=False + f"{DEVICE}:Go", on_update=mocker.ANY, blocking=True ) # Check info tags are added @@ -476,7 +476,7 @@ def test_long_pv_names_discarded(mocker: MockerFixture): short_command_pv_name = "command_short_name".title().replace("_", "") ioc_builder.Action.assert_called_once_with( - f"{DEVICE}:{short_command_pv_name}", on_update=mocker.ANY, blocking=False + f"{DEVICE}:{short_command_pv_name}", on_update=mocker.ANY, blocking=True ) with pytest.raises(AssertionError): long_command_pv_name = long_command_name.title().replace("_", "") From a7a792cb61d7b709068f7a3ad80ac087d6777559 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 24 Mar 2025 12:03:22 +0000 Subject: [PATCH 09/17] Change order of pva op done --- src/fastcs/transport/epics/pva/_pv_handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastcs/transport/epics/pva/_pv_handlers.py b/src/fastcs/transport/epics/pva/_pv_handlers.py index 801d4e63e..6361636e1 100644 --- a/src/fastcs/transport/epics/pva/_pv_handlers.py +++ b/src/fastcs/transport/epics/pva/_pv_handlers.py @@ -83,9 +83,9 @@ async def put(self, pv: SharedPV, op: ServerOperation): # Flip to true once command task starts pv.post({"value": True, **p4p_timestamp_now(), **p4p_alarm_states()}) - op.done() alarm_states = await self._run_command() pv.post({"value": False, **p4p_timestamp_now(), **alarm_states}) + op.done() else: raise RuntimeError("Commands should only take the value `True`.") From 112f4877e3327b4a6130ae5a19042a631a36617f Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 24 Mar 2025 15:21:58 +0000 Subject: [PATCH 10/17] Fix p4p test --- src/fastcs/transport/epics/ca/ioc.py | 4 +++- tests/transport/epics/pva/test_p4p.py | 12 ++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index abf5fad1a..36e4ca014 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -238,7 +238,9 @@ async def wrapped_method(_: Any): await method() record = builder.Action( - f"{pv_prefix}:{pv_name}", on_update=wrapped_method, blocking=True + f"{pv_prefix}:{pv_name}", + on_update=wrapped_method, + blocking=True, ) _add_attr_pvi_info(record, pv_prefix, attr_name, "x") diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index ce192998d..61d75dc47 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -574,11 +574,15 @@ async def some_task(): async def put_pvs(): await asyncio.sleep(0.1) ctxt = Context("pva") - await ctxt.put(f"{pv_prefix}:CommandSpawnsATask", True) - await ctxt.put(f"{pv_prefix}:CommandSpawnsATask", True) - await ctxt.put(f"{pv_prefix}:CommandRunsForAWhile", True) + await asyncio.gather( + ctxt.put(f"{pv_prefix}:CommandSpawnsATask", True), + ctxt.put(f"{pv_prefix}:CommandSpawnsATask", True), + ) assert expected_error_string not in caplog.text - await ctxt.put(f"{pv_prefix}:CommandRunsForAWhile", True) + await asyncio.gather( + ctxt.put(f"{pv_prefix}:CommandRunsForAWhile", True), + ctxt.put(f"{pv_prefix}:CommandRunsForAWhile", True), + ) assert expected_error_string in caplog.text serve = asyncio.ensure_future(fastcs.serve()) From 44300ff0b22322efa1bd8e5e7231486eab920984 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 26 Mar 2025 18:16:16 +0000 Subject: [PATCH 11/17] Add record option blocking check in pva --- src/fastcs/transport/epics/pva/_pv_handlers.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/fastcs/transport/epics/pva/_pv_handlers.py b/src/fastcs/transport/epics/pva/_pv_handlers.py index 6361636e1..35d997479 100644 --- a/src/fastcs/transport/epics/pva/_pv_handlers.py +++ b/src/fastcs/transport/epics/pva/_pv_handlers.py @@ -73,6 +73,7 @@ async def _run_command(self) -> dict: async def put(self, pv: SharedPV, op: ServerOperation): value = op.value() raw_value = value["value"] + blocking = False if raw_value is True: if self._task_in_progress: @@ -81,11 +82,20 @@ async def put(self, pv: SharedPV, op: ServerOperation): "Maybe the command should spawn an asyncio task?" ) + # Try to retrieve block option from record field + requests = op.pvRequest() + if requests: + record_options = requests.get("record", {}).get("_options", {}) + blocking = record_options.get("block") == "true" + # Flip to true once command task starts pv.post({"value": True, **p4p_timestamp_now(), **p4p_alarm_states()}) + if not blocking: + op.done() alarm_states = await self._run_command() pv.post({"value": False, **p4p_timestamp_now(), **alarm_states}) - op.done() + if blocking: + op.done() else: raise RuntimeError("Commands should only take the value `True`.") From f6746e4cf9e52333eed8c9c848de31a3ddad2b3b Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 28 Mar 2025 12:08:53 +0000 Subject: [PATCH 12/17] Convert request to dict and check if block flag true, and add test --- .../transport/epics/pva/_pv_handlers.py | 12 +++--- tests/transport/epics/pva/test_p4p.py | 42 +++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/fastcs/transport/epics/pva/_pv_handlers.py b/src/fastcs/transport/epics/pva/_pv_handlers.py index 35d997479..db6a1c6aa 100644 --- a/src/fastcs/transport/epics/pva/_pv_handlers.py +++ b/src/fastcs/transport/epics/pva/_pv_handlers.py @@ -73,7 +73,6 @@ async def _run_command(self) -> dict: async def put(self, pv: SharedPV, op: ServerOperation): value = op.value() raw_value = value["value"] - blocking = False if raw_value is True: if self._task_in_progress: @@ -82,11 +81,12 @@ async def put(self, pv: SharedPV, op: ServerOperation): "Maybe the command should spawn an asyncio task?" ) - # Try to retrieve block option from record field - requests = op.pvRequest() - if requests: - record_options = requests.get("record", {}).get("_options", {}) - blocking = record_options.get("block") == "true" + # Check if record block request recieved + match op.pvRequest().todict(): + case {"record": {"_options": {"block": "true"}}}: + blocking = True + case _: + blocking = False # Flip to true once command task starts pv.post({"value": True, **p4p_timestamp_now(), **p4p_alarm_states()}) diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index 61d75dc47..bde499440 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -627,3 +627,45 @@ async def put_pvs(): pytest.approx((coro_end_time - coro_start_time).total_seconds(), abs=0.05) == 0.1 ) + + +def test_block_flag_waits_for_callback_completion(): + class SomeController(Controller): + @command() + async def command_runs_for_a_while(self): + await asyncio.sleep(0.2) + + controller = SomeController() + pv_prefix = str(uuid4()) + fastcs = make_fastcs(pv_prefix, controller) + command_runs_for_a_while_times = [] + + async def put_pvs(): + ctxt = Context("pva") + start_time = datetime.now() + await ctxt.put( + f"{pv_prefix}:CommandRunsForAWhile", True, request="record[block=true]" + ) + command_runs_for_a_while_times.append((start_time, datetime.now())) + await ctxt.put( + f"{pv_prefix}:CommandRunsForAWhile", True, request="record[block=false]" + ) + command_runs_for_a_while_times.append((start_time, datetime.now())) + + serve = asyncio.ensure_future(fastcs.serve()) + try: + asyncio.get_event_loop().run_until_complete( + asyncio.wait_for( + asyncio.gather(serve, put_pvs()), + timeout=0.5, + ) + ) + except TimeoutError: + ... + serve.cancel() + + assert len(command_runs_for_a_while_times) == 2 + + for i in range(2): + start, end = command_runs_for_a_while_times[i] + assert pytest.approx((end - start).total_seconds(), abs=0.05) == 0.2 From 862ea4cf1c1b43a2ea966168932498d38817afe2 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 28 Mar 2025 12:24:35 +0000 Subject: [PATCH 13/17] Add iocinit flag to disable pva serving --- src/fastcs/transport/epics/ca/ioc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index 36e4ca014..493acfbbb 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -47,7 +47,7 @@ def run( ) -> None: dispatcher = AsyncioDispatcher(loop) # Needs running loop builder.LoadDatabase() - softioc.iocInit(dispatcher) + softioc.iocInit(dispatcher, enable_pva=False) def _add_pvi_info( From 8ccc575ea3fef1488d757e800bf8111eb847cb89 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 28 Mar 2025 13:24:50 +0000 Subject: [PATCH 14/17] Revert "Add iocinit flag to disable pva serving" This reverts commit 862ea4cf1c1b43a2ea966168932498d38817afe2. --- src/fastcs/transport/epics/ca/ioc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index 493acfbbb..36e4ca014 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -47,7 +47,7 @@ def run( ) -> None: dispatcher = AsyncioDispatcher(loop) # Needs running loop builder.LoadDatabase() - softioc.iocInit(dispatcher, enable_pva=False) + softioc.iocInit(dispatcher) def _add_pvi_info( From e43f555338e6c8100f1d31ad77763586aa10ced4 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 28 Mar 2025 14:12:26 +0000 Subject: [PATCH 15/17] Ignore private import from PVI --- src/fastcs/transport/epics/gui.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastcs/transport/epics/gui.py b/src/fastcs/transport/epics/gui.py index d08f8ce5e..ff586def1 100644 --- a/src/fastcs/transport/epics/gui.py +++ b/src/fastcs/transport/epics/gui.py @@ -1,4 +1,4 @@ -from pvi._format.dls import DLSFormatter +from pvi._format.dls import DLSFormatter # type: ignore from pvi.device import ( LED, ButtonPanel, From e08621f345ab084e593f61472d4411ac28adb040 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 31 Mar 2025 09:16:49 +0000 Subject: [PATCH 16/17] Make unit test clearer --- tests/transport/epics/pva/test_p4p.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index bde499440..668fd51da 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -642,15 +642,14 @@ async def command_runs_for_a_while(self): async def put_pvs(): ctxt = Context("pva") - start_time = datetime.now() - await ctxt.put( - f"{pv_prefix}:CommandRunsForAWhile", True, request="record[block=true]" - ) - command_runs_for_a_while_times.append((start_time, datetime.now())) - await ctxt.put( - f"{pv_prefix}:CommandRunsForAWhile", True, request="record[block=false]" - ) - command_runs_for_a_while_times.append((start_time, datetime.now())) + for block in ["true", "false"]: + start_time = datetime.now() + await ctxt.put( + f"{pv_prefix}:CommandRunsForAWhile", + True, + request=f"record[block={block}]", + ) + command_runs_for_a_while_times.append((start_time, datetime.now())) serve = asyncio.ensure_future(fastcs.serve()) try: @@ -666,6 +665,8 @@ async def put_pvs(): assert len(command_runs_for_a_while_times) == 2 - for i in range(2): - start, end = command_runs_for_a_while_times[i] - assert pytest.approx((end - start).total_seconds(), abs=0.05) == 0.2 + for put_call, expected_duration in enumerate([0.2, 0]): + start, end = command_runs_for_a_while_times[put_call] + assert ( + pytest.approx((end - start).total_seconds(), abs=0.05) == expected_duration + ) From 72aeb86271ba479a8c69358cc45f50a45963062a Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Mon, 31 Mar 2025 11:06:39 +0000 Subject: [PATCH 17/17] Use ctxt wait instead of request --- tests/transport/epics/pva/test_p4p.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index 668fd51da..bd98c4a80 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -642,12 +642,12 @@ async def command_runs_for_a_while(self): async def put_pvs(): ctxt = Context("pva") - for block in ["true", "false"]: + for block in [True, False]: start_time = datetime.now() await ctxt.put( f"{pv_prefix}:CommandRunsForAWhile", True, - request=f"record[block={block}]", + wait=block, ) command_runs_for_a_while_times.append((start_time, datetime.now()))