Skip to content
181 changes: 157 additions & 24 deletions src/fastcs/transports/epics/ca/ioc.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,19 @@
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,
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
Expand Down Expand Up @@ -187,36 +188,168 @@ 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) -> 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, ZNAM="False", ONAM="True", **attribute_record_metadata
)
case Int():
record = builder.longIn(
pv,
LOPR=attribute.datatype.min_alarm,
HOPR=attribute.datatype.max_alarm,
EGU=attribute.datatype.units,
**attribute_record_metadata,
)
case Float():
record = builder.aIn(
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,
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,
**attribute_record_metadata,
)
else:
attribute_record_metadata.update(create_state_keys(attribute.datatype))
record = builder.mbbIn(
pv,
**attribute_record_metadata,
)
case Waveform():
record = builder.WaveformIn(
pv, length=attribute.datatype.shape[0], **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)

Comment on lines +252 to +255
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n -A40 "def record_metadata_from_datatype" src/fastcs/transports/epics/ca/util.py

Repository: DiamondLightSource/FastCS

Length of output: 1651


🏁 Script executed:

rg -n "def _make_in_record" -A20 src/fastcs/transports/epics/ca/ioc.py

Repository: DiamondLightSource/FastCS

Length of output: 896


🏁 Script executed:

rg -n "def _make_out_record" -A25 src/fastcs/transports/epics/ca/ioc.py

Repository: DiamondLightSource/FastCS

Length of output: 971


🏁 Script executed:

rg -n "builder_only_keys" src/fastcs/transports/epics/ca/ioc.py

Repository: DiamondLightSource/FastCS

Length of output: 169


🏁 Script executed:

rg -n "def _make_in_record" -A50 src/fastcs/transports/epics/ca/ioc.py

Repository: DiamondLightSource/FastCS

Length of output: 2160


🏁 Script executed:

rg -n -B5 -A15 "def datatype_updater" src/fastcs/transports/epics/ca/ioc.py

Repository: DiamondLightSource/FastCS

Length of output: 1572


Add filtering for builder-only keys in _make_in_record datatype updater.

The datatype_updater in _make_in_record (lines 252–254) calls record_metadata_from_datatype(datatype) without filtering. For String and Waveform types, this returns the "length" key, which is a builder-only metadata field that cannot be updated via record.set_field() after record creation. When a datatype is updated dynamically, this will cause a runtime error.

Apply the same filtering used in _make_out_record:

🔧 Suggested fix
     def datatype_updater(datatype: DataType):
+        builder_only_keys = {"validate", "length"}
         for name, value in record_metadata_from_datatype(datatype).items():
+            if name in builder_only_keys:
+                continue
             record.set_field(name, value)
🤖 Prompt for AI Agents
In `@src/fastcs/transports/epics/ca/ioc.py` around lines 252 - 255, In
_make_in_record, update the inner function datatype_updater(datatype: DataType)
to filter out builder-only metadata keys (e.g., "length" returned by
record_metadata_from_datatype for String/Waveform) before calling
record.set_field; locate datatype_updater in _make_in_record and apply the same
filtering logic used in _make_out_record so you only iterate and call
record.set_field for allowed (non-builder) keys.

attribute.add_update_datatype_callback(datatype_updater)
return record


def _make_out_record(
pv: str,
attribute: AttrR | AttrW | AttrRW,
on_update: Callable | None = None,
out_record: bool = False,
attribute: AttrW | AttrRW,
on_update: Callable,
) -> RecordWrapper:
builder_callable = builder_callable_from_attribute(attribute, on_update is None)
datatype_record_metadata = record_metadata_from_datatype(
attribute.datatype, out_record
)
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}
if on_update
else {}
)
update = {"on_update": on_update, "always_update": True, "blocking": True}

match attribute.datatype:
case Bool():
record = builder.boolOut(
pv, ZNAM="False", ONAM="True", **update, **attribute_record_metadata
)
case Int():
record = builder.longOut(
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,
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,
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,
**attribute_record_metadata,
)

record = builder_callable(
pv, **update, **datatype_record_metadata, **attribute_record_metadata
)
else:
attribute_record_metadata.update(create_state_keys(attribute.datatype))
record = builder.mbbOut(
pv,
**update,
**attribute_record_metadata,
)
case Waveform():
record = builder.WaveformOut(
pv,
length=attribute.datatype.shape[0],
**update,
**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():
# 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)
Expand All @@ -240,7 +373,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")

Expand Down
65 changes: 12 additions & 53 deletions src/fastcs/transports/epics/ca/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,8 @@
from dataclasses import asdict
from typing import Any

from softioc import builder

from fastcs.attributes import Attribute, AttrR, AttrRW, 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",
Expand Down Expand Up @@ -46,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]:
Expand Down Expand Up @@ -90,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):
Expand All @@ -111,6 +85,17 @@ def _verify_in_datatype(_, value):
return arguments


def create_state_keys(datatype: Enum):
"""Creates a dictionary of state field keys to names"""
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:
Expand Down Expand Up @@ -154,29 +139,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}"
)
18 changes: 0 additions & 18 deletions tests/transports/epics/ca/test_ca_util.py
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 5 additions & 3 deletions tests/transports/epics/ca/test_initial_value.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 or 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,
Expand Down
Loading