Skip to content

Split epics make record #319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ajgdls wants to merge 10 commits into
base: main
Choose a base branch
from
+285 −169
Open

Split epics make record #319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
898eb45
Remove builder_callable_from_attribute method.
ajgdls Feb 10, 2026
08bfce5
Split _make_record into _make_in_record and _make_out_record.
ajgdls Feb 10, 2026
00a518c
Improve test coverage
ajgdls Feb 11, 2026
7e13aba
Removed empty update dict and commented out code.
ajgdls Feb 11, 2026
37c79df
Test improvement.
ajgdls Feb 11, 2026
79e96b2
Fixed EPICs test that was creating the wrong record type.
ajgdls Feb 11, 2026
333dfde
Improvements to EPICS record creation readability.
ajgdls Feb 13, 2026
39c7deb
Added docstring
ajgdls Feb 13, 2026
f503e60
Remove print statements.
ajgdls Feb 13, 2026
ee109fb
Filter out datatype update keys that would cause a runtime error.
ajgdls Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
181 changes: 157 additions & 24 deletions src/fastcs/transports/epics/ca/ioc.py
View file
Open in desktop
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)
Copy link
Contributor

@GDYendell GDYendell Feb 17, 2026

Choose a reason for hiding this comment

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

  • Should this should be the same as the other instance of datatype_updater in removing the non-fields?
    • If so, it could just be one free function and here we create partial that provides the record
    • Maybe it then makes sense to just merge datatype_updater and record_metadata_from_datatype, unless it is easier for testing?
  • Is it definitely the case that we can't call set_field with length or validate?
  • Given the above, should record_metadata_from_datatype even return validate and length?

Copy link
Contributor Author

@ajgdls ajgdls Feb 17, 2026

Choose a reason for hiding this comment

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

I believe that set_field casts the value as a string, and so calling it with validate would fail.
I would agree that record_metadata_from_datatype should therefore not return validate. Investigating length further.


Comment on lines +252 to +255
Copy link

@coderabbitai coderabbitai bot Feb 13, 2026

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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