From 369f4a78ea4319dbcdc2f387861aa5d8895c1663 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Wed, 5 Aug 2026 19:56:53 +0000 Subject: [PATCH 1/9] feat: add enum mapping for aliases --- src/fastcs/demo/controllers.py | 1 + src/fastcs/demo/schema.json | 44 ++++- src/fastcs/transports/epics/ca/ioc.py | 218 +++++++++++++++++++++---- src/fastcs/transports/epics/options.py | 10 +- 4 files changed, 238 insertions(+), 35 deletions(-) diff --git a/src/fastcs/demo/controllers.py b/src/fastcs/demo/controllers.py index 5926fc8ce..2fc10c07b 100755 --- a/src/fastcs/demo/controllers.py +++ b/src/fastcs/demo/controllers.py @@ -95,6 +95,7 @@ async def cancel_all(self) -> None: async def connect(self) -> None: await self.connection.connect(self._settings.ip_settings) + self._connected = True async def reconnect(self): try: diff --git a/src/fastcs/demo/schema.json b/src/fastcs/demo/schema.json index 210c36256..486d7da20 100644 --- a/src/fastcs/demo/schema.json +++ b/src/fastcs/demo/schema.json @@ -1,11 +1,53 @@ { "$defs": { + "EnumMapping": { + "additionalProperties": false, + "properties": { + "pv": { + "title": "Pv", + "type": "string" + }, + "mapping": { + "additionalProperties": { + "anyOf": [ + { + "type": "integer" + }, + { + "type": "string" + }, + { + "type": "number" + }, + { + "type": "boolean" + } + ] + }, + "title": "Mapping", + "type": "object" + } + }, + "required": [ + "pv", + "mapping" + ], + "title": "EnumMapping", + "type": "object" + }, "EpicsCAOptions": { "additionalProperties": false, "properties": { "aliases": { "additionalProperties": { - "type": "string" + "anyOf": [ + { + "type": "string" + }, + { + "$ref": "#/$defs/EnumMapping" + } + ] }, "title": "Aliases", "type": "object" diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index 7f210c424..d9f657796 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -1,7 +1,8 @@ import asyncio from collections import Counter from collections.abc import Awaitable -from typing import Any, Literal +from enum import IntEnum +from typing import Any, Literal, TypeVar from softioc import alarm, builder, softioc from softioc.asyncio_dispatcher import AsyncioDispatcher @@ -9,7 +10,7 @@ from fastcs.attributes import AttrR, AttrRW, AttrW from fastcs.controllers import ControllerAPI -from fastcs.datatypes import DType_T, Waveform +from fastcs.datatypes import DType_T, Enum, Waveform from fastcs.logging import logger from fastcs.methods import Command from fastcs.tracer import Tracer @@ -19,20 +20,29 @@ cast_from_epics_type, cast_to_epics_type, ) +from fastcs.transports.epics.options import EnumMapping from fastcs.transports.epics.util import EPICS_MAX_NAME_LENGTH, pv_prefix_from_path from fastcs.util import snake_to_pascal tracer = Tracer() - +EnumT = TypeVar("EnumT", bound=IntEnum) RBV_SUFFIX = "_RBV" class EpicsCAIOC: """A softioc which handles one or more controllers.""" - def __init__(self, controller_apis: list[ControllerAPI], aliases: dict[str, str]): + def __init__( + self, + controller_apis: list[ControllerAPI], + aliases: dict[str, str | EnumMapping], + ): + alias_pvs = [ + value if isinstance(value, str) else value.pv for value in aliases.values() + ] + if duplicate_aliases := [ - alias for alias, count in Counter(aliases.values()).items() if count > 1 + alias for alias, count in Counter(alias_pvs).items() if count > 1 ]: raise RuntimeError( "Failed to create EPICS CA IOC, as duplicate aliases were provided:" @@ -120,7 +130,7 @@ def _add_sub_controller_pvi_info(parent: ControllerAPI): def _create_and_link_attribute_pvs( - root_controller_api: ControllerAPI, aliases: dict[str, str] + root_controller_api: ControllerAPI, aliases: dict[str, str | EnumMapping] ) -> None: for controller_api in root_controller_api.walk_api(): pv_prefix = pv_prefix_from_path(controller_api.path) @@ -147,7 +157,7 @@ def _create_and_link_attribute_pvs( ) continue - alias = aliases.get(f"{pv_prefix}:{pv_name}", None) + alias = aliases.pop(f"{pv_prefix}:{pv_name}", None) match attribute: case AttrRW(): if full_pv_name_length > (EPICS_MAX_NAME_LENGTH - 4): @@ -158,7 +168,7 @@ def _create_and_link_attribute_pvs( ) attribute.enabled = False else: - alias_rbv = aliases.get( + alias_rbv = aliases.pop( f"{pv_prefix}:{pv_name}{RBV_SUFFIX}", None ) _create_and_link_read_pv( @@ -193,15 +203,9 @@ def _create_and_link_attribute_pvs( ) -def _create_and_link_read_pv( - pv_prefix: str, - pv_name: str, - attr_name: str, - alias: str | None, - attribute: AttrR[DType_T], -) -> None: - pv = f"{pv_prefix}:{pv_name}" - +def _make_in_record_and_add_callback( + pv: str, attribute: AttrR[DType_T] +) -> RecordWrapper: async def async_record_set(value: DType_T): tracer.log_event( "PV set from attribute", topic=attribute, pv=pv, value=repr(value) @@ -210,23 +214,33 @@ async def async_record_set(value: DType_T): record.set(cast_to_epics_type(attribute.datatype, value)) record = _make_in_record(pv, attribute) - - _add_alias(record, alias) - - _add_attr_pvi_info(record, pv_prefix, attr_name, "r") - attribute.add_on_update_callback(async_record_set) + return record -def _create_and_link_write_pv( +def _create_and_link_read_pv( pv_prefix: str, pv_name: str, attr_name: str, - alias: str | None, - attribute: AttrW[DType_T], -): + alias: str | EnumMapping | None, + attribute: AttrR[DType_T], +) -> None: pv = f"{pv_prefix}:{pv_name}" + record = _make_in_record_and_add_callback(pv, attribute) + + if isinstance(alias, str): + _add_alias(record, alias) + elif isinstance(alias, EnumMapping): + enum_attr = _get_read_enum_attr_from_type(alias) + _add_read_enum_alias(alias, attribute, enum_attr) + + _add_attr_pvi_info(record, pv_prefix, attr_name, "r") + + +def _make_out_record_and_sync_setpoint( + pv: str, attribute: AttrW[DType_T] +) -> RecordWrapper: async def on_update(value): logger.info("PV put: {pv} = {value}", pv=pv, value=repr(value)) await _run_and_set_alarm( @@ -241,23 +255,39 @@ async def set_setpoint_without_process(value: DType_T): record.set(cast_to_epics_type(attribute.datatype, value), process=False) record = _make_out_record(pv, attribute, on_update=on_update) + attribute.add_sync_setpoint_callback(set_setpoint_without_process) + return record - _add_alias(record, alias) - _add_attr_pvi_info(record, pv_prefix, attr_name, "w") +def _create_and_link_write_pv( + pv_prefix: str, + pv_name: str, + attr_name: str, + alias: str | EnumMapping | None, + attribute: AttrW[DType_T], +): + pv = f"{pv_prefix}:{pv_name}" - attribute.add_sync_setpoint_callback(set_setpoint_without_process) + record = _make_out_record_and_sync_setpoint(pv, attribute) + + if isinstance(alias, str): + _add_alias(record, alias) + elif isinstance(alias, EnumMapping): + enum_attr = _get_write_enum_attr_from_type(alias) + _add_write_enum_alias(alias, attribute, enum_attr) + + _add_attr_pvi_info(record, pv_prefix, attr_name, "w") def _create_and_link_command_pvs( - root_controller_api: ControllerAPI, aliases: dict[str, str] + root_controller_api: ControllerAPI, aliases: dict[str, str | EnumMapping] ) -> None: for controller_api in root_controller_api.walk_api(): pv_prefix = pv_prefix_from_path(controller_api.path) for attr_name, method in controller_api.command_methods.items(): pv_name = snake_to_pascal(attr_name) - alias = aliases.get(f"{pv_prefix}:{pv_name}", None) + alias = aliases.pop(f"{pv_prefix}:{pv_name}", None) if len(f"{pv_prefix}:{pv_name}") > EPICS_MAX_NAME_LENGTH: print( @@ -276,7 +306,11 @@ def _create_and_link_command_pvs( def _create_and_link_command_pv( - pv_prefix: str, pv_name: str, attr_name: str, alias: str | None, method: Command + pv_prefix: str, + pv_name: str, + attr_name: str, + alias: str | EnumMapping | None, + method: Command, ) -> None: pv = f"{pv_prefix}:{pv_name}" @@ -293,7 +327,11 @@ async def wrapped_method(_: Any): ONAM="Active", ) - _add_alias(record, alias) + if isinstance(alias, str): + _add_alias(record, alias) + elif isinstance(alias, EnumMapping): + enum_attr = _get_write_enum_attr_from_type(alias) + _add_command_enum_alias(alias, method, enum_attr) _add_attr_pvi_info(record, pv_prefix, attr_name, "x") @@ -360,3 +398,117 @@ async def _run_and_set_alarm(record, coro: Awaitable): _set_alarm(record, alarm.NO_ALARM) except Exception: _set_alarm(record, alarm.MAJOR_ALARM) + + +def _get_enum_from_alias( + alias: EnumMapping, +): + members = {name: i for i, name in enumerate(alias.mapping)} + return IntEnum("enum", members) + + +def _get_read_enum_attr_from_type(alias: EnumMapping): + enum = _get_enum_from_alias(alias) + return AttrR(datatype=Enum(enum)) + + +def _get_write_enum_attr_from_type(alias: EnumMapping): + enum = _get_enum_from_alias(alias) + return AttrW(datatype=Enum(enum)) + + +def _add_command_enum_alias( + alias: EnumMapping, method: Command, enum_attr: AttrW[EnumT] +): + record = _make_out_record_and_sync_setpoint(alias.pv, enum_attr) + + async def trigger_command(_, value) -> None: + converted_value = alias.mapping.get(value.name) + + if converted_value is None: + logger.warning( + "Failed to convert enum alias value {value} to command boolean. " + "No mapping exists.", + value=value, + enum_mapping=alias.mapping, + ) + return + + if not isinstance(converted_value, bool): + logger.warning( + "Aliased commands only accept boolean mappings. " + "Got {value} from mapping.", + value=converted_value, + enum_mapping=alias.mapping, + ) + return + + if converted_value: + logger.info("Calling aliased command") + await _run_and_set_alarm(record, method.fn()) + + enum_attr.set_on_put_callback(trigger_command) + + +def _add_read_enum_alias( + alias: EnumMapping, attribute: AttrR[DType_T], enum_attr: AttrR[EnumT] +): + enum = enum_attr.datatype.dtype + + async def convert_from_value(value) -> None: + converted_value = next( + (k for k, v in alias.mapping.items() if v == value), + None, + ) + + if converted_value is not None: + validated_value = enum[converted_value] + logger.info( + "Converting to enum value {enum_value} from fastcs value {value}", + enum_value=validated_value, + value=value, + ) + await enum_attr.update(validated_value) + else: + logger.warning( + "Ignoring enum update as fastcs value {value} has no " + "corresponding value in enum mapping", + value=value, + mapping=alias.mapping, + ) + + attribute.add_on_update_callback(convert_from_value) + _make_in_record_and_add_callback(alias.pv, enum_attr) + + +def _add_write_enum_alias( + alias: EnumMapping, + attribute: AttrW[DType_T], + enum_attr: AttrW[EnumT], +): + record = _make_out_record_and_sync_setpoint(alias.pv, enum_attr) + + async def convert_to_value(_, value) -> None: + converted_value = alias.mapping.get(value.name) + + if converted_value is not None: + logger.info( + "Converting enum value {enum_value} to fastcs value {converted_value}", + enum_value=value, + converted_value=converted_value, + ) + await _run_and_set_alarm( + record, + attribute.put( + cast_from_epics_type(attribute.datatype, converted_value) + ), + ) + else: + logger.warning( + "Ignoring enum put value {value} as it has no " + "corresponding value in enum mapping", + value=value, + mapping=alias.mapping, + ) + + enum_attr.set_on_put_callback(convert_to_value) diff --git a/src/fastcs/transports/epics/options.py b/src/fastcs/transports/epics/options.py index d76323934..ea3b17fc2 100644 --- a/src/fastcs/transports/epics/options.py +++ b/src/fastcs/transports/epics/options.py @@ -31,6 +31,14 @@ class EpicsGUIOptions: title: str = "FastCS Devices" +@dataclass +class EnumMapping: + __pydantic_config__: ClassVar[ConfigDict] = ConfigDict(extra="forbid") + + pv: str + mapping: dict[str, int | str | float | bool] + + @dataclass class EpicsCAOptions: """Channel-Access-specific options. @@ -41,7 +49,7 @@ class EpicsCAOptions: __pydantic_config__: ClassVar[ConfigDict] = ConfigDict(extra="forbid") - aliases: dict[str, str] = field(default_factory=dict) + aliases: dict[str, str | EnumMapping] = field(default_factory=dict) """Mapping of fastcs PV names to their aliases. Setpoint and readback PVs must be aliased separately. From daa443f0d0cf95a1b8a28642ac5c2aba3a2dc26d Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 6 Aug 2026 12:13:01 +0000 Subject: [PATCH 2/9] fix: correct alarm setting on aliased enum records and add tests for enum mapping --- src/fastcs/transports/epics/ca/ioc.py | 98 ++++++++++--------- tests/data/schema.json | 44 ++++++++- tests/example_softioc.py | 19 +++- tests/transports/epics/ca/test_softioc.py | 59 ++++++++++- .../epics/ca/test_softioc_system.py | 36 ++++++- 5 files changed, 201 insertions(+), 55 deletions(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index d9f657796..dcb53dc01 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -1,6 +1,6 @@ import asyncio from collections import Counter -from collections.abc import Awaitable +from collections.abc import Awaitable, Mapping from enum import IntEnum from typing import Any, Literal, TypeVar @@ -35,7 +35,7 @@ class EpicsCAIOC: def __init__( self, controller_apis: list[ControllerAPI], - aliases: dict[str, str | EnumMapping], + aliases: Mapping[str, str | EnumMapping], ): alias_pvs = [ value if isinstance(value, str) else value.pv for value in aliases.values() @@ -130,7 +130,7 @@ def _add_sub_controller_pvi_info(parent: ControllerAPI): def _create_and_link_attribute_pvs( - root_controller_api: ControllerAPI, aliases: dict[str, str | EnumMapping] + root_controller_api: ControllerAPI, aliases: Mapping[str, str | EnumMapping] ) -> None: for controller_api in root_controller_api.walk_api(): pv_prefix = pv_prefix_from_path(controller_api.path) @@ -157,7 +157,7 @@ def _create_and_link_attribute_pvs( ) continue - alias = aliases.pop(f"{pv_prefix}:{pv_name}", None) + alias = aliases.get(f"{pv_prefix}:{pv_name}", None) match attribute: case AttrRW(): if full_pv_name_length > (EPICS_MAX_NAME_LENGTH - 4): @@ -168,7 +168,7 @@ def _create_and_link_attribute_pvs( ) attribute.enabled = False else: - alias_rbv = aliases.pop( + alias_rbv = aliases.get( f"{pv_prefix}:{pv_name}{RBV_SUFFIX}", None ) _create_and_link_read_pv( @@ -203,21 +203,6 @@ def _create_and_link_attribute_pvs( ) -def _make_in_record_and_add_callback( - pv: str, attribute: AttrR[DType_T] -) -> RecordWrapper: - async def async_record_set(value: DType_T): - tracer.log_event( - "PV set from attribute", topic=attribute, pv=pv, value=repr(value) - ) - - record.set(cast_to_epics_type(attribute.datatype, value)) - - record = _make_in_record(pv, attribute) - attribute.add_on_update_callback(async_record_set) - return record - - def _create_and_link_read_pv( pv_prefix: str, pv_name: str, @@ -227,7 +212,14 @@ def _create_and_link_read_pv( ) -> None: pv = f"{pv_prefix}:{pv_name}" - record = _make_in_record_and_add_callback(pv, attribute) + async def async_record_set(value: DType_T): + tracer.log_event( + "PV set from attribute", topic=attribute, pv=pv, value=repr(value) + ) + + record.set(cast_to_epics_type(attribute.datatype, value)) + + record = _make_in_record(pv, attribute) if isinstance(alias, str): _add_alias(record, alias) @@ -236,17 +228,10 @@ def _create_and_link_read_pv( _add_read_enum_alias(alias, attribute, enum_attr) _add_attr_pvi_info(record, pv_prefix, attr_name, "r") + attribute.add_on_update_callback(async_record_set) -def _make_out_record_and_sync_setpoint( - pv: str, attribute: AttrW[DType_T] -) -> RecordWrapper: - async def on_update(value): - logger.info("PV put: {pv} = {value}", pv=pv, value=repr(value)) - await _run_and_set_alarm( - record, attribute.put(cast_from_epics_type(attribute.datatype, value)) - ) - +def _sync_setpoint(pv: str, attribute: AttrW[DType_T], record: RecordWrapper) -> None: async def set_setpoint_without_process(value: DType_T): tracer.log_event( "PV setpoint set from attribute", topic=attribute, pv=pv, value=repr(value) @@ -254,9 +239,7 @@ async def set_setpoint_without_process(value: DType_T): record.set(cast_to_epics_type(attribute.datatype, value), process=False) - record = _make_out_record(pv, attribute, on_update=on_update) attribute.add_sync_setpoint_callback(set_setpoint_without_process) - return record def _create_and_link_write_pv( @@ -268,7 +251,13 @@ def _create_and_link_write_pv( ): pv = f"{pv_prefix}:{pv_name}" - record = _make_out_record_and_sync_setpoint(pv, attribute) + async def on_update(value): + logger.info("PV put: {pv} = {value}", pv=pv, value=repr(value)) + await _run_and_set_alarm( + record, attribute.put(cast_from_epics_type(attribute.datatype, value)) + ) + + record = _make_out_record(pv, attribute, on_update=on_update) if isinstance(alias, str): _add_alias(record, alias) @@ -277,17 +266,18 @@ def _create_and_link_write_pv( _add_write_enum_alias(alias, attribute, enum_attr) _add_attr_pvi_info(record, pv_prefix, attr_name, "w") + _sync_setpoint(pv, attribute, record) def _create_and_link_command_pvs( - root_controller_api: ControllerAPI, aliases: dict[str, str | EnumMapping] + root_controller_api: ControllerAPI, aliases: Mapping[str, str | EnumMapping] ) -> None: for controller_api in root_controller_api.walk_api(): pv_prefix = pv_prefix_from_path(controller_api.path) for attr_name, method in controller_api.command_methods.items(): pv_name = snake_to_pascal(attr_name) - alias = aliases.pop(f"{pv_prefix}:{pv_name}", None) + alias = aliases.get(f"{pv_prefix}:{pv_name}", None) if len(f"{pv_prefix}:{pv_name}") > EPICS_MAX_NAME_LENGTH: print( @@ -385,7 +375,7 @@ def _set_alarm(record: RecordWrapper, alarm_state: int): ) -async def _run_and_set_alarm(record, coro: Awaitable): +async def _run_and_set_alarm(record: RecordWrapper, coro: Awaitable): """Await `coro` and update `record`'s alarm state based on the outcome. On success, clears the alarm (NO_ALARM). On any exception, raises the @@ -418,12 +408,15 @@ def _get_write_enum_attr_from_type(alias: EnumMapping): def _add_command_enum_alias( - alias: EnumMapping, method: Command, enum_attr: AttrW[EnumT] + alias: EnumMapping, + method: Command, + enum_attr: AttrW[EnumT], ): - record = _make_out_record_and_sync_setpoint(alias.pv, enum_attr) - - async def trigger_command(_, value) -> None: - converted_value = alias.mapping.get(value.name) + async def trigger_command(value) -> None: + logger.info("PV put: {pv} = {value}", pv=alias.pv, value=repr(value)) + cast_value = cast_from_epics_type(enum_attr.datatype, value) + await enum_attr.put(cast_value) + converted_value = alias.mapping.get(cast_value.name) if converted_value is None: logger.warning( @@ -445,9 +438,11 @@ async def trigger_command(_, value) -> None: if converted_value: logger.info("Calling aliased command") + await enum_attr.put(value) await _run_and_set_alarm(record, method.fn()) - enum_attr.set_on_put_callback(trigger_command) + record = _make_out_record(alias.pv, enum_attr, on_update=trigger_command) + _sync_setpoint(alias.pv, enum_attr, record) def _add_read_enum_alias( @@ -456,6 +451,11 @@ def _add_read_enum_alias( enum = enum_attr.datatype.dtype async def convert_from_value(value) -> None: + tracer.log_event( + "PV set from attribute", topic=attribute, pv=alias.pv, value=repr(value) + ) + record.set(cast_to_epics_type(enum_attr.datatype, value)) + converted_value = next( (k for k, v in alias.mapping.items() if v == value), None, @@ -477,8 +477,8 @@ async def convert_from_value(value) -> None: mapping=alias.mapping, ) + record = _make_in_record(alias.pv, enum_attr) attribute.add_on_update_callback(convert_from_value) - _make_in_record_and_add_callback(alias.pv, enum_attr) def _add_write_enum_alias( @@ -486,10 +486,11 @@ def _add_write_enum_alias( attribute: AttrW[DType_T], enum_attr: AttrW[EnumT], ): - record = _make_out_record_and_sync_setpoint(alias.pv, enum_attr) - - async def convert_to_value(_, value) -> None: - converted_value = alias.mapping.get(value.name) + async def convert_to_value(value) -> None: + logger.info("PV put: {pv} = {value}", pv=alias.pv, value=repr(value)) + cast_value = cast_from_epics_type(enum_attr.datatype, value) + await enum_attr.put(cast_value) + converted_value = alias.mapping.get(cast_value.name) if converted_value is not None: logger.info( @@ -511,4 +512,5 @@ async def convert_to_value(_, value) -> None: mapping=alias.mapping, ) - enum_attr.set_on_put_callback(convert_to_value) + record = _make_out_record(alias.pv, enum_attr, on_update=convert_to_value) + _sync_setpoint(alias.pv, enum_attr, record) diff --git a/tests/data/schema.json b/tests/data/schema.json index ce2d1e1ff..9b94aa94d 100644 --- a/tests/data/schema.json +++ b/tests/data/schema.json @@ -1,11 +1,53 @@ { "$defs": { + "EnumMapping": { + "additionalProperties": false, + "properties": { + "pv": { + "title": "Pv", + "type": "string" + }, + "mapping": { + "additionalProperties": { + "anyOf": [ + { + "type": "integer" + }, + { + "type": "string" + }, + { + "type": "number" + }, + { + "type": "boolean" + } + ] + }, + "title": "Mapping", + "type": "object" + } + }, + "required": [ + "pv", + "mapping" + ], + "title": "EnumMapping", + "type": "object" + }, "EpicsCAOptions": { "additionalProperties": false, "properties": { "aliases": { "additionalProperties": { - "type": "string" + "anyOf": [ + { + "type": "string" + }, + { + "$ref": "#/$defs/EnumMapping" + } + ] }, "title": "Aliases", "type": "object" diff --git a/tests/example_softioc.py b/tests/example_softioc.py index 3688ff4fb..07b078459 100644 --- a/tests/example_softioc.py +++ b/tests/example_softioc.py @@ -1,16 +1,24 @@ import asyncio +from enum import IntEnum from pathlib import Path from fastcs.attributes import AttrR, AttrRW, AttrW from fastcs.control_system import FastCS from fastcs.controllers import Controller, ControllerVector -from fastcs.datatypes import Int +from fastcs.datatypes import Enum, Int from fastcs.methods import command from fastcs.transports.epics.ca.transport import ( EpicsCAOptions, EpicsCATransport, EpicsGUIOptions, ) +from fastcs.transports.epics.options import EnumMapping + + +class ExampleEnum(IntEnum): + Invalid = 0 + Idle = 1 + Active = 2 class ParentController(Controller): @@ -33,6 +41,8 @@ async def d(self): self.fail_on_next_e = True print("D: FINISHED") + e: AttrRW = AttrRW(Enum(ExampleEnum)) + def run(id="SOFTIOC_TEST_DEVICE"): controller = ParentController() @@ -48,6 +58,13 @@ def run(id="SOFTIOC_TEST_DEVICE"): aliases={ f"{id}:B": f"{id}:AliasB", f"{id}:B_RBV": f"{id}:AliasB_RBV", + f"{id}:ChildVector:0:E": EnumMapping( + pv=f"{id}:EnumAliasE", mapping={"Off": 1, "On": 2} + ), + f"{id}:ChildVector:0:D": EnumMapping( + pv=f"{id}:EnumAliasD", + mapping={"Idle": False, "Active": True}, + ), } ), gui=gui_options, diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 8b7c12205..6ba752409 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -33,6 +33,7 @@ _make_in_record, _make_out_record, ) +from fastcs.transports.epics.options import EnumMapping from fastcs.transports.epics.util import EPICS_MAX_NAME_LENGTH DEVICE = "DEVICE" @@ -45,6 +46,9 @@ class OnOffStates(enum.IntEnum): ENABLED = 1 +async def do_nothing(): ... + + @pytest.mark.asyncio async def test_create_and_link_read_pv(mocker: MockerFixture): make_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_in_record") @@ -148,6 +152,58 @@ async def test_ioc_raises_if_duplicate_aliases_provided(mocker: MockerFixture): EpicsCAIOC(mocker.MagicMock(), aliases) +@pytest.mark.parametrize( + ("create_pv", "add_helper", "expected_type", "mock_attribute"), + [ + ( + _create_and_link_write_pv, + "_add_write_enum_alias", + AttrW, + AttrRW(Int()), + ), + ( + _create_and_link_command_pv, + "_add_command_enum_alias", + AttrW, + Command(do_nothing), + ), + ( + _create_and_link_read_pv, + "_add_read_enum_alias", + AttrR, + AttrR(Int()), + ), + ], +) +@pytest.mark.asyncio +async def test_create_and_link_pv_adds_enum_mapping( + mocker: MockerFixture, + create_pv, + add_helper: str, + expected_type: type[AttrW] | type[AttrR], + mock_attribute: AttrRW | AttrR | Command, +): + add_enum_alias = mocker.patch(f"fastcs.transports.epics.ca.ioc.{add_helper}") + enum_mapping = EnumMapping(pv="enum_alias", mapping={"One": 1, "Two": 2}) + + create_pv( + "PREFIX", + "PV", + "attr", + enum_mapping, + mock_attribute, + ) + + add_enum_alias.assert_called_once() + alias, passed_attribute, enum_attr = add_enum_alias.call_args.args + + assert alias == enum_mapping + assert passed_attribute == mock_attribute + assert isinstance(enum_attr, expected_type) + assert isinstance(enum_attr.datatype, Enum) + assert enum_attr.datatype.names == ["One", "Two"] + + @pytest.mark.parametrize( "attribute,record_type,kwargs", ( @@ -569,9 +625,6 @@ def test_add_attr_pvi_info(mocker: MockerFixture): ) -async def do_nothing(): ... - - class ControllerLongNames(Controller): attr_r_with_reallyreallyreallyreallyreallyreallyreally_long_name = AttrR(Int()) attr_rw_with_a_reallyreally_long_name_that_is_too_long_for_rbv = AttrRW(Int()) diff --git a/tests/transports/epics/ca/test_softioc_system.py b/tests/transports/epics/ca/test_softioc_system.py index f898fe06b..7ced15a63 100644 --- a/tests/transports/epics/ca/test_softioc_system.py +++ b/tests/transports/epics/ca/test_softioc_system.py @@ -44,6 +44,10 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): assert child_pvi["value"] == { "c": {"w": f"{pv_prefix}:ChildVector:0:C"}, "d": {"x": f"{pv_prefix}:ChildVector:0:D"}, + "e": { + "r": f"{pv_prefix}:ChildVector:0:E_RBV", + "w": f"{pv_prefix}:ChildVector:0:E", + }, } # Assert alias. Aliases do not show up in PVI structure @@ -54,7 +58,35 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): assert ctxt.get(f"{pv_prefix}:B") == 20 assert ctxt.get(f"{pv_prefix}:B_RBV") == ctxt.get(f"{pv_prefix}:AliasB_RBV") == 20 + # Assert enum alias. Enum aliases do not show up in PVI structure + enum_value = ctxt.get(f"{pv_prefix}:ChildVector:0:E") + assert enum_value == 0 + assert str(enum_value) == "Invalid" # Default for underlying enum attr + aliased_enum_value = ctxt.get(f"{pv_prefix}:EnumAliasE") + assert aliased_enum_value == 0 + assert str(aliased_enum_value) == "Off" # Default for alias enum attr + + ctxt.put(f"{pv_prefix}:EnumAliasE", 1, wait=True) + assert ( + str(ctxt.get(f"{pv_prefix}:EnumAliasE")) == "On" + ) # 'On' is index 1, but maps to value '2' + converted_enum_value = ctxt.get(f"{pv_prefix}:ChildVector:0:E") + assert converted_enum_value == 2 # Underlying enum attr gets put with 2 + assert str(converted_enum_value) == "Active" + assert str(ctxt.get(f"{pv_prefix}:ChildVector:0:E_RBV")) == "Active" + # Assert command exceptions set record alarm states ctxt.put(f"{pv_prefix}:ChildVector:0:D", True, wait=True) - d_alarm = ctxt.get(f"{pv_prefix}:ChildVector:0:D.SEVR") - assert d_alarm == alarm.MAJOR_ALARM + assert ctxt.get(f"{pv_prefix}:ChildVector:0:D.SEVR") == alarm.MAJOR_ALARM + ctxt.put(f"{pv_prefix}:ChildVector:0:D", True, wait=True) + assert ( + ctxt.get(f"{pv_prefix}:ChildVector:0:D.SEVR") == alarm.NO_ALARM + ) # Second put resets alarm + + # Assert command aliased to enum + ctxt.put( + f"{pv_prefix}:EnumAliasD", "Active", wait=True + ) # This aliases to True on command 'D' + assert ctxt.get(f"{pv_prefix}:EnumAliasD.SEVR") == alarm.MAJOR_ALARM + ctxt.put(f"{pv_prefix}:EnumAliasD", "Active", wait=True) + assert ctxt.get(f"{pv_prefix}:EnumAliasD.SEVR") == alarm.NO_ALARM From 7b28b044852e4e3e661ba69d9ee21ca0d7b9d2dc Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 6 Aug 2026 13:45:25 +0000 Subject: [PATCH 3/9] fix: use correct value on enum alias update and add test --- src/fastcs/transports/epics/ca/ioc.py | 9 ++++----- tests/example_softioc.py | 3 +++ tests/transports/epics/ca/test_softioc_system.py | 6 ++++++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index dcb53dc01..a1703bcc5 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -451,11 +451,6 @@ def _add_read_enum_alias( enum = enum_attr.datatype.dtype async def convert_from_value(value) -> None: - tracer.log_event( - "PV set from attribute", topic=attribute, pv=alias.pv, value=repr(value) - ) - record.set(cast_to_epics_type(enum_attr.datatype, value)) - converted_value = next( (k for k, v in alias.mapping.items() if v == value), None, @@ -463,6 +458,10 @@ async def convert_from_value(value) -> None: if converted_value is not None: validated_value = enum[converted_value] + tracer.log_event( + "PV set from attribute", topic=attribute, pv=alias.pv, value=repr(value) + ) + record.set(cast_to_epics_type(enum_attr.datatype, validated_value)) logger.info( "Converting to enum value {enum_value} from fastcs value {value}", enum_value=validated_value, diff --git a/tests/example_softioc.py b/tests/example_softioc.py index 07b078459..57a949755 100644 --- a/tests/example_softioc.py +++ b/tests/example_softioc.py @@ -61,6 +61,9 @@ def run(id="SOFTIOC_TEST_DEVICE"): f"{id}:ChildVector:0:E": EnumMapping( pv=f"{id}:EnumAliasE", mapping={"Off": 1, "On": 2} ), + f"{id}:ChildVector:0:E_RBV": EnumMapping( + pv=f"{id}:EnumAliasE_RBV", mapping={"Off": 1, "On": 2} + ), f"{id}:ChildVector:0:D": EnumMapping( pv=f"{id}:EnumAliasD", mapping={"Idle": False, "Active": True}, diff --git a/tests/transports/epics/ca/test_softioc_system.py b/tests/transports/epics/ca/test_softioc_system.py index 7ced15a63..53ff2ab89 100644 --- a/tests/transports/epics/ca/test_softioc_system.py +++ b/tests/transports/epics/ca/test_softioc_system.py @@ -75,6 +75,12 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): assert str(converted_enum_value) == "Active" assert str(ctxt.get(f"{pv_prefix}:ChildVector:0:E_RBV")) == "Active" + ctxt.put(f"{pv_prefix}:ChildVector:0:E", 1, wait=True) + assert str(ctxt.get(f"{pv_prefix}:ChildVector:0:E_RBV")) == "Idle" + assert ( + str(ctxt.get(f"{pv_prefix}:EnumAliasE_RBV")) == "Off" + ) # Aliased enum gets converted update + # Assert command exceptions set record alarm states ctxt.put(f"{pv_prefix}:ChildVector:0:D", True, wait=True) assert ctxt.get(f"{pv_prefix}:ChildVector:0:D.SEVR") == alarm.MAJOR_ALARM From a06629a9b9ab39ca01d0270ab0500b36343f9125 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 6 Aug 2026 13:54:16 +0000 Subject: [PATCH 4/9] chore: add type alias for enum mapping values --- src/fastcs/demo/schema.json | 6 +++--- src/fastcs/transports/epics/options.py | 6 ++++-- tests/data/schema.json | 6 +++--- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/fastcs/demo/schema.json b/src/fastcs/demo/schema.json index 486d7da20..662e3862a 100644 --- a/src/fastcs/demo/schema.json +++ b/src/fastcs/demo/schema.json @@ -11,16 +11,16 @@ "additionalProperties": { "anyOf": [ { - "type": "integer" + "type": "boolean" }, { - "type": "string" + "type": "integer" }, { "type": "number" }, { - "type": "boolean" + "type": "string" } ] }, diff --git a/src/fastcs/transports/epics/options.py b/src/fastcs/transports/epics/options.py index ea3b17fc2..1dadfe378 100644 --- a/src/fastcs/transports/epics/options.py +++ b/src/fastcs/transports/epics/options.py @@ -1,10 +1,12 @@ from dataclasses import dataclass, field from enum import Enum from pathlib import Path -from typing import ClassVar +from typing import ClassVar, TypeAlias from pydantic import ConfigDict +EnumMappingValue: TypeAlias = bool | int | float | str + @dataclass class EpicsDocsOptions: @@ -36,7 +38,7 @@ class EnumMapping: __pydantic_config__: ClassVar[ConfigDict] = ConfigDict(extra="forbid") pv: str - mapping: dict[str, int | str | float | bool] + mapping: dict[str, EnumMappingValue] @dataclass diff --git a/tests/data/schema.json b/tests/data/schema.json index 9b94aa94d..b26fb2a97 100644 --- a/tests/data/schema.json +++ b/tests/data/schema.json @@ -11,16 +11,16 @@ "additionalProperties": { "anyOf": [ { - "type": "integer" + "type": "boolean" }, { - "type": "string" + "type": "integer" }, { "type": "number" }, { - "type": "boolean" + "type": "string" } ] }, From 3e00549a9f7b42a30224deaa1c1c69c2a24a9821 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 3 Sep 2026 14:06:19 +0000 Subject: [PATCH 5/9] fix: check length of enum pvs --- src/fastcs/transports/epics/ca/ioc.py | 60 +++++++++++------------ tests/transports/epics/ca/test_softioc.py | 28 ++++++++++- 2 files changed, 55 insertions(+), 33 deletions(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index a1703bcc5..d6668cd7f 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -147,30 +147,18 @@ def _create_and_link_attribute_pvs( continue pv_name = snake_to_pascal(attr_name) - full_pv_name_length = len(f"{pv_prefix}:{pv_name}") - if full_pv_name_length > EPICS_MAX_NAME_LENGTH: + if not _validate_pv_length(attr_name, f"{pv_prefix}:{pv_name}"): attribute.enabled = False - logger.warning( - f"Not creating PV for {attr_name} for controller" - f" {controller_api.path} as full name would exceed" - f" {EPICS_MAX_NAME_LENGTH} characters" - ) continue alias = aliases.get(f"{pv_prefix}:{pv_name}", None) match attribute: case AttrRW(): - if full_pv_name_length > (EPICS_MAX_NAME_LENGTH - 4): - logger.warning( - f"Not creating PVs for {attr_name} as _RBV PV" - f" name would exceed {EPICS_MAX_NAME_LENGTH}" - " characters" - ) + rbv_pv = f"{pv_prefix}:{pv_name}{RBV_SUFFIX}" + if not _validate_pv_length(attr_name, rbv_pv): attribute.enabled = False else: - alias_rbv = aliases.get( - f"{pv_prefix}:{pv_name}{RBV_SUFFIX}", None - ) + alias_rbv = aliases.get(rbv_pv, None) _create_and_link_read_pv( pv_prefix, f"{pv_name}{RBV_SUFFIX}", @@ -222,7 +210,7 @@ async def async_record_set(value: DType_T): record = _make_in_record(pv, attribute) if isinstance(alias, str): - _add_alias(record, alias) + _add_alias(record, alias, attr_name) elif isinstance(alias, EnumMapping): enum_attr = _get_read_enum_attr_from_type(alias) _add_read_enum_alias(alias, attribute, enum_attr) @@ -260,7 +248,7 @@ async def on_update(value): record = _make_out_record(pv, attribute, on_update=on_update) if isinstance(alias, str): - _add_alias(record, alias) + _add_alias(record, alias, attr_name) elif isinstance(alias, EnumMapping): enum_attr = _get_write_enum_attr_from_type(alias) _add_write_enum_alias(alias, attribute, enum_attr) @@ -279,11 +267,7 @@ def _create_and_link_command_pvs( pv_name = snake_to_pascal(attr_name) alias = aliases.get(f"{pv_prefix}:{pv_name}", None) - if len(f"{pv_prefix}:{pv_name}") > EPICS_MAX_NAME_LENGTH: - print( - f"Not creating PV for {attr_name} as full name would exceed" - f" {EPICS_MAX_NAME_LENGTH} characters" - ) + if not _validate_pv_length(attr_name, f"{pv_prefix}:{pv_name}"): method.enabled = False else: _create_and_link_command_pv( @@ -318,7 +302,7 @@ async def wrapped_method(_: Any): ) if isinstance(alias, str): - _add_alias(record, alias) + _add_alias(record, alias, attr_name) elif isinstance(alias, EnumMapping): enum_attr = _get_write_enum_attr_from_type(alias) _add_command_enum_alias(alias, method, enum_attr) @@ -355,14 +339,19 @@ def _add_attr_pvi_info( ) -def _add_alias(record: RecordWrapper, alias: str | None): +def _validate_pv_length(attribute_name: str, pv: str): + if len(pv) > EPICS_MAX_NAME_LENGTH: + logger.warning( + f"Not creating PV '{pv}' for {attribute_name}, as full name would exceed" + f" {EPICS_MAX_NAME_LENGTH} characters" + ) + return False + return True + + +def _add_alias(record: RecordWrapper, alias: str, attr_name: str): if alias is not None: - if len(alias) > EPICS_MAX_NAME_LENGTH: - logger.warning( - f"Not creating alias {alias}, as full name would exceed" - f" {EPICS_MAX_NAME_LENGTH} characters" - ) - else: + if _validate_pv_length(attr_name, alias): record.add_alias(alias) @@ -412,6 +401,9 @@ def _add_command_enum_alias( method: Command, enum_attr: AttrW[EnumT], ): + if not _validate_pv_length(str(method), alias.pv): + return + async def trigger_command(value) -> None: logger.info("PV put: {pv} = {value}", pv=alias.pv, value=repr(value)) cast_value = cast_from_epics_type(enum_attr.datatype, value) @@ -448,6 +440,9 @@ async def trigger_command(value) -> None: def _add_read_enum_alias( alias: EnumMapping, attribute: AttrR[DType_T], enum_attr: AttrR[EnumT] ): + if not _validate_pv_length(attribute.name, alias.pv): + return + enum = enum_attr.datatype.dtype async def convert_from_value(value) -> None: @@ -485,6 +480,9 @@ def _add_write_enum_alias( attribute: AttrW[DType_T], enum_attr: AttrW[EnumT], ): + if not _validate_pv_length(attribute.name, alias.pv): + return + async def convert_to_value(value) -> None: logger.info("PV put: {pv} = {value}", pv=alias.pv, value=repr(value)) cast_value = cast_from_epics_type(enum_attr.datatype, value) diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 6ba752409..6f6cdd2fb 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -23,8 +23,11 @@ EpicsCAIOC, _add_alias, _add_attr_pvi_info, + _add_command_enum_alias, _add_pvi_info, + _add_read_enum_alias, _add_sub_controller_pvi_info, + _add_write_enum_alias, _create_and_link_command_pv, _create_and_link_read_pv, _create_and_link_write_pv, @@ -133,16 +136,37 @@ async def test_add_alias_skips_alias_if_too_long(mocker: MockerFixture): too_long_alias_name = f"long_{alias_name}" record = mocker.MagicMock() - _add_alias(record, alias_name) + _add_alias(record, alias_name, "attr") record.add_alias.assert_called_once_with(alias_name) - _add_alias(record, too_long_alias_name) + _add_alias(record, too_long_alias_name, "attr") with pytest.raises(AssertionError): # assert alias that is too long is not added record.add_alias.assert_called_once_with(too_long_alias_name) +@pytest.mark.parametrize("alias_type", ("read", "write", "command")) +@pytest.mark.asyncio +async def test_enum_alias_skips_pv_if_too_long(mocker: MockerFixture, alias_type: str): + alias = EnumMapping(pv="alias", mapping={"One": 1}) + mocker.patch( + "fastcs.transports.epics.ca.ioc.EPICS_MAX_NAME_LENGTH", len(alias.pv) - 1 + ) + make_in_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_in_record") + make_out_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_out_record") + + if alias_type == "read": + _add_read_enum_alias(alias, AttrR(Int()), AttrR(Enum(OnOffStates))) + elif alias_type == "write": + _add_write_enum_alias(alias, AttrW(Int()), AttrW(Enum(OnOffStates))) + else: + _add_command_enum_alias(alias, Command(do_nothing), AttrW(Enum(OnOffStates))) + + make_in_record.assert_not_called() + make_out_record.assert_not_called() + + @pytest.mark.asyncio async def test_ioc_raises_if_duplicate_aliases_provided(mocker: MockerFixture): aliases = {"A": "Alias", "B": "Alias"} From 54c43d64e8a0f762dcb47332fa2c104abfd7b56e Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Thu, 3 Sep 2026 16:01:46 +0000 Subject: [PATCH 6/9] fix: sync setpoint enum value if underlying setpoint pv changes --- src/fastcs/transports/epics/ca/ioc.py | 26 ++++++++++++++++++- .../epics/ca/test_softioc_system.py | 1 + 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index d6668cd7f..a70301c45 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -447,7 +447,11 @@ def _add_read_enum_alias( async def convert_from_value(value) -> None: converted_value = next( - (k for k, v in alias.mapping.items() if v == value), + ( + name + for name, mapped_value in alias.mapping.items() + if mapped_value == value + ), None, ) @@ -511,3 +515,23 @@ async def convert_to_value(value) -> None: record = _make_out_record(alias.pv, enum_attr, on_update=convert_to_value) _sync_setpoint(alias.pv, enum_attr, record) + + async def update_enum_alias(value: DType_T) -> None: + converted_value = next( + ( + name + for name, mapped_value in alias.mapping.items() + if mapped_value == value + ), + None, + ) + + if converted_value is not None: + enum_value = enum_attr.datatype.dtype[converted_value] + record.set( + cast_to_epics_type(enum_attr.datatype, enum_value), + process=False, + ) + + if isinstance(attribute, AttrR): + attribute.add_on_update_callback(update_enum_alias) diff --git a/tests/transports/epics/ca/test_softioc_system.py b/tests/transports/epics/ca/test_softioc_system.py index 53ff2ab89..8f4a3b5c8 100644 --- a/tests/transports/epics/ca/test_softioc_system.py +++ b/tests/transports/epics/ca/test_softioc_system.py @@ -80,6 +80,7 @@ def test_ioc(softioc_subprocess: tuple[str, Queue]): assert ( str(ctxt.get(f"{pv_prefix}:EnumAliasE_RBV")) == "Off" ) # Aliased enum gets converted update + assert str(ctxt.get(f"{pv_prefix}:EnumAliasE")) == "Off" # Assert command exceptions set record alarm states ctxt.put(f"{pv_prefix}:ChildVector:0:D", True, wait=True) From 9f0e8dab0e52be395830883ff547ef9a6dee4efd Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 9 Oct 2026 13:08:51 +0000 Subject: [PATCH 7/9] refactor: streamline enum aliasing and address issues with forward vs backward mapping --- src/fastcs/transports/epics/ca/ioc.py | 131 ++++++++++++++------------ 1 file changed, 72 insertions(+), 59 deletions(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index a70301c45..8c251c3f0 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -10,7 +10,7 @@ from fastcs.attributes import AttrR, AttrRW, AttrW from fastcs.controllers import ControllerAPI -from fastcs.datatypes import DType_T, Enum, Waveform +from fastcs.datatypes import Bool, DataType, DType_T, Enum, Waveform from fastcs.logging import logger from fastcs.methods import Command from fastcs.tracer import Tracer @@ -396,6 +396,45 @@ def _get_write_enum_attr_from_type(alias: EnumMapping): return AttrW(datatype=Enum(enum)) +def _resolve_mapping( + alias: EnumMapping, datatype: DataType[DType_T] +) -> dict[str, DType_T] | None: + """Convert the mapping values into values of the attribute's datatype. + + Logs a warning and returns None if the mapping is invalid for the datatype. + """ + # Resolved once at creation, so datatype updates on aliased attributes are + # not supported + try: + # validate casts loosely (e.g. bool("false") is True), this is accepted + resolved = { + name: datatype.validate(value) for name, value in alias.mapping.items() + } + names = _reverse_mapping(resolved) + except (ValueError, TypeError) as e: + logger.warning( + "Not creating enum alias PV {pv}, as mapping is invalid for {datatype}: " + "{error}", + pv=alias.pv, + datatype=datatype, + error=e, + ) + return None + if len(names) != len(resolved): + logger.warning( + "Not creating enum alias PV {pv}, as mapping maps multiple names to the " + "same value", + pv=alias.pv, + mapping=alias.mapping, + ) + return None + return resolved + + +def _reverse_mapping(resolved: dict[str, DType_T]) -> dict[DType_T, str]: + return {value: name for name, value in resolved.items()} + + def _add_command_enum_alias( alias: EnumMapping, method: Command, @@ -404,33 +443,16 @@ def _add_command_enum_alias( if not _validate_pv_length(str(method), alias.pv): return + # validate casts loosely (e.g. bool("false") is True), this is accepted + resolved = {name: Bool().validate(value) for name, value in alias.mapping.items()} + async def trigger_command(value) -> None: logger.info("PV put: {pv} = {value}", pv=alias.pv, value=repr(value)) cast_value = cast_from_epics_type(enum_attr.datatype, value) await enum_attr.put(cast_value) - converted_value = alias.mapping.get(cast_value.name) - - if converted_value is None: - logger.warning( - "Failed to convert enum alias value {value} to command boolean. " - "No mapping exists.", - value=value, - enum_mapping=alias.mapping, - ) - return - if not isinstance(converted_value, bool): - logger.warning( - "Aliased commands only accept boolean mappings. " - "Got {value} from mapping.", - value=converted_value, - enum_mapping=alias.mapping, - ) - return - - if converted_value: + if resolved[cast_value.name]: logger.info("Calling aliased command") - await enum_attr.put(value) await _run_and_set_alarm(record, method.fn()) record = _make_out_record(alias.pv, enum_attr, on_update=trigger_command) @@ -444,16 +466,13 @@ def _add_read_enum_alias( return enum = enum_attr.datatype.dtype + resolved = _resolve_mapping(alias, attribute.datatype) + if resolved is None: + return + names = _reverse_mapping(resolved) async def convert_from_value(value) -> None: - converted_value = next( - ( - name - for name, mapped_value in alias.mapping.items() - if mapped_value == value - ), - None, - ) + converted_value = names.get(value) if converted_value is not None: validated_value = enum[converted_value] @@ -476,6 +495,9 @@ async def convert_from_value(value) -> None: ) record = _make_in_record(alias.pv, enum_attr) + initial_name = names.get(attribute.get()) + if initial_name is not None: + record.set(cast_to_epics_type(enum_attr.datatype, enum[initial_name])) attribute.add_on_update_callback(convert_from_value) @@ -487,44 +509,28 @@ def _add_write_enum_alias( if not _validate_pv_length(attribute.name, alias.pv): return + resolved = _resolve_mapping(alias, attribute.datatype) + if resolved is None: + return + names = _reverse_mapping(resolved) + async def convert_to_value(value) -> None: logger.info("PV put: {pv} = {value}", pv=alias.pv, value=repr(value)) cast_value = cast_from_epics_type(enum_attr.datatype, value) await enum_attr.put(cast_value) - converted_value = alias.mapping.get(cast_value.name) - - if converted_value is not None: - logger.info( - "Converting enum value {enum_value} to fastcs value {converted_value}", - enum_value=value, - converted_value=converted_value, - ) - await _run_and_set_alarm( - record, - attribute.put( - cast_from_epics_type(attribute.datatype, converted_value) - ), - ) - else: - logger.warning( - "Ignoring enum put value {value} as it has no " - "corresponding value in enum mapping", - value=value, - mapping=alias.mapping, - ) + converted_value = resolved[cast_value.name] + logger.info( + "Converting enum value {enum_value} to fastcs value {converted_value}", + enum_value=value, + converted_value=converted_value, + ) + await _run_and_set_alarm(record, attribute.put(converted_value)) record = _make_out_record(alias.pv, enum_attr, on_update=convert_to_value) _sync_setpoint(alias.pv, enum_attr, record) async def update_enum_alias(value: DType_T) -> None: - converted_value = next( - ( - name - for name, mapped_value in alias.mapping.items() - if mapped_value == value - ), - None, - ) + converted_value = names.get(value) if converted_value is not None: enum_value = enum_attr.datatype.dtype[converted_value] @@ -532,6 +538,13 @@ async def update_enum_alias(value: DType_T) -> None: cast_to_epics_type(enum_attr.datatype, enum_value), process=False, ) + else: + logger.warning( + "Ignoring enum setpoint sync as fastcs value {value} has no " + "corresponding value in enum mapping", + value=value, + mapping=alias.mapping, + ) if isinstance(attribute, AttrR): attribute.add_on_update_callback(update_enum_alias) From 13db3aef36b3bb390e75c66762401e39238e3871 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 9 Oct 2026 13:09:06 +0000 Subject: [PATCH 8/9] tests: add tests to cover edge cases in enum mapping --- tests/transports/epics/ca/test_softioc.py | 165 ++++++++++++++++++++++ 1 file changed, 165 insertions(+) diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 6f6cdd2fb..556896cf0 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -31,6 +31,10 @@ _create_and_link_command_pv, _create_and_link_read_pv, _create_and_link_write_pv, + _get_read_enum_attr_from_type, + _get_write_enum_attr_from_type, + _resolve_mapping, + _reverse_mapping, ) from fastcs.transports.epics.ca.util import ( _make_in_record, @@ -49,6 +53,16 @@ class OnOffStates(enum.IntEnum): ENABLED = 1 +class GapEnum(enum.IntEnum): + LOW = 1 + HIGH = 5 + + +class PlainEnum(enum.Enum): + LOW = "low" + HIGH = "high" + + async def do_nothing(): ... @@ -167,6 +181,157 @@ async def test_enum_alias_skips_pv_if_too_long(mocker: MockerFixture, alias_type make_out_record.assert_not_called() +@pytest.mark.parametrize( + "datatype,mapping,expected", + [ + (Enum(GapEnum), {"Off": 1, "On": 5}, {"Off": GapEnum.LOW, "On": GapEnum.HIGH}), + ( + Enum(PlainEnum), + {"Off": "low", "On": "high"}, + {"Off": PlainEnum.LOW, "On": PlainEnum.HIGH}, + ), + (Int(), {"Off": 0, "On": 10}, {"Off": 0, "On": 10}), + ], +) +def test_resolve_mapping_converts_to_attribute_datatype(datatype, mapping, expected): + resolved = _resolve_mapping(EnumMapping(pv="A", mapping=mapping), datatype) + + assert resolved is not None + assert resolved == expected + assert all(type(value) is type(expected[key]) for key, value in resolved.items()) + + +# EnumMapping does not validate at runtime, so untyped config can pass a list +unhashable_mapping: Any = {"a": [1, 2]} + + +@pytest.mark.parametrize( + "datatype,mapping,message", + [ + # 7 has no member in GapEnum + (Enum(GapEnum), {"On": 7}, "mapping is invalid"), + # many-to-one mapping + (Int(), {"a": 1, "b": 1}, "multiple names to the same value"), + (Waveform(np.int32, shape=(2,)), unhashable_mapping, "mapping is invalid"), + ], +) +def test_resolve_mapping_warns_and_returns_none_on_invalid_mapping( + datatype, mapping, message, loguru_caplog +): + alias = EnumMapping(pv="A", mapping=mapping) + + assert _resolve_mapping(alias, datatype) is None + assert message in loguru_caplog.text + + +@pytest.mark.parametrize("alias_type", ("read", "write")) +def test_enum_alias_skips_pv_if_mapping_invalid(mocker: MockerFixture, alias_type): + make_in_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_in_record") + make_out_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_out_record") + alias = EnumMapping(pv="A", mapping={"On": 7}) + + if alias_type == "read": + attribute = AttrR(Enum(GapEnum)) + _add_read_enum_alias(alias, attribute, _get_read_enum_attr_from_type(alias)) + else: + attribute = AttrRW(Enum(GapEnum)) + _add_write_enum_alias(alias, attribute, _get_write_enum_attr_from_type(alias)) + + make_in_record.assert_not_called() + make_out_record.assert_not_called() + + +def test_reverse_mapping(): + assert _reverse_mapping({"Off": GapEnum.LOW, "On": GapEnum.HIGH}) == { + GapEnum.LOW: "Off", + GapEnum.HIGH: "On", + } + + +@pytest.mark.parametrize("alias_index,expected", [(0, GapEnum.LOW), (1, GapEnum.HIGH)]) +@pytest.mark.asyncio +async def test_write_enum_alias_puts_mapped_value( + mocker: MockerFixture, alias_index: int, expected: GapEnum +): + make_out_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_out_record") + alias = EnumMapping(pv="A", mapping={"Off": 1, "On": 5}) + attribute = AttrRW(Enum(GapEnum)) + + _add_write_enum_alias(alias, attribute, _get_write_enum_attr_from_type(alias)) + await make_out_record.call_args.kwargs["on_update"](alias_index) + + assert attribute.get() == expected + + +@pytest.mark.asyncio +async def test_write_enum_alias_syncs_from_attribute(mocker: MockerFixture): + record = mocker.patch("fastcs.transports.epics.ca.ioc._make_out_record")() + alias = EnumMapping(pv="A", mapping={"Off": 1, "On": 5}) + attribute = AttrRW(Enum(GapEnum)) + + _add_write_enum_alias(alias, attribute, _get_write_enum_attr_from_type(alias)) + await attribute.update(GapEnum.HIGH) + + record.set.assert_called_with(1, process=False) + + +@pytest.mark.asyncio +async def test_write_enum_alias_warns_on_unmapped_value( + mocker: MockerFixture, loguru_caplog +): + record = mocker.patch("fastcs.transports.epics.ca.ioc._make_out_record")() + alias = EnumMapping(pv="A", mapping={"Off": 1, "On": 5}) + attribute = AttrRW(Int()) + + _add_write_enum_alias(alias, attribute, _get_write_enum_attr_from_type(alias)) + record.set.reset_mock() + await attribute.update(3) + + record.set.assert_not_called() + assert "Ignoring enum setpoint sync" in loguru_caplog.text + + +@pytest.mark.asyncio +async def test_read_enum_alias_sets_record_from_plain_enum(mocker: MockerFixture): + record = mocker.patch("fastcs.transports.epics.ca.ioc._make_in_record")() + alias = EnumMapping(pv="A", mapping={"Off": "low", "On": "high"}) + attribute = AttrR(Enum(PlainEnum)) + + _add_read_enum_alias(alias, attribute, _get_read_enum_attr_from_type(alias)) + await attribute.update(PlainEnum.HIGH) + + record.set.assert_called_with(1) + + +def test_read_enum_alias_sets_record_from_initial_value(mocker: MockerFixture): + record = mocker.patch("fastcs.transports.epics.ca.ioc._make_in_record")() + alias = EnumMapping(pv="A", mapping={"Off": 1, "On": 5}) + attribute = AttrR(Enum(GapEnum), initial_value=GapEnum.HIGH) + + _add_read_enum_alias(alias, attribute, _get_read_enum_attr_from_type(alias)) + + record.set.assert_called_once_with(1) + + +@pytest.mark.parametrize("alias_index,called", [(0, False), (1, True)]) +@pytest.mark.asyncio +async def test_command_enum_alias_converts_mapping_to_bool( + mocker: MockerFixture, alias_index: int, called: bool +): + make_out_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_out_record") + calls = [] + + async def fn(): + calls.append(True) + + alias = EnumMapping(pv="A", mapping={"Idle": 0, "Go": 1}) + + _add_command_enum_alias(alias, Command(fn), _get_write_enum_attr_from_type(alias)) + await make_out_record.call_args.kwargs["on_update"](alias_index) + + assert bool(calls) is called + + @pytest.mark.asyncio async def test_ioc_raises_if_duplicate_aliases_provided(mocker: MockerFixture): aliases = {"A": "Alias", "B": "Alias"} From e38ae41c99ea0beec6e596862adb5b7c525c8e32 Mon Sep 17 00:00:00 2001 From: Shihab Suliman Date: Fri, 9 Oct 2026 13:30:11 +0000 Subject: [PATCH 9/9] fix: log on failed casts to epics types instead of exception --- src/fastcs/transports/epics/ca/ioc.py | 33 +++++++++++--- tests/transports/epics/ca/test_softioc.py | 52 ++++++++++++++++++++++- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/src/fastcs/transports/epics/ca/ioc.py b/src/fastcs/transports/epics/ca/ioc.py index 8c251c3f0..ddc169046 100644 --- a/src/fastcs/transports/epics/ca/ioc.py +++ b/src/fastcs/transports/epics/ca/ioc.py @@ -241,9 +241,10 @@ def _create_and_link_write_pv( async def on_update(value): logger.info("PV put: {pv} = {value}", pv=pv, value=repr(value)) - await _run_and_set_alarm( - record, attribute.put(cast_from_epics_type(attribute.datatype, value)) - ) + cast_value = _cast_put(record, attribute.datatype, pv, value) + if cast_value is None: + return + await _run_and_set_alarm(record, attribute.put(cast_value)) record = _make_out_record(pv, attribute, on_update=on_update) @@ -355,6 +356,24 @@ def _add_alias(record: RecordWrapper, alias: str, attr_name: str): record.add_alias(alias) +def _cast_put( + record: RecordWrapper, datatype: DataType[DType_T], pv: str, value +) -> DType_T | None: + """Cast an EPICS put value, or log and raise a MAJOR_ALARM if it is invalid.""" + try: + return cast_from_epics_type(datatype, value) + except (IndexError, ValueError) as e: + logger.warning( + "Ignoring put {value} to {pv} as it is invalid for {datatype}: {error}", + value=value, + pv=pv, + datatype=datatype, + error=e, + ) + _set_alarm(record, alarm.MAJOR_ALARM) + return None + + def _set_alarm(record: RecordWrapper, alarm_state: int): record.set( record.get(), @@ -448,7 +467,9 @@ def _add_command_enum_alias( async def trigger_command(value) -> None: logger.info("PV put: {pv} = {value}", pv=alias.pv, value=repr(value)) - cast_value = cast_from_epics_type(enum_attr.datatype, value) + cast_value = _cast_put(record, enum_attr.datatype, alias.pv, value) + if cast_value is None: + return await enum_attr.put(cast_value) if resolved[cast_value.name]: @@ -516,7 +537,9 @@ def _add_write_enum_alias( async def convert_to_value(value) -> None: logger.info("PV put: {pv} = {value}", pv=alias.pv, value=repr(value)) - cast_value = cast_from_epics_type(enum_attr.datatype, value) + cast_value = _cast_put(record, enum_attr.datatype, alias.pv, value) + if cast_value is None: + return await enum_attr.put(cast_value) converted_value = resolved[cast_value.name] logger.info( diff --git a/tests/transports/epics/ca/test_softioc.py b/tests/transports/epics/ca/test_softioc.py index 556896cf0..767649786 100644 --- a/tests/transports/epics/ca/test_softioc.py +++ b/tests/transports/epics/ca/test_softioc.py @@ -5,7 +5,7 @@ import numpy as np import pytest from pytest_mock import MockerFixture -from softioc import softioc +from softioc import alarm, softioc from tests.assertable_controller import ( AssertableControllerAPI, MyTestAttributeIORef, @@ -496,6 +496,56 @@ async def test_create_and_link_write_pv(mocker: MockerFixture): attribute.put.assert_called_once_with(1) +@pytest.mark.asyncio +async def test_write_pv_invalid_enum_index_put_sets_alarm( + mocker: MockerFixture, loguru_caplog +): + make_out_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_out_record") + set_alarm = mocker.patch("fastcs.transports.epics.ca.ioc._set_alarm") + attribute = AttrW(Enum(GapEnum)) + attribute.put = mocker.AsyncMock() + + _create_and_link_write_pv("PREFIX", "PV", "attr", None, attribute) + # GapEnum only has 2 members + await make_out_record.call_args.kwargs["on_update"](4) + + attribute.put.assert_not_called() + set_alarm.assert_called_once_with(make_out_record.return_value, alarm.MAJOR_ALARM) + assert "Ignoring put 4" in loguru_caplog.text + + +@pytest.mark.parametrize("alias_type", ("write", "command")) +@pytest.mark.asyncio +async def test_enum_alias_invalid_index_put_sets_alarm( + mocker: MockerFixture, alias_type: str, loguru_caplog +): + make_out_record = mocker.patch("fastcs.transports.epics.ca.ioc._make_out_record") + set_alarm = mocker.patch("fastcs.transports.epics.ca.ioc._set_alarm") + record = make_out_record.return_value + + alias = EnumMapping(pv="A", mapping={"Off": 1, "On": 5}) + attribute = AttrW(Enum(GapEnum)) + attribute.put = mocker.AsyncMock() + calls = [] + + async def fn(): + calls.append(True) + + enum_attr = _get_write_enum_attr_from_type(alias) + if alias_type == "write": + _add_write_enum_alias(alias, attribute, enum_attr) + else: + _add_command_enum_alias(alias, Command(fn), enum_attr) + + # The alias enum only has 2 members + await make_out_record.call_args.kwargs["on_update"](4) + + attribute.put.assert_not_called() + assert not calls + set_alarm.assert_called_once_with(record, alarm.MAJOR_ALARM) + assert "Ignoring put 4" in loguru_caplog.text + + class LongEnum(enum.Enum): THIS = 0 IS = 1