From 52556c1f12a04a344950d45fa655e46edd686dff Mon Sep 17 00:00:00 2001 From: James Souter Date: Thu, 3 Apr 2025 08:31:05 +0100 Subject: [PATCH 1/5] CA enums with more than 16 values backed by string record --- src/fastcs/transport/epics/ca/util.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/fastcs/transport/epics/ca/util.py b/src/fastcs/transport/epics/ca/util.py index 7d8ee8d9f..4763a8d87 100644 --- a/src/fastcs/transport/epics/ca/util.py +++ b/src/fastcs/transport/epics/ca/util.py @@ -86,7 +86,12 @@ def cast_from_epics_type(datatype: DataType[T], value: object) -> T: """Casts from an EPICS datatype to a FastCS datatype.""" match datatype: case Enum(): - return datatype.validate(datatype.members[value]) + if len(datatype.members) <= MBB_MAX_CHOICES: + # epics type is mbb + return datatype.validate(datatype.members[value]) + else: # enum backed by string record + # epics type is string + return datatype.validate(datatype.enum_cls(value)) case datatype if issubclass(type(datatype), EPICS_ALLOWED_DATATYPES): return datatype.validate(value) # type: ignore case _: @@ -97,7 +102,10 @@ def cast_to_epics_type(datatype: DataType[T], value: T) -> object: """Casts from an attribute's datatype to an EPICS datatype.""" match datatype: case Enum(): - return datatype.index_of(datatype.validate(value)) + if len(datatype.members) <= MBB_MAX_CHOICES: + return datatype.index_of(datatype.validate(value)) + else: # enum backed by string record + return datatype.validate(value).value case datatype if issubclass(type(datatype), EPICS_ALLOWED_DATATYPES): return datatype.validate(value) case _: @@ -119,7 +127,7 @@ def builder_callable_from_attribute( return builder.longStringIn if make_in_record else builder.longStringOut case Enum(): if len(attribute.datatype.members) > MBB_MAX_CHOICES: - return builder.longIn if make_in_record else builder.longOut + return builder.longStringIn if make_in_record else builder.longStringOut else: return builder.mbbIn if make_in_record else builder.mbbOut case Waveform(): From a9b5b66cc40e1bc2f5a6bbf3d6143910d34d716f Mon Sep 17 00:00:00 2001 From: James Souter Date: Mon, 7 Apr 2025 15:36:34 +0100 Subject: [PATCH 2/5] Use enum name instead of value as underlying epics value test int and string enums in CA, test validation casting to and from epics --- src/fastcs/transport/epics/ca/util.py | 6 +- tests/transport/epics/ca/test_softioc.py | 7 -- tests/transport/epics/ca/test_util.py | 141 +++++++++++++++++++++++ 3 files changed, 143 insertions(+), 11 deletions(-) diff --git a/src/fastcs/transport/epics/ca/util.py b/src/fastcs/transport/epics/ca/util.py index 4763a8d87..1566f043a 100644 --- a/src/fastcs/transport/epics/ca/util.py +++ b/src/fastcs/transport/epics/ca/util.py @@ -87,11 +87,9 @@ def cast_from_epics_type(datatype: DataType[T], value: object) -> T: match datatype: case Enum(): if len(datatype.members) <= MBB_MAX_CHOICES: - # epics type is mbb return datatype.validate(datatype.members[value]) else: # enum backed by string record - # epics type is string - return datatype.validate(datatype.enum_cls(value)) + return datatype.validate(datatype.enum_cls[value]) case datatype if issubclass(type(datatype), EPICS_ALLOWED_DATATYPES): return datatype.validate(value) # type: ignore case _: @@ -105,7 +103,7 @@ def cast_to_epics_type(datatype: DataType[T], value: T) -> object: if len(datatype.members) <= MBB_MAX_CHOICES: return datatype.index_of(datatype.validate(value)) else: # enum backed by string record - return datatype.validate(value).value + return datatype.validate(value).name case datatype if issubclass(type(datatype), EPICS_ALLOWED_DATATYPES): return datatype.validate(value) case _: diff --git a/tests/transport/epics/ca/test_softioc.py b/tests/transport/epics/ca/test_softioc.py index b27703d49..b25e20bed 100644 --- a/tests/transport/epics/ca/test_softioc.py +++ b/tests/transport/epics/ca/test_softioc.py @@ -30,7 +30,6 @@ _make_record, ) from fastcs.transport.epics.ca.util import ( - MBB_STATE_FIELDS, record_metadata_from_attribute, record_metadata_from_datatype, ) @@ -45,12 +44,6 @@ class OnOffStates(enum.IntEnum): ENABLED = 1 -def record_input_from_enum(enum_cls: type[enum.IntEnum]) -> dict[str, str]: - return dict( - zip(MBB_STATE_FIELDS, [member.name for member in enum_cls], strict=False) - ) - - @pytest.mark.asyncio async def test_create_and_link_read_pv(mocker: MockerFixture): make_record = mocker.patch("fastcs.transport.epics.ca.ioc._make_record") diff --git a/tests/transport/epics/ca/test_util.py b/tests/transport/epics/ca/test_util.py index e69de29bb..790bcd460 100644 --- a/tests/transport/epics/ca/test_util.py +++ b/tests/transport/epics/ca/test_util.py @@ -0,0 +1,141 @@ +import enum + +import pytest +from softioc import builder + +from fastcs.attributes import AttrRW +from fastcs.datatypes import Bool, Enum, Float, Int, String +from fastcs.transport.epics.ca.util import ( + builder_callable_from_attribute, + cast_from_epics_type, + cast_to_epics_type, +) + + +class ShortEnum(enum.Enum): + NOT = 0 + TOO = 1 + MANY = 2 + VALUES = 3 + + +class LongEnum(enum.Enum): + THIS = 0 + IS = 1 + AN = 2 + ENUM = 3 + WITH = 4 + ALTOGETHER = 5 + TOO = 6 + MANY = 7 + VALUES = 8 + TO = 9 + BE = 10 + DESCRIBED = 11 + BY = 12 + MBB = 14 + TYPE = 15 + EPICS = 16 + RECORDS = 17 + + +class LongMixedEnum(enum.Enum): + THIS = "the value is THIS" + IS = 1 + AN = "the value is AN" + ENUM = 3 + WITH = "the value is WITH" + ALTOGETHER = 5 + TOO = "the value is TOO" + MANY = 7 + VALUES = "the value is VALUES" + TO = 9 + BE = "the value is BE" + DESCRIBED = 11 + BY = "the value is BY" + MBB = 13 + TYPE = "the value is TYPE" + EPICS = None + RECORDS = "the value is RECORDS" + + +class ShortMixedEnum(enum.Enum): + STRING_MEMBER = "I am a string" + INT_MEMBER = 2 + NONE_MEMBER = None + + +@pytest.mark.parametrize( + "datatype,input,output", + [ + (Enum(ShortEnum), ShortEnum.TOO, 1), + # in CA, enums with too many values become epics strings + (Enum(LongMixedEnum), LongMixedEnum.BE, "BE"), # string value + (Enum(LongMixedEnum), LongMixedEnum.EPICS, "EPICS"), # None value + (Enum(LongMixedEnum), LongMixedEnum.MBB, "MBB"), # int value + (Int(), 4, 4), + (Float(), 1.0, 1.0), + (Bool(), True, True), + (String(), "hey", "hey"), + # shorter enums can be represented by integers from 0-15 + (Enum(ShortMixedEnum), ShortMixedEnum.STRING_MEMBER, 0), + (Enum(ShortMixedEnum), ShortMixedEnum.INT_MEMBER, 1), + (Enum(ShortMixedEnum), ShortMixedEnum.NONE_MEMBER, 2), + ], +) +def test_casting_to_epics(datatype, input, output): + assert cast_to_epics_type(datatype, input) == output + + +@pytest.mark.parametrize( + "datatype, input", + [ + # TODO cover Waveform and Table cases + (Enum(ShortEnum), 0), # can't use index + (Enum(ShortEnum), LongEnum.TOO), # wrong enum.Enum class + (Int(), 4.0), + (Float(), 1), + (Bool(), None), + (String(), 10), + ], +) +def test_cast_to_epics_validations(datatype, input): + with pytest.raises(ValueError): + cast_to_epics_type(datatype, input) + + +@pytest.mark.parametrize( + "datatype,from_epics,result", + [ + # long enums backed by strings + (Enum(LongMixedEnum), "BE", LongMixedEnum.BE), # string value + (Enum(LongMixedEnum), "EPICS", LongMixedEnum.EPICS), # None value + (Enum(LongMixedEnum), "MBB", LongMixedEnum.MBB), # int value + (Int(), 4, 4), + (Float(), 1.0, 1.0), + (Bool(), True, True), + (String(), "hey", "hey"), + (Enum(ShortEnum), 2, ShortEnum.MANY), + # short enums backed by mbbi/mbbo + (Enum(ShortMixedEnum), 0, ShortMixedEnum.STRING_MEMBER), + (Enum(ShortMixedEnum), 1, ShortMixedEnum.INT_MEMBER), + (Enum(ShortMixedEnum), 2, ShortMixedEnum.NONE_MEMBER), + ], +) +def test_cast_from_epics_type(datatype, from_epics, result): + assert cast_from_epics_type(datatype, from_epics) == result + + +@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 From 97c16bb4352e71016c2e3aee92e7e5461eaef079 Mon Sep 17 00:00:00 2001 From: James Souter Date: Thu, 10 Apr 2025 08:42:35 +0100 Subject: [PATCH 3/5] validate that enums records with many options can't be put with invalid values in CA --- src/fastcs/transport/epics/ca/ioc.py | 9 +++++++-- src/fastcs/transport/epics/ca/util.py | 10 +++++++++- tests/transport/epics/ca/test_softioc.py | 4 +++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/fastcs/transport/epics/ca/ioc.py b/src/fastcs/transport/epics/ca/ioc.py index 36e4ca014..47a202af0 100644 --- a/src/fastcs/transport/epics/ca/ioc.py +++ b/src/fastcs/transport/epics/ca/ioc.py @@ -171,9 +171,12 @@ def _make_record( pv: str, attribute: AttrR | AttrW | AttrRW, on_update: Callable | None = None, + out_record: bool = False, ) -> RecordWrapper: builder_callable = builder_callable_from_attribute(attribute, on_update is None) - datatype_record_metadata = record_metadata_from_datatype(attribute.datatype) + datatype_record_metadata = record_metadata_from_datatype( + attribute.datatype, out_record + ) attribute_record_metadata = record_metadata_from_attribute(attribute) update = {"always_update": True, "on_update": on_update} if on_update else {} @@ -201,7 +204,9 @@ async def on_update(value): async def async_write_display(value: T): record.set(cast_to_epics_type(attribute.datatype, value), process=False) - record = _make_record(f"{pv_prefix}:{pv_name}", attribute, on_update=on_update) + record = _make_record( + f"{pv_prefix}:{pv_name}", attribute, on_update=on_update, out_record=True + ) _add_attr_pvi_info(record, pv_prefix, attr_name, "w") diff --git a/src/fastcs/transport/epics/ca/util.py b/src/fastcs/transport/epics/ca/util.py index 1566f043a..729179c30 100644 --- a/src/fastcs/transport/epics/ca/util.py +++ b/src/fastcs/transport/epics/ca/util.py @@ -50,7 +50,9 @@ def record_metadata_from_attribute( return {"DESC": attribute.description} -def record_metadata_from_datatype(datatype: DataType[T]) -> dict[str, str]: +def record_metadata_from_datatype( + datatype: DataType[T], out_record: bool = False +) -> dict[str, str]: """Converts attributes on the `DataType` to the field name/value in the record metadata.""" @@ -78,6 +80,12 @@ def record_metadata_from_datatype(datatype: DataType[T]) -> dict[str, str]: ) ) arguments.update(state_keys) + elif out_record: # no validators for in type records + + def _verify_in_datatype(_, value): + return value in [member.name for member in datatype.members] + + arguments["validate"] = _verify_in_datatype return arguments diff --git a/tests/transport/epics/ca/test_softioc.py b/tests/transport/epics/ca/test_softioc.py index b25e20bed..ad21fac89 100644 --- a/tests/transport/epics/ca/test_softioc.py +++ b/tests/transport/epics/ca/test_softioc.py @@ -120,7 +120,9 @@ 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) + make_record.assert_called_once_with( + "PREFIX:PV", attribute, on_update=mocker.ANY, out_record=True + ) 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 From 665f521ca5d0fba9cb02b49993941ac617347989 Mon Sep 17 00:00:00 2001 From: James Souter Date: Thu, 10 Apr 2025 14:44:16 +0100 Subject: [PATCH 4/5] add names property to Enum, fix typing --- src/fastcs/datatypes.py | 4 ++++ src/fastcs/transport/epics/ca/util.py | 4 ++-- src/fastcs/transport/epics/gui.py | 4 +--- src/fastcs/transport/epics/pva/types.py | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/fastcs/datatypes.py b/src/fastcs/datatypes.py index 8b96b81a1..31d5eb6b4 100644 --- a/src/fastcs/datatypes.py +++ b/src/fastcs/datatypes.py @@ -143,6 +143,10 @@ def index_of(self, value: T_Enum) -> int: def members(self) -> list[T_Enum]: return list(self.enum_cls) + @cached_property + def names(self) -> list[str]: + return [member.name for member in self.members] + @property def dtype(self) -> type[T_Enum]: return self.enum_cls diff --git a/src/fastcs/transport/epics/ca/util.py b/src/fastcs/transport/epics/ca/util.py index 729179c30..0baa58587 100644 --- a/src/fastcs/transport/epics/ca/util.py +++ b/src/fastcs/transport/epics/ca/util.py @@ -75,7 +75,7 @@ def record_metadata_from_datatype( state_keys = dict( zip( MBB_STATE_FIELDS, - [member.name for member in datatype.members], + datatype.names, strict=False, ) ) @@ -83,7 +83,7 @@ def record_metadata_from_datatype( elif out_record: # no validators for in type records def _verify_in_datatype(_, value): - return value in [member.name for member in datatype.members] + return value in datatype.names arguments["validate"] = _verify_in_datatype diff --git a/src/fastcs/transport/epics/gui.py b/src/fastcs/transport/epics/gui.py index ff586def1..0ad048fcd 100644 --- a/src/fastcs/transport/epics/gui.py +++ b/src/fastcs/transport/epics/gui.py @@ -69,9 +69,7 @@ def _get_write_widget(attribute: AttrW) -> WriteWidgetUnion | None: case String(): return TextWrite(format=TextFormat.string) case Enum(): - return ComboBox( - choices=[member.name for member in attribute.datatype.members] - ) + return ComboBox(choices=attribute.datatype.names) case Waveform(): return None case datatype: diff --git a/src/fastcs/transport/epics/pva/types.py b/src/fastcs/transport/epics/pva/types.py index 08a83b1ea..256d5d832 100644 --- a/src/fastcs/transport/epics/pva/types.py +++ b/src/fastcs/transport/epics/pva/types.py @@ -183,7 +183,7 @@ def cast_to_p4p_value(attribute: Attribute[T], value: T) -> object: case Enum(): return { "index": attribute.datatype.index_of(value), - "choices": [member.name for member in attribute.datatype.members], + "choices": attribute.datatype.names, } case Waveform(): return attribute.datatype.validate(value) From b32d86757917be588ea85920be4e3d9857f15706 Mon Sep 17 00:00:00 2001 From: James Souter Date: Thu, 10 Apr 2025 15:17:02 +0100 Subject: [PATCH 5/5] Test that long enums are validated in CA --- tests/transport/epics/ca/test_softioc.py | 33 +++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/transport/epics/ca/test_softioc.py b/tests/transport/epics/ca/test_softioc.py index ad21fac89..4e73b517d 100644 --- a/tests/transport/epics/ca/test_softioc.py +++ b/tests/transport/epics/ca/test_softioc.py @@ -139,6 +139,26 @@ async def test_create_and_link_write_pv(mocker: MockerFixture): attribute.process_without_display_update.assert_called_once_with(1) +class LongEnum(enum.Enum): + THIS = 0 + IS = 1 + AN = 2 + ENUM = 3 + WITH = 4 + ALTOGETHER = 5 + TOO = 6 + MANY = 7 + VALUES = 8 + TO = 9 + BE = 10 + DESCRIBED = 11 + BY = 12 + MBB = 14 + TYPE = 15 + EPICS = 16 + RECORDS = 17 + + @pytest.mark.parametrize( "attribute,record_type,kwargs", ( @@ -159,7 +179,7 @@ def test_make_output_record( update = mocker.MagicMock() pv = "PV" - _make_record(pv, attribute, on_update=update) + _make_record(pv, attribute, on_update=update, out_record=True) kwargs.update(record_metadata_from_datatype(attribute.datatype)) kwargs.update(record_metadata_from_attribute(attribute)) @@ -171,6 +191,17 @@ def test_make_output_record( ) +def test_long_enum_validator(mocker: MockerFixture): + builder = mocker.patch("fastcs.transport.epics.ca.util.builder") + update = mocker.MagicMock() + attribute = AttrRW(Enum(LongEnum)) + pv = "PV" + record = _make_record(pv, attribute, on_update=update, out_record=True) + 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): # Pass a mock as attribute to provoke the fallback case matching on datatype with pytest.raises(FastCSException):