From 898eb451072ef8d5ad34e33ffecd75997031094c Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Tue, 10 Feb 2026 15:39:24 +0000 Subject: [PATCH 01/11] Remove builder_callable_from_attribute method. --- src/fastcs/transports/epics/ca/ioc.py | 36 +++++++++++++++++--- src/fastcs/transports/epics/ca/util.py | 31 +---------------- tests/transports/epics/ca/test_ca_util.py | 18 ---------- tests/transports/epics/ca/test_softioc.py | 41 +++++++++++------------ 4 files changed, 53 insertions(+), 73 deletions(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index fbaaa993..6147278e 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -7,15 +7,15 @@ from softioc.pythonSoftIoc import RecordWrapper from fastcs.attributes import AttrR, AttrRW, AttrW -from fastcs.datatypes import DataType, DType_T -from fastcs.datatypes.waveform import Waveform +from fastcs.datatypes import Bool, DataType, DType_T, Enum, Float, Int, String, Waveform +from fastcs.exceptions import FastCSError from fastcs.logging import bind_logger from fastcs.methods import Command from fastcs.tracer import Tracer from fastcs.transports.controller_api import ControllerAPI from fastcs.transports.epics import EpicsIOCOptions from fastcs.transports.epics.ca.util import ( - builder_callable_from_attribute, + MBB_MAX_CHOICES, cast_from_epics_type, cast_to_epics_type, record_metadata_from_attribute, @@ -199,7 +199,35 @@ def _make_record( on_update: Callable | None = None, out_record: bool = False, ) -> RecordWrapper: - builder_callable = builder_callable_from_attribute(attribute, on_update is None) + match attribute.datatype: + case Bool(): + builder_callable = builder.boolIn if on_update is None else builder.boolOut + case Int(): + builder_callable = builder.longIn if on_update is None else builder.longOut + case Float(): + builder_callable = builder.aIn if on_update is None else builder.aOut + case String(): + builder_callable = ( + builder.longStringIn if on_update is None else builder.longStringOut + ) + case Enum(): + if len(attribute.datatype.members) > MBB_MAX_CHOICES: + builder_callable = ( + builder.longStringIn if on_update is None else builder.longStringOut + ) + else: + builder_callable = ( + builder.mbbIn if on_update is None else builder.mbbOut + ) + case Waveform(): + builder_callable = ( + builder.WaveformIn if on_update is None else builder.WaveformOut + ) + case _: + raise FastCSError( + f"EPICS unsupported datatype on {attribute}: {attribute.datatype}" + ) + datatype_record_metadata = record_metadata_from_datatype( attribute.datatype, out_record ) diff --git a/src/fastcs/transports/epics/ca/util.py b/src/fastcs/transports/epics/ca/util.py index 2f19d250..27464b84 100644 --- a/src/fastcs/transports/epics/ca/util.py +++ b/src/fastcs/transports/epics/ca/util.py @@ -2,12 +2,9 @@ from dataclasses import asdict from typing import Any -from softioc import builder - -from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW +from fastcs.attributes import Attribute, AttrR, AttrW from fastcs.datatypes import Bool, DType_T, Enum, Float, Int, String, Waveform from fastcs.datatypes.datatype import DataType -from fastcs.exceptions import FastCSError _MBB_FIELD_PREFIXES = ( "ZR", @@ -154,29 +151,3 @@ def cast_to_epics_type(datatype: DataType[DType_T], value: DType_T) -> Any: return value case _: raise ValueError(f"Unsupported datatype {datatype}") - - -def builder_callable_from_attribute( - attribute: AttrR | AttrW | AttrRW, make_in_record: bool -): - """Returns a callable to make the softioc record from an attribute instance.""" - match attribute.datatype: - case Bool(): - return builder.boolIn if make_in_record else builder.boolOut - case Int(): - return builder.longIn if make_in_record else builder.longOut - case Float(): - return builder.aIn if make_in_record else builder.aOut - case String(): - return builder.longStringIn if make_in_record else builder.longStringOut - case Enum(): - if len(attribute.datatype.members) > MBB_MAX_CHOICES: - return builder.longStringIn if make_in_record else builder.longStringOut - else: - return builder.mbbIn if make_in_record else builder.mbbOut - case Waveform(): - return builder.WaveformIn if make_in_record else builder.WaveformOut - case _: - raise FastCSError( - f"EPICS unsupported datatype on {attribute}: {attribute.datatype}" - ) diff --git a/tests/transports/epics/ca/test_ca_util.py b/tests/transports/epics/ca/test_ca_util.py index 2993b1e6..8488c41d 100644 --- a/tests/transports/epics/ca/test_ca_util.py +++ b/tests/transports/epics/ca/test_ca_util.py @@ -1,12 +1,9 @@ import enum import pytest -from softioc import builder -from fastcs.attributes import AttrRW from fastcs.datatypes import Bool, Enum, Float, Int, String from fastcs.transports.epics.ca.util import ( - builder_callable_from_attribute, cast_from_epics_type, cast_to_epics_type, record_metadata_from_datatype, @@ -137,21 +134,6 @@ def test_cast_from_epics_validations(datatype, input): cast_from_epics_type(datatype, input) -@pytest.mark.parametrize( - "datatype,in_record,out_record", - [ - (Enum(ShortEnum), builder.mbbIn, builder.mbbOut), - # long enums use string even if all values are ints - (Enum(LongEnum), builder.longStringIn, builder.longStringOut), - (Enum(LongMixedEnum), builder.longStringIn, builder.longStringOut), - ], -) -def test_builder_callable_enum_types(datatype, in_record, out_record): - attr = AttrRW(datatype) - assert builder_callable_from_attribute(attr, False) == out_record - assert builder_callable_from_attribute(attr, True) == in_record - - def test_drive_metadata_from_datatype(): dtype = Float(units="mm", min=-10.0, max=10.0, min_alarm=-5, max_alarm=5, prec=3) out_arguments = record_metadata_from_datatype(dtype, True) diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 27a53e25..41a42393 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -91,7 +91,7 @@ def test_make_input_record( kwargs: dict[str, Any], mocker: MockerFixture, ): - builder = mocker.patch("fastcs.transports.epics.ca.util.builder") + builder = mocker.patch("fastcs.transports.epics.ca.ioc.builder") pv = "PV" _make_record(pv, attribute) @@ -179,7 +179,7 @@ def test_make_output_record( kwargs: dict[str, Any], mocker: MockerFixture, ): - builder = mocker.patch("fastcs.transports.epics.ca.util.builder") + builder = mocker.patch("fastcs.transports.epics.ca.ioc.builder") update = mocker.MagicMock() pv = "PV" @@ -196,7 +196,7 @@ def test_make_output_record( def test_long_enum_validator(mocker: MockerFixture): - builder = mocker.patch("fastcs.transports.epics.ca.util.builder") + builder = mocker.patch("fastcs.transports.epics.ca.ioc.builder") update = mocker.MagicMock() attribute = AttrRW(Enum(LongEnum)) pv = "PV" @@ -230,7 +230,7 @@ def epics_controller_api(class_mocker: MockerFixture): def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): ioc_builder = mocker.patch("fastcs.transports.epics.ca.ioc.builder") - builder = mocker.patch("fastcs.transports.epics.ca.util.builder") + # builder = mocker.patch("fastcs.transports.epics.ca.util.builder") add_pvi_info = mocker.patch("fastcs.transports.epics.ca.ioc._add_pvi_info") add_sub_controller_pvi_info = mocker.patch( "fastcs.transports.epics.ca.ioc._add_sub_controller_pvi_info" @@ -239,21 +239,21 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): EpicsCAIOC(DEVICE, epics_controller_api) # Check records are created - builder.boolIn.assert_called_once_with( + ioc_builder.boolIn.assert_called_once_with( f"{DEVICE}:ReadBool", **record_metadata_from_attribute(epics_controller_api.attributes["read_bool"]), **record_metadata_from_datatype( epics_controller_api.attributes["read_bool"].datatype ), ) - builder.longIn.assert_any_call( + ioc_builder.longIn.assert_any_call( f"{DEVICE}:ReadInt", **record_metadata_from_attribute(epics_controller_api.attributes["read_int"]), **record_metadata_from_datatype( epics_controller_api.attributes["read_int"].datatype ), ) - builder.aIn.assert_called_once_with( + ioc_builder.aIn.assert_called_once_with( f"{DEVICE}:ReadWriteFloat_RBV", **record_metadata_from_attribute( epics_controller_api.attributes["read_write_float"] @@ -262,7 +262,7 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): epics_controller_api.attributes["read_write_float"].datatype ), ) - builder.aOut.assert_any_call( + ioc_builder.aOut.assert_any_call( f"{DEVICE}:ReadWriteFloat", always_update=True, blocking=True, @@ -275,7 +275,7 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): out_record=True, ), ) - builder.longIn.assert_any_call( + ioc_builder.longIn.assert_any_call( f"{DEVICE}:ReadWriteInt_RBV", **record_metadata_from_attribute( epics_controller_api.attributes["read_write_int"] @@ -284,7 +284,7 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): epics_controller_api.attributes["read_write_int"].datatype ), ) - builder.longOut.assert_called_with( + ioc_builder.longOut.assert_called_with( f"{DEVICE}:ReadWriteInt", always_update=True, blocking=True, @@ -296,14 +296,14 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): epics_controller_api.attributes["read_write_int"].datatype, out_record=True ), ) - builder.mbbIn.assert_called_once_with( + ioc_builder.mbbIn.assert_called_once_with( f"{DEVICE}:Enum_RBV", **record_metadata_from_attribute(epics_controller_api.attributes["enum"]), **record_metadata_from_datatype( epics_controller_api.attributes["enum"].datatype ), ) - builder.mbbOut.assert_called_once_with( + ioc_builder.mbbOut.assert_called_once_with( f"{DEVICE}:Enum", always_update=True, blocking=True, @@ -313,7 +313,7 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): epics_controller_api.attributes["enum"].datatype, out_record=True ), ) - builder.boolOut.assert_called_once_with( + ioc_builder.boolOut.assert_called_once_with( f"{DEVICE}:WriteBool", always_update=True, blocking=True, @@ -450,7 +450,6 @@ class ControllerLongNames(Controller): def test_long_pv_names_discarded(mocker: MockerFixture): ioc_builder = mocker.patch("fastcs.transports.epics.ca.ioc.builder") - builder = mocker.patch("fastcs.transports.epics.ca.util.builder") long_name_controller_api = AssertableControllerAPI(ControllerLongNames(), mocker) long_attr_name = "attr_r_with_reallyreallyreallyreallyreallyreallyreally_long_name" long_rw_name = "attr_rw_with_a_reallyreally_long_name_that_is_too_long_for_RBV" @@ -461,7 +460,7 @@ def test_long_pv_names_discarded(mocker: MockerFixture): assert not long_name_controller_api.attributes[long_attr_name].enabled short_pv_name = "attr_rw_short_name".title().replace("_", "") - builder.longOut.assert_called_once_with( + ioc_builder.longOut.assert_called_once_with( f"{DEVICE}:{short_pv_name}", always_update=True, blocking=True, @@ -474,7 +473,7 @@ def test_long_pv_names_discarded(mocker: MockerFixture): long_name_controller_api.attributes["attr_rw_short_name"] ), ) - builder.longIn.assert_called_once_with( + ioc_builder.longIn.assert_called_once_with( f"{DEVICE}:{short_pv_name}_RBV", **record_metadata_from_datatype( long_name_controller_api.attributes[ @@ -490,7 +489,7 @@ def test_long_pv_names_discarded(mocker: MockerFixture): long_pv_name = long_attr_name.title().replace("_", "") with pytest.raises(AssertionError): - builder.longIn.assert_called_once_with(f"{DEVICE}:{long_pv_name}") + ioc_builder.longIn.assert_called_once_with(f"{DEVICE}:{long_pv_name}") long_rw_pv_name = long_rw_name.title().replace("_", "") # neither the readback nor setpoint PV gets made if the full pv name with _RBV @@ -502,14 +501,14 @@ def test_long_pv_names_discarded(mocker: MockerFixture): ) with pytest.raises(AssertionError): - builder.longOut.assert_called_once_with( + ioc_builder.longOut.assert_called_once_with( f"{DEVICE}:{long_rw_pv_name}", always_update=True, blocking=True, on_update=mocker.ANY, ) with pytest.raises(AssertionError): - builder.longIn.assert_called_once_with(f"{DEVICE}:{long_rw_pv_name}_RBV") + ioc_builder.longIn.assert_called_once_with(f"{DEVICE}:{long_rw_pv_name}_RBV") assert long_name_controller_api.command_methods["command_short_name"].enabled long_command_name = ( @@ -528,7 +527,7 @@ def test_long_pv_names_discarded(mocker: MockerFixture): ) with pytest.raises(AssertionError): long_command_pv_name = long_command_name.title().replace("_", "") - builder.aOut.assert_called_once_with( + ioc_builder.aOut.assert_called_once_with( f"{DEVICE}:{long_command_pv_name}", initial_value=0, always_update=True, @@ -557,7 +556,7 @@ def test_non_1d_waveforms_discarded(mocker: MockerFixture): def test_update_datatype(mocker: MockerFixture): - builder = mocker.patch("fastcs.transports.epics.ca.util.builder") + builder = mocker.patch("fastcs.transports.epics.ca.ioc.builder") pv_name = f"{DEVICE}:Attr" From 08bfce59866e75009df309409181032b47a0a11d Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Tue, 10 Feb 2026 16:42:04 +0000 Subject: [PATCH 02/11] Split _make_record into _make_in_record and _make_out_record. --- src/fastcs/transports/epics/ca/ioc.py | 116 ++++++++++++++---- .../transports/epics/ca/test_initial_value.py | 8 +- tests/transports/epics/ca/test_softioc.py | 27 ++-- 3 files changed, 109 insertions(+), 42 deletions(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index 6147278e..7d04de54 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -187,64 +187,128 @@ async def async_record_set(value: DType_T): record.set(cast_to_epics_type(attribute.datatype, value)) - record = _make_record(pv, attribute) + record = _make_in_record(pv, attribute) _add_attr_pvi_info(record, pv_prefix, attr_name, "r") attribute.add_on_update_callback(async_record_set) -def _make_record( +def _make_in_record( pv: str, attribute: AttrR | AttrW | AttrRW, - on_update: Callable | None = None, - out_record: bool = False, ) -> RecordWrapper: + datatype_record_metadata = record_metadata_from_datatype(attribute.datatype) + attribute_record_metadata = record_metadata_from_attribute(attribute) + + update = {} + match attribute.datatype: case Bool(): - builder_callable = builder.boolIn if on_update is None else builder.boolOut + record = builder.boolIn( + pv, **update, **datatype_record_metadata, **attribute_record_metadata + ) case Int(): - builder_callable = builder.longIn if on_update is None else builder.longOut + record = builder.longIn( + pv, **update, **datatype_record_metadata, **attribute_record_metadata + ) case Float(): - builder_callable = builder.aIn if on_update is None else builder.aOut + record = builder.aIn( + pv, **update, **datatype_record_metadata, **attribute_record_metadata + ) case String(): - builder_callable = ( - builder.longStringIn if on_update is None else builder.longStringOut + record = builder.longStringIn( + pv, **update, **datatype_record_metadata, **attribute_record_metadata ) case Enum(): if len(attribute.datatype.members) > MBB_MAX_CHOICES: - builder_callable = ( - builder.longStringIn if on_update is None else builder.longStringOut + record = builder.longStringIn( + pv, + **update, + **datatype_record_metadata, + **attribute_record_metadata, ) else: - builder_callable = ( - builder.mbbIn if on_update is None else builder.mbbOut + record = builder.mbbIn( + pv, + **update, + **datatype_record_metadata, + **attribute_record_metadata, ) case Waveform(): - builder_callable = ( - builder.WaveformIn if on_update is None else builder.WaveformOut + record = builder.WaveformIn( + pv, **update, **datatype_record_metadata, **attribute_record_metadata ) case _: raise FastCSError( f"EPICS unsupported datatype on {attribute}: {attribute.datatype}" ) + def datatype_updater(datatype: DataType): + for name, value in record_metadata_from_datatype(datatype).items(): + record.set_field(name, value) + + attribute.add_update_datatype_callback(datatype_updater) + return record + + +def _make_out_record( + pv: str, + attribute: AttrR | AttrW | AttrRW, + on_update: Callable, +) -> RecordWrapper: datatype_record_metadata = record_metadata_from_datatype( - attribute.datatype, out_record + attribute.datatype, out_record=True ) attribute_record_metadata = record_metadata_from_attribute(attribute) - update = ( - {"on_update": on_update, "always_update": True, "blocking": True} - if on_update - else {} - ) + update = {"on_update": on_update, "always_update": True, "blocking": True} - record = builder_callable( - pv, **update, **datatype_record_metadata, **attribute_record_metadata - ) + match attribute.datatype: + case Bool(): + record = builder.boolOut( + pv, **update, **datatype_record_metadata, **attribute_record_metadata + ) + case Int(): + record = builder.longOut( + pv, **update, **datatype_record_metadata, **attribute_record_metadata + ) + case Float(): + record = builder.aOut( + pv, **update, **datatype_record_metadata, **attribute_record_metadata + ) + case String(): + record = builder.longStringOut( + pv, **update, **datatype_record_metadata, **attribute_record_metadata + ) + case Enum(): + if len(attribute.datatype.members) > MBB_MAX_CHOICES: + record = builder.longStringOut( + pv, + **update, + **datatype_record_metadata, + **attribute_record_metadata, + ) + + else: + record = builder.mbbOut( + pv, + **update, + **datatype_record_metadata, + **attribute_record_metadata, + ) + case Waveform(): + record = builder.WaveformOut( + pv, **update, **datatype_record_metadata, **attribute_record_metadata + ) + case _: + raise FastCSError( + f"EPICS unsupported datatype on {attribute}: {attribute.datatype}" + ) def datatype_updater(datatype: DataType): - for name, value in record_metadata_from_datatype(datatype, out_record).items(): + for name, value in record_metadata_from_datatype( + datatype, out_record=True + ).items(): record.set_field(name, value) attribute.add_update_datatype_callback(datatype_updater) @@ -268,7 +332,7 @@ async def set_setpoint_without_process(value: DType_T): record.set(cast_to_epics_type(attribute.datatype, value), process=False) - record = _make_record(pv, attribute, on_update=on_update, out_record=True) + record = _make_out_record(pv, attribute, on_update=on_update) _add_attr_pvi_info(record, pv_prefix, attr_name, "w") diff --git a/tests/transports/epics/ca/test_initial_value.py b/tests/transports/epics/ca/test_initial_value.py index e340b4c4..f3b43fe9 100644 --- a/tests/transports/epics/ca/test_initial_value.py +++ b/tests/transports/epics/ca/test_initial_value.py @@ -59,16 +59,18 @@ async def test_initial_values_set_in_ca(mocker): loop, ) - record_spy = mocker.spy(ca_ioc, "_make_record") + record_spy = mocker.spy(ca_ioc, "_make_in_record") + record_spy_out = mocker.spy(ca_ioc, "_make_out_record") task = asyncio.create_task(fastcs.serve(interactive=False)) try: async with asyncio.timeout(3): - while not record_spy.spy_return_list: + while not record_spy.spy_return_list and not record_spy_out.spy_return_list: await asyncio.sleep(0) initial_values = { - wrapper.name: wrapper.get() for wrapper in record_spy.spy_return_list + wrapper.name: wrapper.get() + for wrapper in record_spy.spy_return_list + record_spy_out.spy_return_list } for name, value in { "SOFTIOC_INITIAL_DEVICE:Bool": 1, diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 41a42393..1752bed6 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -27,7 +27,8 @@ _add_sub_controller_pvi_info, _create_and_link_read_pv, _create_and_link_write_pv, - _make_record, + _make_in_record, + _make_out_record, ) from fastcs.transports.epics.ca.util import ( record_metadata_from_attribute, @@ -46,7 +47,7 @@ class OnOffStates(enum.IntEnum): @pytest.mark.asyncio async def test_create_and_link_read_pv(mocker: MockerFixture): - make_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_record") + make_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_in_record") add_attr_pvi_info = mocker.patch( "fastcs.transports.epics.ca.ioc._add_attr_pvi_info" ) @@ -94,7 +95,7 @@ def test_make_input_record( builder = mocker.patch("fastcs.transports.epics.ca.ioc.builder") pv = "PV" - _make_record(pv, attribute) + _make_in_record(pv, attribute) kwargs.update(record_metadata_from_datatype(attribute.datatype)) kwargs.update(record_metadata_from_attribute(attribute)) @@ -105,14 +106,15 @@ def test_make_input_record( def test_make_record_raises(mocker: MockerFixture): + mocker.patch("fastcs.transports.epics.ca.ioc.record_metadata_from_datatype") # Pass a mock as attribute to provoke the fallback case matching on datatype with pytest.raises(FastCSError): - _make_record("PV", mocker.MagicMock()) + _make_in_record("PV", mocker.MagicMock()) @pytest.mark.asyncio async def test_create_and_link_write_pv(mocker: MockerFixture): - make_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_record") + make_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_out_record") add_attr_pvi_info = mocker.patch( "fastcs.transports.epics.ca.ioc._add_attr_pvi_info" ) @@ -124,9 +126,7 @@ async def test_create_and_link_write_pv(mocker: MockerFixture): _create_and_link_write_pv("PREFIX", "PV", "attr", attribute) - make_record.assert_called_once_with( - "PREFIX:PV", attribute, on_update=mocker.ANY, out_record=True - ) + make_record.assert_called_once_with("PREFIX:PV", attribute, on_update=mocker.ANY) add_attr_pvi_info.assert_called_once_with(record, "PREFIX", "attr", "w") # Extract the write update callback generated and set in the function and call it @@ -183,7 +183,7 @@ def test_make_output_record( update = mocker.MagicMock() pv = "PV" - _make_record(pv, attribute, on_update=update, out_record=True) + _make_out_record(pv, attribute, on_update=update) kwargs.update(record_metadata_from_datatype(attribute.datatype, out_record=True)) kwargs.update(record_metadata_from_attribute(attribute)) @@ -200,16 +200,17 @@ def test_long_enum_validator(mocker: MockerFixture): update = mocker.MagicMock() attribute = AttrRW(Enum(LongEnum)) pv = "PV" - record = _make_record(pv, attribute, on_update=update, out_record=True) + record = _make_out_record(pv, attribute, on_update=update) validator = builder.longStringOut.call_args.kwargs["validate"] assert validator(record, "THIS") # value is one of the Enum names assert not validator(record, "an invalid string value") def test_get_output_record_raises(mocker: MockerFixture): + mocker.patch("fastcs.transports.epics.ca.ioc.record_metadata_from_datatype") # Pass a mock as attribute to provoke the fallback case matching on datatype with pytest.raises(FastCSError): - _make_record("PV", mocker.MagicMock(), on_update=mocker.MagicMock()) + _make_out_record("PV", mocker.MagicMock(), on_update=mocker.MagicMock()) class EpicsController(MyTestController): @@ -561,7 +562,7 @@ def test_update_datatype(mocker: MockerFixture): pv_name = f"{DEVICE}:Attr" attr_r = AttrR(Int()) - record_r = _make_record(pv_name, attr_r) + record_r = _make_in_record(pv_name, attr_r) builder.longIn.assert_called_once_with( pv_name, @@ -580,7 +581,7 @@ def test_update_datatype(mocker: MockerFixture): attr_r.update_datatype(String()) # type: ignore attr_w = AttrW(Int()) - record_w = _make_record(pv_name, attr_w, on_update=mocker.ANY, out_record=True) + record_w = _make_out_record(pv_name, attr_w, on_update=mocker.ANY) builder.longIn.assert_called_once_with( pv_name, From 00a518c1cc765677552593f3b2fac5a2c9d4ccee Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Wed, 11 Feb 2026 11:12:03 +0000 Subject: [PATCH 03/11] Improve test coverage --- tests/transports/epics/ca/test_softioc.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 1752bed6..fb1d1570 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -206,6 +206,14 @@ def test_long_enum_validator(mocker: MockerFixture): assert not validator(record, "an invalid string value") +def test_long_enum_in_creation(mocker: MockerFixture): + builder = mocker.patch("fastcs.transports.epics.ca.ioc.builder") + attribute = AttrR(Enum(LongEnum)) + pv = "PV" + _make_in_record(pv, attribute) + assert builder.longStringIn.call_args.kwargs["initial_value"] == "THIS" + + def test_get_output_record_raises(mocker: MockerFixture): mocker.patch("fastcs.transports.epics.ca.ioc.record_metadata_from_datatype") # Pass a mock as attribute to provoke the fallback case matching on datatype From 7e13abacc27ba876173bd9dea6aa58f35f684f90 Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Wed, 11 Feb 2026 11:16:29 +0000 Subject: [PATCH 04/11] Removed empty update dict and commented out code. --- src/fastcs/transports/epics/ca/ioc.py | 14 +++++--------- tests/transports/epics/ca/test_softioc.py | 1 - 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index 7d04de54..9b9898a3 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -200,43 +200,39 @@ def _make_in_record( datatype_record_metadata = record_metadata_from_datatype(attribute.datatype) attribute_record_metadata = record_metadata_from_attribute(attribute) - update = {} - match attribute.datatype: case Bool(): record = builder.boolIn( - pv, **update, **datatype_record_metadata, **attribute_record_metadata + pv, **datatype_record_metadata, **attribute_record_metadata ) case Int(): record = builder.longIn( - pv, **update, **datatype_record_metadata, **attribute_record_metadata + pv, **datatype_record_metadata, **attribute_record_metadata ) case Float(): record = builder.aIn( - pv, **update, **datatype_record_metadata, **attribute_record_metadata + pv, **datatype_record_metadata, **attribute_record_metadata ) case String(): record = builder.longStringIn( - pv, **update, **datatype_record_metadata, **attribute_record_metadata + pv, **datatype_record_metadata, **attribute_record_metadata ) case Enum(): if len(attribute.datatype.members) > MBB_MAX_CHOICES: record = builder.longStringIn( pv, - **update, **datatype_record_metadata, **attribute_record_metadata, ) else: record = builder.mbbIn( pv, - **update, **datatype_record_metadata, **attribute_record_metadata, ) case Waveform(): record = builder.WaveformIn( - pv, **update, **datatype_record_metadata, **attribute_record_metadata + pv, **datatype_record_metadata, **attribute_record_metadata ) case _: raise FastCSError( diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index fb1d1570..50bbadec 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -239,7 +239,6 @@ def epics_controller_api(class_mocker: MockerFixture): def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): ioc_builder = mocker.patch("fastcs.transports.epics.ca.ioc.builder") - # builder = mocker.patch("fastcs.transports.epics.ca.util.builder") add_pvi_info = mocker.patch("fastcs.transports.epics.ca.ioc._add_pvi_info") add_sub_controller_pvi_info = mocker.patch( "fastcs.transports.epics.ca.ioc._add_sub_controller_pvi_info" From 37c79df1d6f23c5fc8db6b1c37a2277caa9ac4c2 Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Wed, 11 Feb 2026 11:21:52 +0000 Subject: [PATCH 05/11] Test improvement. --- tests/transports/epics/ca/test_initial_value.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/transports/epics/ca/test_initial_value.py b/tests/transports/epics/ca/test_initial_value.py index f3b43fe9..727cb0d7 100644 --- a/tests/transports/epics/ca/test_initial_value.py +++ b/tests/transports/epics/ca/test_initial_value.py @@ -65,7 +65,7 @@ async def test_initial_values_set_in_ca(mocker): task = asyncio.create_task(fastcs.serve(interactive=False)) try: async with asyncio.timeout(3): - while not record_spy.spy_return_list and not record_spy_out.spy_return_list: + while not record_spy.spy_return_list or not record_spy_out.spy_return_list: await asyncio.sleep(0) initial_values = { From 79e96b21d0dee1013380edcc8b681c704765f019 Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Wed, 11 Feb 2026 15:00:20 +0000 Subject: [PATCH 06/11] Fixed EPICs test that was creating the wrong record type. --- tests/transports/epics/ca/test_softioc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 50bbadec..1eaf6c90 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -590,7 +590,7 @@ def test_update_datatype(mocker: MockerFixture): attr_w = AttrW(Int()) record_w = _make_out_record(pv_name, attr_w, on_update=mocker.ANY) - builder.longIn.assert_called_once_with( + builder.longOut.assert_called_once_with( pv_name, **record_metadata_from_attribute(attr_w), **record_metadata_from_datatype(attr_w.datatype), From 333dfdee02d283c0c2240139ff3953a43d1b57f4 Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Fri, 13 Feb 2026 15:51:27 +0000 Subject: [PATCH 07/11] Improvements to EPICS record creation readability. --- src/fastcs/transports/epics/ca/ioc.py | 93 +++++++++++----- src/fastcs/transports/epics/ca/util.py | 35 ++---- .../transports/epics/ca/test_initial_value.py | 2 + tests/transports/epics/ca/test_softioc.py | 105 ++++++++++++------ 4 files changed, 149 insertions(+), 86 deletions(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index 9b9898a3..5c7d1f2d 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -15,10 +15,11 @@ from fastcs.transports.controller_api import ControllerAPI from fastcs.transports.epics import EpicsIOCOptions from fastcs.transports.epics.ca.util import ( + DEFAULT_STRING_WAVEFORM_LENGTH, MBB_MAX_CHOICES, cast_from_epics_type, cast_to_epics_type, - record_metadata_from_attribute, + create_state_keys, record_metadata_from_datatype, ) from fastcs.transports.epics.util import controller_pv_prefix @@ -193,46 +194,55 @@ async def async_record_set(value: DType_T): attribute.add_on_update_callback(async_record_set) -def _make_in_record( - pv: str, - attribute: AttrR | AttrW | AttrRW, -) -> RecordWrapper: - datatype_record_metadata = record_metadata_from_datatype(attribute.datatype) - attribute_record_metadata = record_metadata_from_attribute(attribute) +def _make_in_record(pv: str, attribute: AttrR) -> RecordWrapper: + attribute_record_metadata = { + "DESC": attribute.description, + "initial_value": cast_to_epics_type(attribute.datatype, attribute.get()), + } match attribute.datatype: case Bool(): record = builder.boolIn( - pv, **datatype_record_metadata, **attribute_record_metadata + pv, ZNAM="False", ONAM="True", **attribute_record_metadata ) case Int(): record = builder.longIn( - pv, **datatype_record_metadata, **attribute_record_metadata + pv, + LOPR=attribute.datatype.min_alarm, + HOPR=attribute.datatype.max_alarm, + EGU=attribute.datatype.units, + **attribute_record_metadata, ) case Float(): record = builder.aIn( - pv, **datatype_record_metadata, **attribute_record_metadata + pv, + LOPR=attribute.datatype.min_alarm, + HOPR=attribute.datatype.max_alarm, + EGU=attribute.datatype.units, + PREC=attribute.datatype.prec, + **attribute_record_metadata, ) case String(): record = builder.longStringIn( - pv, **datatype_record_metadata, **attribute_record_metadata + pv, + length=attribute.datatype.length or DEFAULT_STRING_WAVEFORM_LENGTH, + **attribute_record_metadata, ) case Enum(): if len(attribute.datatype.members) > MBB_MAX_CHOICES: record = builder.longStringIn( pv, - **datatype_record_metadata, **attribute_record_metadata, ) else: + attribute_record_metadata.update(create_state_keys(attribute.datatype)) record = builder.mbbIn( pv, - **datatype_record_metadata, **attribute_record_metadata, ) case Waveform(): record = builder.WaveformIn( - pv, **datatype_record_metadata, **attribute_record_metadata + pv, length=attribute.datatype.shape[0], **attribute_record_metadata ) case _: raise FastCSError( @@ -249,52 +259,83 @@ def datatype_updater(datatype: DataType): def _make_out_record( pv: str, - attribute: AttrR | AttrW | AttrRW, + attribute: AttrW | AttrRW, on_update: Callable, ) -> RecordWrapper: - datatype_record_metadata = record_metadata_from_datatype( - attribute.datatype, out_record=True - ) - attribute_record_metadata = record_metadata_from_attribute(attribute) + attribute_record_metadata = { + "DESC": attribute.description, + "initial_value": cast_to_epics_type( + attribute.datatype, + attribute.get() + if isinstance(attribute, AttrR) + else attribute.datatype.initial_value, + ), + } update = {"on_update": on_update, "always_update": True, "blocking": True} match attribute.datatype: case Bool(): record = builder.boolOut( - pv, **update, **datatype_record_metadata, **attribute_record_metadata + pv, ZNAM="False", ONAM="True", **update, **attribute_record_metadata ) case Int(): record = builder.longOut( - pv, **update, **datatype_record_metadata, **attribute_record_metadata + pv, + LOPR=attribute.datatype.min_alarm, + HOPR=attribute.datatype.max_alarm, + EGU=attribute.datatype.units, + DRVL=attribute.datatype.min, + DRVH=attribute.datatype.max, + **update, + **attribute_record_metadata, ) case Float(): record = builder.aOut( - pv, **update, **datatype_record_metadata, **attribute_record_metadata + pv, + LOPR=attribute.datatype.min_alarm, + HOPR=attribute.datatype.max_alarm, + EGU=attribute.datatype.units, + PREC=attribute.datatype.prec, + DRVL=attribute.datatype.min, + DRVH=attribute.datatype.max, + **update, + **attribute_record_metadata, ) case String(): record = builder.longStringOut( - pv, **update, **datatype_record_metadata, **attribute_record_metadata + pv, + length=attribute.datatype.length or DEFAULT_STRING_WAVEFORM_LENGTH, + **update, + **attribute_record_metadata, ) case Enum(): if len(attribute.datatype.members) > MBB_MAX_CHOICES: + datatype: Enum = attribute.datatype + + def _verify_in_datatype(_, value): + return value in datatype.names + record = builder.longStringOut( pv, + validate=_verify_in_datatype, **update, - **datatype_record_metadata, **attribute_record_metadata, ) else: + attribute_record_metadata.update(create_state_keys(attribute.datatype)) record = builder.mbbOut( pv, **update, - **datatype_record_metadata, **attribute_record_metadata, ) case Waveform(): record = builder.WaveformOut( - pv, **update, **datatype_record_metadata, **attribute_record_metadata + pv, + length=attribute.datatype.shape[0], + **update, + **attribute_record_metadata, ) case _: raise FastCSError( diff --git a/src/fastcs/transports/epics/ca/util.py b/src/fastcs/transports/epics/ca/util.py index 27464b84..47693e6f 100644 --- a/src/fastcs/transports/epics/ca/util.py +++ b/src/fastcs/transports/epics/ca/util.py @@ -2,7 +2,6 @@ from dataclasses import asdict from typing import Any -from fastcs.attributes import Attribute, AttrR, AttrW from fastcs.datatypes import Bool, DType_T, Enum, Float, Int, String, Waveform from fastcs.datatypes.datatype import DataType @@ -43,21 +42,6 @@ } -def record_metadata_from_attribute(attribute: Attribute[DType_T]) -> dict[str, Any]: - """Converts attributes on the `Attribute` to the - field name/value in the record metadata.""" - metadata: dict[str, Any] = {"DESC": attribute.description} - initial = None - match attribute: - case AttrR(): - initial = attribute.get() - case AttrW(): - initial = attribute.datatype.initial_value - if initial is not None: - metadata["initial_value"] = cast_to_epics_type(attribute.datatype, initial) - return metadata - - def record_metadata_from_datatype( datatype: DataType[Any], out_record: bool = False ) -> dict[str, str]: @@ -87,14 +71,7 @@ def record_metadata_from_datatype( arguments["length"] = datatype.shape[0] case Enum(): if len(datatype.members) <= MBB_MAX_CHOICES: - state_keys = dict( - zip( - MBB_STATE_FIELDS, - datatype.names, - strict=False, - ) - ) - arguments.update(state_keys) + arguments.update(create_state_keys(datatype)) elif out_record: # no validators for in type records def _verify_in_datatype(_, value): @@ -108,6 +85,16 @@ def _verify_in_datatype(_, value): return arguments +def create_state_keys(datatype: Enum): + return dict( + zip( + MBB_STATE_FIELDS, + datatype.names, + strict=False, + ) + ) + + def cast_from_epics_type(datatype: DataType[DType_T], value: object) -> DType_T: """Casts from an EPICS datatype to a FastCS datatype.""" match datatype: diff --git a/tests/transports/epics/ca/test_initial_value.py b/tests/transports/epics/ca/test_initial_value.py index 727cb0d7..8139b457 100644 --- a/tests/transports/epics/ca/test_initial_value.py +++ b/tests/transports/epics/ca/test_initial_value.py @@ -72,6 +72,7 @@ async def test_initial_values_set_in_ca(mocker): wrapper.name: wrapper.get() for wrapper in record_spy.spy_return_list + record_spy_out.spy_return_list } + print(f"{initial_values}") for name, value in { "SOFTIOC_INITIAL_DEVICE:Bool": 1, "SOFTIOC_INITIAL_DEVICE:BoolR": 0, @@ -109,6 +110,7 @@ async def test_initial_values_set_in_ca(mocker): "SOFTIOC_INITIAL_DEVICE:WaveformW": 10 * [0], "SOFTIOC_INITIAL_DEVICE:Waveform_RBV": [0, 1, 2, 3, 4, 5, 6, 7, 8, 9], }.items(): + print(f"{name} => {value} {initial_values[name]}") assert np.array_equal(value, initial_values[name]) except Exception as e: raise e diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 1eaf6c90..38ed6861 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -31,7 +31,6 @@ _make_out_record, ) from fastcs.transports.epics.ca.util import ( - record_metadata_from_attribute, record_metadata_from_datatype, ) @@ -72,18 +71,41 @@ async def test_create_and_link_read_pv(mocker: MockerFixture): @pytest.mark.parametrize( "attribute,record_type,kwargs", ( - (AttrR(String()), "longStringIn", {}), + (AttrR(String()), "longStringIn", {"DESC": None, "initial_value": ""}), ( AttrR(Enum(ColourEnum)), "mbbIn", - {"ZRST": "RED", "ONST": "GREEN", "TWST": "BLUE"}, + { + "ZRST": "RED", + "ONST": "GREEN", + "TWST": "BLUE", + "DESC": None, + "initial_value": 0, + }, ), ( - AttrR(Enum(enum.IntEnum("ONOFF_STATES", {"DISABLED": 0, "ENABLED": 1}))), + AttrR( + Enum( + enum.IntEnum( + "ONOFF_STATES", + {"DISABLED": 0, "ENABLED": 1}, + ) + ) + ), "mbbIn", - {"ZRST": "DISABLED", "ONST": "ENABLED"}, + {"ZRST": "DISABLED", "ONST": "ENABLED", "DESC": None, "initial_value": 0}, + ), + ( + AttrR(Waveform(np.int32, (10,))), + "WaveformIn", + { + "DESC": None, + # array( + # [0, 0, 0, 0, 0, 0, 0, 0, 0, 0], dtype=np.int32 + # ), + }, + # {"DESC": None, "initial_value": None}, ), - (AttrR(Waveform(np.int32, (10,))), "WaveformIn", {}), ), ) def test_make_input_record( @@ -97,8 +119,9 @@ def test_make_input_record( pv = "PV" _make_in_record(pv, attribute) kwargs.update(record_metadata_from_datatype(attribute.datatype)) - kwargs.update(record_metadata_from_attribute(attribute)) + if record_type == "WaveformIn": + kwargs["initial_value"] = mocker.ANY getattr(builder, record_type).assert_called_once_with( pv, **kwargs, @@ -107,6 +130,7 @@ def test_make_input_record( def test_make_record_raises(mocker: MockerFixture): mocker.patch("fastcs.transports.epics.ca.ioc.record_metadata_from_datatype") + mocker.patch("fastcs.transports.epics.ca.ioc.cast_to_epics_type") # Pass a mock as attribute to provoke the fallback case matching on datatype with pytest.raises(FastCSError): _make_in_record("PV", mocker.MagicMock()) @@ -169,7 +193,12 @@ class LongEnum(enum.Enum): ( AttrW(Enum(enum.IntEnum("ONOFF_STATES", {"DISABLED": 0, "ENABLED": 1}))), "mbbOut", - {"ZRST": "DISABLED", "ONST": "ENABLED"}, + { + "ZRST": "DISABLED", + "ONST": "ENABLED", + "DESC": None, + "initial_value": 0, + }, ), ), ) @@ -186,7 +215,6 @@ def test_make_output_record( _make_out_record(pv, attribute, on_update=update) kwargs.update(record_metadata_from_datatype(attribute.datatype, out_record=True)) - kwargs.update(record_metadata_from_attribute(attribute)) kwargs.update({"always_update": True, "on_update": update, "blocking": True}) getattr(builder, record_type).assert_called_once_with( @@ -216,6 +244,7 @@ def test_long_enum_in_creation(mocker: MockerFixture): def test_get_output_record_raises(mocker: MockerFixture): mocker.patch("fastcs.transports.epics.ca.ioc.record_metadata_from_datatype") + mocker.patch("fastcs.transports.epics.ca.ioc.cast_to_epics_type") # Pass a mock as attribute to provoke the fallback case matching on datatype with pytest.raises(FastCSError): _make_out_record("PV", mocker.MagicMock(), on_update=mocker.MagicMock()) @@ -249,35 +278,35 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): # Check records are created ioc_builder.boolIn.assert_called_once_with( f"{DEVICE}:ReadBool", - **record_metadata_from_attribute(epics_controller_api.attributes["read_bool"]), + DESC=None, + initial_value=False, **record_metadata_from_datatype( epics_controller_api.attributes["read_bool"].datatype ), ) ioc_builder.longIn.assert_any_call( f"{DEVICE}:ReadInt", - **record_metadata_from_attribute(epics_controller_api.attributes["read_int"]), + DESC=None, + initial_value=0, **record_metadata_from_datatype( epics_controller_api.attributes["read_int"].datatype ), ) ioc_builder.aIn.assert_called_once_with( f"{DEVICE}:ReadWriteFloat_RBV", - **record_metadata_from_attribute( - epics_controller_api.attributes["read_write_float"] - ), + DESC=None, + initial_value=0.0, **record_metadata_from_datatype( epics_controller_api.attributes["read_write_float"].datatype ), ) ioc_builder.aOut.assert_any_call( f"{DEVICE}:ReadWriteFloat", + DESC=None, + initial_value=0.0, always_update=True, blocking=True, on_update=mocker.ANY, - **record_metadata_from_attribute( - epics_controller_api.attributes["read_write_float"] - ), **record_metadata_from_datatype( epics_controller_api.attributes["read_write_float"].datatype, out_record=True, @@ -285,38 +314,38 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): ) ioc_builder.longIn.assert_any_call( f"{DEVICE}:ReadWriteInt_RBV", - **record_metadata_from_attribute( - epics_controller_api.attributes["read_write_int"] - ), + DESC=None, + initial_value=0, **record_metadata_from_datatype( epics_controller_api.attributes["read_write_int"].datatype ), ) ioc_builder.longOut.assert_called_with( f"{DEVICE}:ReadWriteInt", + DESC=None, + initial_value=0, always_update=True, blocking=True, on_update=mocker.ANY, - **record_metadata_from_attribute( - epics_controller_api.attributes["read_write_int"] - ), **record_metadata_from_datatype( epics_controller_api.attributes["read_write_int"].datatype, out_record=True ), ) ioc_builder.mbbIn.assert_called_once_with( f"{DEVICE}:Enum_RBV", - **record_metadata_from_attribute(epics_controller_api.attributes["enum"]), + DESC=None, + initial_value=0, **record_metadata_from_datatype( epics_controller_api.attributes["enum"].datatype ), ) ioc_builder.mbbOut.assert_called_once_with( f"{DEVICE}:Enum", + DESC=None, + initial_value=0, always_update=True, blocking=True, on_update=mocker.ANY, - **record_metadata_from_attribute(epics_controller_api.attributes["enum"]), **record_metadata_from_datatype( epics_controller_api.attributes["enum"].datatype, out_record=True ), @@ -326,7 +355,8 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): always_update=True, blocking=True, on_update=mocker.ANY, - **record_metadata_from_attribute(epics_controller_api.attributes["write_bool"]), + DESC=None, + initial_value=False, **record_metadata_from_datatype( epics_controller_api.attributes["write_bool"].datatype, out_record=True ), @@ -473,26 +503,22 @@ def test_long_pv_names_discarded(mocker: MockerFixture): always_update=True, blocking=True, on_update=mocker.ANY, + DESC=None, + initial_value=0, **record_metadata_from_datatype( long_name_controller_api.attributes["attr_rw_short_name"].datatype, out_record=True, ), - **record_metadata_from_attribute( - long_name_controller_api.attributes["attr_rw_short_name"] - ), ) ioc_builder.longIn.assert_called_once_with( f"{DEVICE}:{short_pv_name}_RBV", + DESC=None, + initial_value=0, **record_metadata_from_datatype( long_name_controller_api.attributes[ "attr_rw_with_a_reallyreally_long_name_that_is_too_long_for_rbv" ].datatype ), - **record_metadata_from_attribute( - long_name_controller_api.attributes[ - "attr_rw_with_a_reallyreally_long_name_that_is_too_long_for_rbv" - ] - ), ) long_pv_name = long_attr_name.title().replace("_", "") @@ -573,8 +599,9 @@ def test_update_datatype(mocker: MockerFixture): builder.longIn.assert_called_once_with( pv_name, - **record_metadata_from_attribute(attr_r), **record_metadata_from_datatype(attr_r.datatype), + DESC=None, + initial_value=0, ) record_r.set_field.assert_not_called() attr_r.update_datatype(Int(units="m", min_alarm=-3)) @@ -592,8 +619,14 @@ def test_update_datatype(mocker: MockerFixture): builder.longOut.assert_called_once_with( pv_name, - **record_metadata_from_attribute(attr_w), **record_metadata_from_datatype(attr_w.datatype), + DESC=None, + initial_value=0, + DRVL=None, + DRVH=None, + on_update=mocker.ANY, + always_update=True, + blocking=True, ) record_w.set_field.assert_not_called() attr_w.update_datatype(Int(units="m", min_alarm=-1, min=-3)) From 39c7debb02d3b17346eda13b480e3d50d6b1f8ec Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Fri, 13 Feb 2026 15:55:35 +0000 Subject: [PATCH 08/11] Added docstring --- src/fastcs/transports/epics/ca/util.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fastcs/transports/epics/ca/util.py b/src/fastcs/transports/epics/ca/util.py index 47693e6f..bbfbaca2 100644 --- a/src/fastcs/transports/epics/ca/util.py +++ b/src/fastcs/transports/epics/ca/util.py @@ -86,6 +86,7 @@ def _verify_in_datatype(_, value): def create_state_keys(datatype: Enum): + """Creates a dictionary of state field keys to names""" return dict( zip( MBB_STATE_FIELDS, From f503e6061a21f0093faea48e8b2afae600766d45 Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Fri, 13 Feb 2026 17:12:55 +0000 Subject: [PATCH 09/11] Remove print statements. --- tests/transports/epics/ca/test_initial_value.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/transports/epics/ca/test_initial_value.py b/tests/transports/epics/ca/test_initial_value.py index 8139b457..727cb0d7 100644 --- a/tests/transports/epics/ca/test_initial_value.py +++ b/tests/transports/epics/ca/test_initial_value.py @@ -72,7 +72,6 @@ async def test_initial_values_set_in_ca(mocker): wrapper.name: wrapper.get() for wrapper in record_spy.spy_return_list + record_spy_out.spy_return_list } - print(f"{initial_values}") for name, value in { "SOFTIOC_INITIAL_DEVICE:Bool": 1, "SOFTIOC_INITIAL_DEVICE:BoolR": 0, @@ -110,7 +109,6 @@ async def test_initial_values_set_in_ca(mocker): "SOFTIOC_INITIAL_DEVICE:WaveformW": 10 * [0], "SOFTIOC_INITIAL_DEVICE:Waveform_RBV": [0, 1, 2, 3, 4, 5, 6, 7, 8, 9], }.items(): - print(f"{name} => {value} {initial_values[name]}") assert np.array_equal(value, initial_values[name]) except Exception as e: raise e From ee109fb1b116dbc73de54b0e5f1b21e0fa134deb Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Fri, 13 Feb 2026 17:13:56 +0000 Subject: [PATCH 10/11] Filter out datatype update keys that would cause a runtime error. --- src/fastcs/transports/epics/ca/ioc.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index 5c7d1f2d..887aa3af 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -343,9 +343,13 @@ def _verify_in_datatype(_, value): ) def datatype_updater(datatype: DataType): + # Filter out keys that can't be set via set field + builder_only_keys = {"validate", "length"} for name, value in record_metadata_from_datatype( datatype, out_record=True ).items(): + if name in builder_only_keys: + continue record.set_field(name, value) attribute.add_update_datatype_callback(datatype_updater) From 9f8e941c2c90681c3b07030067123bb45eb5dd28 Mon Sep 17 00:00:00 2001 From: Alan Greer Date: Wed, 18 Feb 2026 17:43:03 +0000 Subject: [PATCH 11/11] Removed record_metadata_from_datatype function. --- src/fastcs/transports/epics/ca/ioc.py | 20 ++--- src/fastcs/transports/epics/ca/util.py | 53 ++--------- tests/transports/epics/ca/test_ca_util.py | 16 ---- tests/transports/epics/ca/test_softioc.py | 104 +++++++++++----------- 4 files changed, 69 insertions(+), 124 deletions(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index 887aa3af..a1110ce2 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -1,5 +1,6 @@ import asyncio from collections.abc import Callable +from dataclasses import asdict from typing import Any, Literal from softioc import builder, softioc @@ -15,12 +16,13 @@ from fastcs.transports.controller_api import ControllerAPI from fastcs.transports.epics import EpicsIOCOptions from fastcs.transports.epics.ca.util import ( + DATATYPE_FIELD_TO_IN_RECORD_FIELD, + DATATYPE_FIELD_TO_OUT_RECORD_FIELD, DEFAULT_STRING_WAVEFORM_LENGTH, MBB_MAX_CHOICES, cast_from_epics_type, cast_to_epics_type, create_state_keys, - record_metadata_from_datatype, ) from fastcs.transports.epics.util import controller_pv_prefix from fastcs.util import snake_to_pascal @@ -250,8 +252,9 @@ def _make_in_record(pv: str, attribute: AttrR) -> RecordWrapper: ) def datatype_updater(datatype: DataType): - for name, value in record_metadata_from_datatype(datatype).items(): - record.set_field(name, value) + for name, value in asdict(datatype).items(): + if name in DATATYPE_FIELD_TO_IN_RECORD_FIELD: + record.set_field(DATATYPE_FIELD_TO_IN_RECORD_FIELD[name], value) attribute.add_update_datatype_callback(datatype_updater) return record @@ -343,14 +346,9 @@ def _verify_in_datatype(_, value): ) def datatype_updater(datatype: DataType): - # Filter out keys that can't be set via set field - builder_only_keys = {"validate", "length"} - for name, value in record_metadata_from_datatype( - datatype, out_record=True - ).items(): - if name in builder_only_keys: - continue - record.set_field(name, value) + for name, value in asdict(datatype).items(): + if name in DATATYPE_FIELD_TO_OUT_RECORD_FIELD: + record.set_field(DATATYPE_FIELD_TO_OUT_RECORD_FIELD[name], value) attribute.add_update_datatype_callback(datatype_updater) return record diff --git a/src/fastcs/transports/epics/ca/util.py b/src/fastcs/transports/epics/ca/util.py index bbfbaca2..2ad82191 100644 --- a/src/fastcs/transports/epics/ca/util.py +++ b/src/fastcs/transports/epics/ca/util.py @@ -1,5 +1,4 @@ import enum -from dataclasses import asdict from typing import Any from fastcs.datatypes import Bool, DType_T, Enum, Float, Int, String, Waveform @@ -32,7 +31,14 @@ EPICS_ALLOWED_DATATYPES = (Bool, Enum, Float, Int, String, Waveform) DEFAULT_STRING_WAVEFORM_LENGTH = 256 -DATATYPE_FIELD_TO_RECORD_FIELD = { +DATATYPE_FIELD_TO_IN_RECORD_FIELD = { + "prec": "PREC", + "units": "EGU", + "min_alarm": "LOPR", + "max_alarm": "HOPR", +} + +DATATYPE_FIELD_TO_OUT_RECORD_FIELD = { "prec": "PREC", "units": "EGU", "min": "DRVL", @@ -42,49 +48,6 @@ } -def record_metadata_from_datatype( - datatype: DataType[Any], out_record: bool = False -) -> dict[str, str]: - """Converts attributes on the `DataType` to the - field name/value in the record metadata.""" - - arguments = { - DATATYPE_FIELD_TO_RECORD_FIELD[field]: value - for field, value in asdict(datatype).items() - if field in DATATYPE_FIELD_TO_RECORD_FIELD - } - - if not out_record: - # in type records don't have DRVL/DRVH fields - arguments.pop("DRVL", None) - arguments.pop("DRVH", None) - - match datatype: - case String(): - arguments["length"] = datatype.length or DEFAULT_STRING_WAVEFORM_LENGTH - case Waveform(): - if len(datatype.shape) != 1: - raise TypeError( - f"Unsupported shape {datatype.shape}, the EPICS transport only " - "supports to 1D arrays" - ) - arguments["length"] = datatype.shape[0] - case Enum(): - if len(datatype.members) <= MBB_MAX_CHOICES: - arguments.update(create_state_keys(datatype)) - elif out_record: # no validators for in type records - - def _verify_in_datatype(_, value): - return value in datatype.names - - arguments["validate"] = _verify_in_datatype - case Bool(): - arguments["ZNAM"] = "False" - arguments["ONAM"] = "True" - - return arguments - - def create_state_keys(datatype: Enum): """Creates a dictionary of state field keys to names""" return dict( diff --git a/tests/transports/epics/ca/test_ca_util.py b/tests/transports/epics/ca/test_ca_util.py index 8488c41d..f484fca9 100644 --- a/tests/transports/epics/ca/test_ca_util.py +++ b/tests/transports/epics/ca/test_ca_util.py @@ -6,7 +6,6 @@ from fastcs.transports.epics.ca.util import ( cast_from_epics_type, cast_to_epics_type, - record_metadata_from_datatype, ) @@ -132,18 +131,3 @@ def test_cast_from_epics_type(datatype, from_epics, result): def test_cast_from_epics_validations(datatype, input): with pytest.raises(ValueError): cast_from_epics_type(datatype, input) - - -def test_drive_metadata_from_datatype(): - dtype = Float(units="mm", min=-10.0, max=10.0, min_alarm=-5, max_alarm=5, prec=3) - out_arguments = record_metadata_from_datatype(dtype, True) - assert out_arguments == { - "DRVH": 10.0, - "DRVL": -10.0, - "EGU": "mm", - "HOPR": 5, - "LOPR": -5, - "PREC": 3, - } - in_arguments = record_metadata_from_datatype(dtype, False) - assert in_arguments == {"EGU": "mm", "HOPR": 5, "LOPR": -5, "PREC": 3} diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 38ed6861..1135c90c 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -30,9 +30,6 @@ _make_in_record, _make_out_record, ) -from fastcs.transports.epics.ca.util import ( - record_metadata_from_datatype, -) DEVICE = "DEVICE" @@ -71,7 +68,11 @@ async def test_create_and_link_read_pv(mocker: MockerFixture): @pytest.mark.parametrize( "attribute,record_type,kwargs", ( - (AttrR(String()), "longStringIn", {"DESC": None, "initial_value": ""}), + ( + AttrR(String()), + "longStringIn", + {"length": 256, "DESC": None, "initial_value": ""}, + ), ( AttrR(Enum(ColourEnum)), "mbbIn", @@ -100,11 +101,8 @@ async def test_create_and_link_read_pv(mocker: MockerFixture): "WaveformIn", { "DESC": None, - # array( - # [0, 0, 0, 0, 0, 0, 0, 0, 0, 0], dtype=np.int32 - # ), + "length": 10, }, - # {"DESC": None, "initial_value": None}, ), ), ) @@ -118,7 +116,6 @@ def test_make_input_record( pv = "PV" _make_in_record(pv, attribute) - kwargs.update(record_metadata_from_datatype(attribute.datatype)) if record_type == "WaveformIn": kwargs["initial_value"] = mocker.ANY @@ -129,7 +126,6 @@ def test_make_input_record( def test_make_record_raises(mocker: MockerFixture): - mocker.patch("fastcs.transports.epics.ca.ioc.record_metadata_from_datatype") mocker.patch("fastcs.transports.epics.ca.ioc.cast_to_epics_type") # Pass a mock as attribute to provoke the fallback case matching on datatype with pytest.raises(FastCSError): @@ -214,7 +210,6 @@ def test_make_output_record( pv = "PV" _make_out_record(pv, attribute, on_update=update) - kwargs.update(record_metadata_from_datatype(attribute.datatype, out_record=True)) kwargs.update({"always_update": True, "on_update": update, "blocking": True}) getattr(builder, record_type).assert_called_once_with( @@ -243,7 +238,6 @@ def test_long_enum_in_creation(mocker: MockerFixture): def test_get_output_record_raises(mocker: MockerFixture): - mocker.patch("fastcs.transports.epics.ca.ioc.record_metadata_from_datatype") mocker.patch("fastcs.transports.epics.ca.ioc.cast_to_epics_type") # Pass a mock as attribute to provoke the fallback case matching on datatype with pytest.raises(FastCSError): @@ -279,76 +273,80 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): ioc_builder.boolIn.assert_called_once_with( f"{DEVICE}:ReadBool", DESC=None, + ZNAM="False", + ONAM="True", initial_value=False, - **record_metadata_from_datatype( - epics_controller_api.attributes["read_bool"].datatype - ), ) ioc_builder.longIn.assert_any_call( f"{DEVICE}:ReadInt", DESC=None, + EGU=None, + LOPR=None, + HOPR=None, initial_value=0, - **record_metadata_from_datatype( - epics_controller_api.attributes["read_int"].datatype - ), ) ioc_builder.aIn.assert_called_once_with( f"{DEVICE}:ReadWriteFloat_RBV", DESC=None, + LOPR=None, + HOPR=None, + EGU=None, + PREC=2, initial_value=0.0, - **record_metadata_from_datatype( - epics_controller_api.attributes["read_write_float"].datatype - ), ) - ioc_builder.aOut.assert_any_call( + ioc_builder.aOut.assert_called_once_with( f"{DEVICE}:ReadWriteFloat", DESC=None, + LOPR=None, + HOPR=None, + EGU=None, + PREC=2, + DRVL=None, + DRVH=None, initial_value=0.0, always_update=True, blocking=True, on_update=mocker.ANY, - **record_metadata_from_datatype( - epics_controller_api.attributes["read_write_float"].datatype, - out_record=True, - ), ) ioc_builder.longIn.assert_any_call( f"{DEVICE}:ReadWriteInt_RBV", DESC=None, + LOPR=None, + HOPR=None, + EGU=None, initial_value=0, - **record_metadata_from_datatype( - epics_controller_api.attributes["read_write_int"].datatype - ), ) ioc_builder.longOut.assert_called_with( f"{DEVICE}:ReadWriteInt", + LOPR=None, + HOPR=None, + EGU=None, + DRVL=None, + DRVH=None, DESC=None, initial_value=0, always_update=True, blocking=True, on_update=mocker.ANY, - **record_metadata_from_datatype( - epics_controller_api.attributes["read_write_int"].datatype, out_record=True - ), ) ioc_builder.mbbIn.assert_called_once_with( f"{DEVICE}:Enum_RBV", DESC=None, initial_value=0, - **record_metadata_from_datatype( - epics_controller_api.attributes["enum"].datatype - ), + ZRST="RED", + ONST="GREEN", + TWST="BLUE", ) ioc_builder.mbbOut.assert_called_once_with( f"{DEVICE}:Enum", DESC=None, initial_value=0, + ZRST="RED", + ONST="GREEN", + TWST="BLUE", always_update=True, blocking=True, on_update=mocker.ANY, - **record_metadata_from_datatype( - epics_controller_api.attributes["enum"].datatype, out_record=True - ), ) ioc_builder.boolOut.assert_called_once_with( f"{DEVICE}:WriteBool", @@ -356,10 +354,9 @@ def test_ioc(mocker: MockerFixture, epics_controller_api: ControllerAPI): blocking=True, on_update=mocker.ANY, DESC=None, + ZNAM="False", + ONAM="True", initial_value=False, - **record_metadata_from_datatype( - epics_controller_api.attributes["write_bool"].datatype, out_record=True - ), ) ioc_builder.Action.assert_any_call( f"{DEVICE}:Go", @@ -501,24 +498,23 @@ def test_long_pv_names_discarded(mocker: MockerFixture): ioc_builder.longOut.assert_called_once_with( f"{DEVICE}:{short_pv_name}", always_update=True, + LOPR=None, + HOPR=None, + EGU=None, + DRVL=None, + DRVH=None, blocking=True, on_update=mocker.ANY, DESC=None, initial_value=0, - **record_metadata_from_datatype( - long_name_controller_api.attributes["attr_rw_short_name"].datatype, - out_record=True, - ), ) ioc_builder.longIn.assert_called_once_with( f"{DEVICE}:{short_pv_name}_RBV", DESC=None, initial_value=0, - **record_metadata_from_datatype( - long_name_controller_api.attributes[ - "attr_rw_with_a_reallyreally_long_name_that_is_too_long_for_rbv" - ].datatype - ), + LOPR=None, + HOPR=None, + EGU=None, ) long_pv_name = long_attr_name.title().replace("_", "") @@ -599,7 +595,9 @@ def test_update_datatype(mocker: MockerFixture): builder.longIn.assert_called_once_with( pv_name, - **record_metadata_from_datatype(attr_r.datatype), + LOPR=None, + HOPR=None, + EGU=None, DESC=None, initial_value=0, ) @@ -619,8 +617,10 @@ def test_update_datatype(mocker: MockerFixture): builder.longOut.assert_called_once_with( pv_name, - **record_metadata_from_datatype(attr_w.datatype), DESC=None, + LOPR=None, + HOPR=None, + EGU=None, initial_value=0, DRVL=None, DRVH=None,