Skip to content

Commit

Permalink
Convert float prec 0 pvs to int on request (#718)
Browse files Browse the repository at this point in the history
You can specify SignalRW[int] for a float pv with prec 0, but this wasn't actually doing the conversion internally which would put the wrong datatype in the documents. Change this to convert to a python float.
  • Loading branch information
coretl authored Jan 6, 2025
1 parent 6c42a1f commit 4f4458c
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 36 deletions.
13 changes: 11 additions & 2 deletions src/ophyd_async/epics/core/_aioca.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ def _metadata_from_augmented_value(
metadata = metadata.copy()
if hasattr(value, "units") and datatype not in (str, bool):
metadata["units"] = value.units
if hasattr(value, "precision") and not isnan(value.precision):
if (
hasattr(value, "precision")
and not isnan(value.precision)
and datatype is not int
):
metadata["precision"] = value.precision
if (limits := _limits_from_augmented_value(value)) and datatype is not bool:
metadata["limits"] = limits
Expand Down Expand Up @@ -102,6 +106,11 @@ def __getattribute__(self, __name: str) -> Any:
raise NotImplementedError("No PV has been set as connect() has not been called")


class CaIntConverter(CaConverter[int]):
def value(self, value: AugmentedValue) -> int:
return int(value) # type: ignore


class CaArrayConverter(CaConverter[np.ndarray]):
def value(self, value: AugmentedValue) -> np.ndarray:
# A less expensive conversion
Expand Down Expand Up @@ -204,7 +213,7 @@ def make_converter(
and get_unique({k: v.precision for k, v in values.items()}, "precision") == 0
):
# Allow int signals to represent float records when prec is 0
return CaConverter(int, pv_dbr)
return CaIntConverter(int, pv_dbr)
elif datatype in (None, inferred_datatype):
# If datatype matches what we are given then allow it and use inferred converter
return converter_cls(inferred_datatype, pv_dbr)
Expand Down
16 changes: 12 additions & 4 deletions src/ophyd_async/epics/core/_p4p.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def _metadata_from_value(datatype: type[SignalDatatype], value: Any) -> SignalMe
hasattr(display_data, "precision")
and not isnan(display_data.precision)
and specifier[-1] in _float_specifiers
and datatype is not int
):
metadata["precision"] = display_data.precision
if (limits := _limits_from_value(value)) and specifier[-1] in _number_specifiers:
Expand All @@ -93,16 +94,23 @@ def __init__(self, datatype: type[SignalDatatypeT]):
self.datatype = datatype

def value(self, value: Any) -> SignalDatatypeT:
# for channel access ca_xxx classes, this
# invokes __pos__ operator to return an instance of
# the builtin base class
# Normally the value will be of the correct python type
return value["value"]

def write_value(self, value: Any) -> Any:
# The pva library will do the conversion for us
return value


class PvaIntConverter(PvaConverter[int]):
def __init__(self):
super().__init__(int)

def value(self, value: Any) -> int:
# Convert to an int
return int(value["value"])


class PvaLongStringConverter(PvaConverter[str]):
def __init__(self):
super().__init__(str)
Expand Down Expand Up @@ -270,7 +278,7 @@ def make_converter(datatype: type | None, values: dict[str, Any]) -> PvaConverte
== 0
):
# Allow int signals to represent float records when prec is 0
return PvaConverter(int)
return PvaIntConverter()
elif inferred_datatype is str and (enum_cls := get_enum_cls(datatype)):
# Allow strings to be used as enums until QSRV supports this
return PvaConverter(str)
Expand Down
1 change: 1 addition & 0 deletions src/ophyd_async/epics/testing/_example_ioc.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class EpicsTestTable(Table):
class EpicsTestCaDevice(EpicsDevice):
my_int: A[SignalRW[int], PvSuffix("int")]
my_float: A[SignalRW[float], PvSuffix("float")]
float_prec_0: A[SignalRW[int], PvSuffix("float_prec_0")]
my_str: A[SignalRW[str], PvSuffix("str")]
longstr: A[SignalRW[str], PvSuffix("longstr")]
longstr2: A[SignalRW[str], PvSuffix("longstr2.VAL$")]
Expand Down
2 changes: 2 additions & 0 deletions src/ophyd_async/testing/_assert.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ def __init__(self, signal: SignalR):

async def assert_updates(self, expected_value):
# Get an update, value and reading
expected_type = type(expected_value)
if isinstance(expected_value, Table):
expected_value = ApproxTable(expected_value)
else:
Expand All @@ -191,6 +192,7 @@ async def assert_updates(self, expected_value):
reading = await self.signal.read()
# Check they match what we expected
assert value == expected_value
assert type(value) is expected_type
expected_reading = {
self.signal.name: {
"value": expected_value,
Expand Down
115 changes: 85 additions & 30 deletions tests/epics/signal/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,37 +152,81 @@ async def assert_monitor_then_put(
),
"my_float": ExpectedData(3.141, 43.5, "number", "<f8", precision=1, units="mm"),
"my_str": ExpectedData("hello", "goodbye", "string", "|S40"),
"enum": ExpectedData(
EpicsTestEnum.B,
EpicsTestEnum.C,
"string",
"|S40",
choices=["Aaa", "Bbb", "Ccc"],
"uint8a": ExpectedData(
np.array([0, 255], dtype=np.uint8),
np.array([218], dtype=np.uint8),
"array",
"|u1",
units="",
),
"subset_enum": ExpectedData(
EpicsTestSubsetEnum.B,
EpicsTestSubsetEnum.A,
"string",
"|S40",
choices=["Aaa", "Bbb", "Ccc"],
"int16a": ExpectedData(
np.array([-32768, 32767], dtype=np.int16),
np.array([-855], dtype=np.int16),
"array",
"<i2",
units="",
),
"int32a": ExpectedData(
np.array([-2147483648, 2147483647], dtype=np.int32),
np.array([-2], dtype=np.int32),
"array",
"<i4",
units="",
),
"uint8a": ExpectedData([0, 255], [218], "array", "|u1", units=""),
"int16a": ExpectedData([-32768, 32767], [-855], "array", "<i2", units=""),
"int32a": ExpectedData([-2147483648, 2147483647], [-2], "array", "<i4", units=""),
"float32a": ExpectedData(
[0.000002, -123.123], [1.0], "array", "<f4", units="", precision=0
np.array([0.000002, -123.123], dtype=np.float32),
np.array([1.0], dtype=np.float32),
"array",
"<f4",
units="",
precision=0,
),
"float64a": ExpectedData(
[0.1, -12345678.123], [0.2], "array", "<f8", units="", precision=0
np.array([0.1, -12345678.123], dtype=np.float64),
np.array([0.2], dtype=np.float64),
"array",
"<f8",
units="",
precision=0,
),
"stra": ExpectedData(["five", "six", "seven"], ["nine", "ten"], "array", "|S40"),
}
PVA_INFERRED = {
"int8a": ExpectedData([-128, 127], [-8, 3, 44], "array", "|i1", units=""),
"uint16a": ExpectedData([0, 65535], [5666], "array", "<u2", units=""),
"uint32a": ExpectedData([0, 4294967295], [1022233], "array", "<u4", units=""),
"int64a": ExpectedData([-2147483649, 2147483648], [-3], "array", "<i8", units=""),
"uint64a": ExpectedData([0, 4294967297], [995444], "array", "<u8", units=""),
"int8a": ExpectedData(
np.array([-128, 127], dtype=np.int8),
np.array([-8, 3, 44], dtype=np.int8),
"array",
"|i1",
units="",
),
"uint16a": ExpectedData(
np.array([0, 65535], dtype=np.uint16),
np.array([5666], dtype=np.uint16),
"array",
"<u2",
units="",
),
"uint32a": ExpectedData(
np.array([0, 4294967295], dtype=np.uint32),
np.array([1022233], dtype=np.uint32),
"array",
"<u4",
units="",
),
"int64a": ExpectedData(
np.array([-2147483649, 2147483648], dtype=np.int64),
np.array([-3], dtype=np.int64),
"array",
"<i8",
units="",
),
"uint64a": ExpectedData(
np.array([0, 4294967297], dtype=np.uint64),
np.array([995444], dtype=np.uint64),
"array",
"<u8",
units="",
),
}


Expand Down Expand Up @@ -222,6 +266,21 @@ async def test_epics_get_put_monitor_for_inferred_types(
),
"my_bool": ExpectedData(True, False, "boolean", dtype_numpy="|b1"),
"bool_unnamed": ExpectedData(True, False, "boolean", dtype_numpy="|b1"),
"enum": ExpectedData(
EpicsTestEnum.B,
EpicsTestEnum.C,
"string",
"|S40",
choices=["Aaa", "Bbb", "Ccc"],
),
"subset_enum": ExpectedData(
EpicsTestSubsetEnum.B,
EpicsTestSubsetEnum.A,
"string",
"|S40",
choices=["Aaa", "Bbb", "Ccc"],
),
"float_prec_0": ExpectedData(3, 4, "integer", default_int_type, units="mm"),
}
PVA_OVERRIDE = {}

Expand Down Expand Up @@ -557,14 +616,6 @@ def test_signal_helpers():
assert _get_epics_backend(execute).write_pv == "Execute"


@pytest.mark.parametrize("protocol", get_args(Protocol))
async def test_signals_created_for_prec_0_float_can_use_int(
ioc_devices: EpicsTestIocAndDevices, protocol: Protocol
):
sig = epics_signal_rw(int, ioc_devices.get_pv(protocol, "float_prec_0"))
await sig.connect()


@pytest.mark.parametrize("protocol", get_args(Protocol))
async def test_signals_created_for_not_prec_0_float_cannot_use_int(
ioc_devices: EpicsTestIocAndDevices, protocol: Protocol
Expand Down Expand Up @@ -650,6 +701,10 @@ def my_plan():
yield from store_settings(tmp_provider, "test_file", device)
with open(tmp_path / "test_file.yaml") as actual_file:
with open(HERE / f"test_yaml_save_{protocol}.yaml") as expected_file:
# If this test fails because you added a signal, then you can regenerate
# the test data with:
# cp /tmp/pytest-of-root/pytest-current/test_retrieve_apply_store_sett0/test_file.yaml tests/epics/signal/test_yaml_save_ca.yaml # noqa: E501
# cp /tmp/pytest-of-root/pytest-current/test_retrieve_apply_store_sett1/test_file.yaml tests/epics/signal/test_yaml_save_pva.yaml # noqa: E501
assert yaml.safe_load(actual_file) == yaml.safe_load(expected_file)

RE(my_plan())
1 change: 1 addition & 0 deletions tests/epics/signal/test_yaml_save_ca.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ enum: Bbb
enum2: Bbb
float32a: [1.9999999949504854e-06, -123.12300109863281]
float64a: [0.1, -12345678.123]
float_prec_0: 3
int16a: [-32768, 32767]
int32a: [-2147483648, 2147483647]
lessint: 42
Expand Down
1 change: 1 addition & 0 deletions tests/epics/signal/test_yaml_save_pva.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ enum: Bbb
enum2: Bbb
float32a: [1.9999999949504854e-06, -123.12300109863281]
float64a: [0.1, -12345678.123]
float_prec_0: 3.0
int16a: [-32768, 32767]
int32a: [-2147483648, 2147483647]
int64a: [-2147483649, 2147483648]
Expand Down

0 comments on commit 4f4458c

Please sign in to comment.