From 468b91ba2131f38e8b9f9540afd017766845a085 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 18 Jul 2025 14:06:47 +0000 Subject: [PATCH 1/8] Seperate shared pv creation for _rbv signals --- .../transport/epics/pva/_pv_handlers.py | 50 +++++++++++++++++++ src/fastcs/transport/epics/pva/ioc.py | 24 +++++++-- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/fastcs/transport/epics/pva/_pv_handlers.py b/src/fastcs/transport/epics/pva/_pv_handlers.py index db6a1c6aa..29d64edd6 100644 --- a/src/fastcs/transport/epics/pva/_pv_handlers.py +++ b/src/fastcs/transport/epics/pva/_pv_handlers.py @@ -13,6 +13,7 @@ from .types import ( MAJOR_ALARM_SEVERITY, RECORD_ALARM_STATUS, + T, cast_from_p4p_value, cast_to_p4p_value, make_p4p_type, @@ -100,6 +101,55 @@ async def put(self, pv: SharedPV, op: ServerOperation): raise RuntimeError("Commands should only take the value `True`.") +def _make_shared_pv_arguments( + attribute: Attribute, initial_value: T +) -> dict[str, object]: + type_ = make_p4p_type(attribute) + kwargs = {"initial": cast_to_p4p_value(attribute, initial_value)} + if isinstance(type_, (NTEnum | NTNDArray | NTTable)): + kwargs["nt"] = type_ + else: + + def _wrap(value: dict): + return Value(type_, value) + + kwargs["wrap"] = _wrap + + return kwargs + + +def make_shared_read_pv(attribute: AttrR) -> SharedPV: + initial_value = attribute.get() + + kwargs = _make_shared_pv_arguments(attribute, initial_value) + + shared_pv = SharedPV(**kwargs) + + async def on_update(value): + shared_pv.post(cast_to_p4p_value(attribute, value)) + + attribute.add_update_callback(on_update) + + return shared_pv + + +def make_shared_write_pv(attribute: AttrW) -> SharedPV: + initial_value = attribute.datatype.initial_value + + kwargs = _make_shared_pv_arguments(attribute, initial_value) + + kwargs["handler"] = WritePvHandler(attribute) + + shared_pv = SharedPV(**kwargs) + + async def async_write_display(value): + shared_pv.post(cast_to_p4p_value(attribute, value)) + + attribute.add_write_display_callback(async_write_display) + + return shared_pv + + def make_shared_pv(attribute: Attribute) -> SharedPV: initial_value = ( attribute.get() diff --git a/src/fastcs/transport/epics/pva/ioc.py b/src/fastcs/transport/epics/pva/ioc.py index 8d9919712..4cbcc6f2c 100644 --- a/src/fastcs/transport/epics/pva/ioc.py +++ b/src/fastcs/transport/epics/pva/ioc.py @@ -6,7 +6,11 @@ from fastcs.controller_api import ControllerAPI from fastcs.util import snake_to_pascal -from ._pv_handlers import make_command_pv, make_shared_pv +from ._pv_handlers import ( + make_command_pv, + make_shared_read_pv, + make_shared_write_pv, +) from .pvi_tree import AccessModeType, PviTree @@ -42,9 +46,21 @@ async def parse_attributes( for attr_name, attribute in controller_api.attributes.items(): pv_name = get_pv_name(pv_prefix, attr_name) - attribute_pv = make_shared_pv(attribute) - provider.add(pv_name, attribute_pv) - pvi_tree.add_signal(pv_name, _attribute_to_access(attribute)) + match attribute: + case AttrRW(): + attribute_pv = make_shared_write_pv(attribute) + attribute_pv_rbv = make_shared_read_pv(attribute) + provider.add(pv_name, attribute_pv) + provider.add(f"{pv_name}_RBV", attribute_pv_rbv) + pvi_tree.add_signal(pv_name, "rw") + case AttrR(): + attribute_pv = make_shared_read_pv(attribute) + provider.add(pv_name, attribute_pv) + pvi_tree.add_signal(pv_name, "r") + case AttrW(): + attribute_pv_rbv = make_shared_write_pv(attribute) + provider.add(f"{pv_name}_RBV", attribute_pv_rbv) + pvi_tree.add_signal(pv_name, "w") for attr_name, method in controller_api.command_methods.items(): pv_name = get_pv_name(pv_prefix, attr_name) From 3971c7c4388a532ae158588f000cb2cd16e14624 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 18 Jul 2025 16:17:02 +0000 Subject: [PATCH 2/8] Amend tests to use _RBV --- tests/transport/epics/pva/test_p4p.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index 3f3a6c8d8..616485c6e 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -256,7 +256,7 @@ async def _wait_and_set_attr_r(): await controller.b.set(-0.9111111) a_values, b_values = [], [] - a_monitor = ctxt.monitor(f"{pv_prefix}:A", a_values.append) + a_monitor = ctxt.monitor(f"{pv_prefix}:A_RBV", a_values.append) b_monitor = ctxt.monitor(f"{pv_prefix}:B", b_values.append) serve = asyncio.ensure_future(fastcs.serve()) wait_and_set_attr_r = asyncio.ensure_future(_wait_and_set_attr_r()) @@ -464,10 +464,12 @@ async def _wait_and_put_pvs(): ) waveform_values, table_values, enum_values = [], [], [] - waveform_monitor = ctxt.monitor(f"{pv_prefix}:SomeWaveform", waveform_values.append) - table_monitor = ctxt.monitor(f"{pv_prefix}:SomeTable", table_values.append) + waveform_monitor = ctxt.monitor( + f"{pv_prefix}:SomeWaveform_RBV", waveform_values.append + ) + table_monitor = ctxt.monitor(f"{pv_prefix}:SomeTable_RBV", table_values.append) enum_monitor = ctxt.monitor( - f"{pv_prefix}:SomeEnum", + f"{pv_prefix}:SomeEnum_RBV", enum_values.append, ) serve = asyncio.ensure_future(fastcs.serve()) From 8378ed08ee25f53cfda3a19bb1840ea7084a45b5 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 18 Jul 2025 16:17:32 +0000 Subject: [PATCH 3/8] Fix AttrW wrong pv name --- src/fastcs/transport/epics/pva/ioc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fastcs/transport/epics/pva/ioc.py b/src/fastcs/transport/epics/pva/ioc.py index 4cbcc6f2c..d17e6083b 100644 --- a/src/fastcs/transport/epics/pva/ioc.py +++ b/src/fastcs/transport/epics/pva/ioc.py @@ -58,8 +58,8 @@ async def parse_attributes( provider.add(pv_name, attribute_pv) pvi_tree.add_signal(pv_name, "r") case AttrW(): - attribute_pv_rbv = make_shared_write_pv(attribute) - provider.add(f"{pv_name}_RBV", attribute_pv_rbv) + attribute_pv = make_shared_write_pv(attribute) + provider.add(pv_name, attribute_pv) pvi_tree.add_signal(pv_name, "w") for attr_name, method in controller_api.command_methods.items(): From 88c5c8df20836032a4ae9a16ffab5557b0598511 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 23 Jul 2025 11:48:03 +0000 Subject: [PATCH 4/8] fix: remove write pv handler pv post --- src/fastcs/transport/epics/pva/_pv_handlers.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/fastcs/transport/epics/pva/_pv_handlers.py b/src/fastcs/transport/epics/pva/_pv_handlers.py index 29d64edd6..35d007dc4 100644 --- a/src/fastcs/transport/epics/pva/_pv_handlers.py +++ b/src/fastcs/transport/epics/pva/_pv_handlers.py @@ -45,9 +45,6 @@ async def put(self, pv: SharedPV, op: ServerOperation): cast_value = cast_from_p4p_value(self._attr_w, raw_value) await self._attr_w.process_without_display_update(cast_value) - if type(self._attr_w) is AttrW: - # For AttrRW a post is done from the `_process_callback`. - pv.post(cast_to_p4p_value(self._attr_w, cast_value)) op.done() From d14f550266b5ded036c9b588ffe448d89cc15586 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 23 Jul 2025 15:24:48 +0000 Subject: [PATCH 5/8] Implement suggested API --- .../transport/epics/pva/_pv_handlers.py | 32 +++++++------------ 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/fastcs/transport/epics/pva/_pv_handlers.py b/src/fastcs/transport/epics/pva/_pv_handlers.py index 35d007dc4..001bae862 100644 --- a/src/fastcs/transport/epics/pva/_pv_handlers.py +++ b/src/fastcs/transport/epics/pva/_pv_handlers.py @@ -98,29 +98,23 @@ async def put(self, pv: SharedPV, op: ServerOperation): raise RuntimeError("Commands should only take the value `True`.") -def _make_shared_pv_arguments( - attribute: Attribute, initial_value: T -) -> dict[str, object]: +def _make_shared_pv_arguments(attribute: Attribute) -> dict[str, object]: type_ = make_p4p_type(attribute) - kwargs = {"initial": cast_to_p4p_value(attribute, initial_value)} if isinstance(type_, (NTEnum | NTNDArray | NTTable)): - kwargs["nt"] = type_ + return {"nt": type_} else: def _wrap(value: dict): return Value(type_, value) - kwargs["wrap"] = _wrap - - return kwargs + return {"wrap": _wrap} def make_shared_read_pv(attribute: AttrR) -> SharedPV: - initial_value = attribute.get() - - kwargs = _make_shared_pv_arguments(attribute, initial_value) - - shared_pv = SharedPV(**kwargs) + shared_pv = SharedPV( + initial=cast_to_p4p_value(attribute, attribute.get()), + **_make_shared_pv_arguments(attribute), + ) async def on_update(value): shared_pv.post(cast_to_p4p_value(attribute, value)) @@ -131,13 +125,11 @@ async def on_update(value): def make_shared_write_pv(attribute: AttrW) -> SharedPV: - initial_value = attribute.datatype.initial_value - - kwargs = _make_shared_pv_arguments(attribute, initial_value) - - kwargs["handler"] = WritePvHandler(attribute) - - shared_pv = SharedPV(**kwargs) + shared_pv = SharedPV( + handler=WritePvHandler(attribute), + initial=cast_to_p4p_value(attribute, attribute.datatype.initial_value), + **_make_shared_pv_arguments(attribute), + ) async def async_write_display(value): shared_pv.post(cast_to_p4p_value(attribute, value)) From 89ef55e1213f18c717c8c3350fe360c6e21bc779 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 23 Jul 2025 15:27:07 +0000 Subject: [PATCH 6/8] Remove old make shared pv function --- .../transport/epics/pva/_pv_handlers.py | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/src/fastcs/transport/epics/pva/_pv_handlers.py b/src/fastcs/transport/epics/pva/_pv_handlers.py index 001bae862..3f8c3609a 100644 --- a/src/fastcs/transport/epics/pva/_pv_handlers.py +++ b/src/fastcs/transport/epics/pva/_pv_handlers.py @@ -13,7 +13,6 @@ from .types import ( MAJOR_ALARM_SEVERITY, RECORD_ALARM_STATUS, - T, cast_from_p4p_value, cast_to_p4p_value, make_p4p_type, @@ -139,39 +138,6 @@ async def async_write_display(value): return shared_pv -def make_shared_pv(attribute: Attribute) -> SharedPV: - initial_value = ( - attribute.get() - if isinstance(attribute, AttrR) - else attribute.datatype.initial_value - ) - - type_ = make_p4p_type(attribute) - kwargs = {"initial": cast_to_p4p_value(attribute, initial_value)} - if isinstance(type_, (NTEnum | NTNDArray | NTTable)): - kwargs["nt"] = type_ - else: - - def _wrap(value: dict): - return Value(type_, value) - - kwargs["wrap"] = _wrap - - if isinstance(attribute, AttrW): - kwargs["handler"] = WritePvHandler(attribute) - - shared_pv = SharedPV(**kwargs) - - if isinstance(attribute, AttrR): - - async def on_update(value): - shared_pv.post(cast_to_p4p_value(attribute, value)) - - attribute.add_update_callback(on_update) - - return shared_pv - - def make_command_pv(command: CommandCallback) -> SharedPV: type_ = NTScalar.buildType("?", display=True, control=True) From a5eaa62d6267febcbfd37fe18f92472ad6695506 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 24 Jul 2025 10:43:42 +0000 Subject: [PATCH 7/8] Add comments to clarify put and set usage --- tests/transport/epics/pva/test_p4p.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index 616485c6e..ace63bb63 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -448,6 +448,8 @@ class SomeController(Controller): async def _wait_and_set_attrs(): await asyncio.sleep(0.1) + # This demonstrates an update from hardware, + # resulting in only a change in the read back. await asyncio.gather( controller.some_waveform.set(server_set_waveform_value), controller.some_table.set(server_set_table_value), @@ -457,6 +459,8 @@ async def _wait_and_set_attrs(): async def _wait_and_put_pvs(): await asyncio.sleep(0.3) ctxt = Context("pva") + # This demonstrates a client put, + # resulting in a change in the demand and read back. await asyncio.gather( ctxt.put(f"{pv_prefix}:SomeWaveform", client_put_waveform_value), ctxt.put(f"{pv_prefix}:SomeTable", client_put_table_value), From 325d670a4915989b6ccd3ca9514d4cf3917c6269 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 24 Jul 2025 10:46:43 +0000 Subject: [PATCH 8/8] Add comment to clarify monitoring RBVs --- tests/transport/epics/pva/test_p4p.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/transport/epics/pva/test_p4p.py b/tests/transport/epics/pva/test_p4p.py index ace63bb63..16479c199 100644 --- a/tests/transport/epics/pva/test_p4p.py +++ b/tests/transport/epics/pva/test_p4p.py @@ -468,6 +468,8 @@ async def _wait_and_put_pvs(): ) waveform_values, table_values, enum_values = [], [], [] + + # Monitoring read backs to capture both client and server sets. waveform_monitor = ctxt.monitor( f"{pv_prefix}:SomeWaveform_RBV", waveform_values.append ) @@ -476,6 +478,7 @@ async def _wait_and_put_pvs(): f"{pv_prefix}:SomeEnum_RBV", enum_values.append, ) + serve = asyncio.ensure_future(fastcs.serve()) wait_and_set_attrs = asyncio.ensure_future(_wait_and_set_attrs()) wait_and_put_pvs = asyncio.ensure_future(_wait_and_put_pvs())