From a0f3990a61ce8e4fa52187c2be934e7fd3a4087f Mon Sep 17 00:00:00 2001 From: Joseph Ware Date: Thu, 14 Mar 2024 15:28:40 +0000 Subject: [PATCH 01/11] Use information from signal backend to populate DataKey --- src/ophyd_async/epics/_backend/_aioca.py | 72 ++++++++++++--- tests/epics/test_signals.py | 111 ++++++++++++++--------- 2 files changed, 126 insertions(+), 57 deletions(-) diff --git a/src/ophyd_async/epics/_backend/_aioca.py b/src/ophyd_async/epics/_backend/_aioca.py index 00cb0cbfc3..1cda7c2d2e 100644 --- a/src/ophyd_async/epics/_backend/_aioca.py +++ b/src/ophyd_async/epics/_backend/_aioca.py @@ -2,6 +2,7 @@ import sys from dataclasses import dataclass from enum import Enum +from math import isnan from typing import Any, Dict, Optional, Sequence, Type, Union from aioca import ( @@ -37,7 +38,62 @@ dbr.DBR_CHAR: "string", dbr.DBR_LONG: "integer", dbr.DBR_DOUBLE: "number", + dbr.DBR_ENUM: "string", } +_num_dbr = {dbr.DBR_SHORT, dbr.DBR_CHAR, dbr.DBR_LONG, dbr.DBR_FLOAT, dbr.DBR_DOUBLE} +_floating_dbr = {dbr.DBR_FLOAT, dbr.DBR_DOUBLE} + + +def _data_key_from_augmented_value(source: str, value: AugmentedValue) -> Descriptor: + """Use the return value of get with FORMAT_CTRL to construct a Descriptor + describing the signal. See docstring of AugmentedValue for expected + value fields by DBR type. + + Args: + value (AugmentedValue): Description of the the return type of a DB record + + Returns: + Descriptor: A rich DataKey describing the DB record + """ + + if not value.ok: + raise Exception(f"{source} error: {value.errorcode}: {str(value)}") + + scalar = value.element_count == 1 + + d = Descriptor( + source=source, + dtype=dbr_to_dtype[value.datatype] if scalar else "array", + shape=[] if scalar else [len(value)], + ) + + if value.datatype in _num_dbr: + d["units"] = value.units + if not isnan(value.lower_alarm_limit): + d["lower_alarm_limit"] = value.lower_alarm_limit + if not isnan(value.upper_alarm_limit): + d["upper_alarm_limit"] = value.upper_alarm_limit + if not isnan(value.lower_ctrl_limit): + d["lower_ctrl_limit"] = value.lower_ctrl_limit + if not isnan(value.upper_ctrl_limit): + d["upper_ctrl_limit"] = value.upper_ctrl_limit + if not isnan(value.lower_disp_limit): + d["lower_disp_limit"] = value.lower_disp_limit + if not isnan(value.upper_disp_limit): + d["upper_disp_limit"] = value.upper_disp_limit + if not isnan(value.lower_warning_limit): + d["lower_warning_limit"] = value.lower_warning_limit + if not isnan(value.upper_warning_limit): + d["upper_warning_limit"] = value.upper_warning_limit + + if value.datatype in _floating_dbr and not isnan(value.precision): + d["precision"] = value.precision + if value.datatype is dbr.DBR_ENUM: + d["choices"] = value.enums + if value.datatype is dbr.DBR_STRING and not isnan(value.timestamp): + d["timestamp"] = value.timestamp + + return d @dataclass @@ -59,7 +115,7 @@ def reading(self, value: AugmentedValue): ) def descriptor(self, source: str, value: AugmentedValue) -> Descriptor: - return dict(source=source, dtype=dbr_to_dtype[value.datatype], shape=[]) + return _data_key_from_augmented_value(source, value) class CaLongStrConverter(CaConverter): @@ -72,11 +128,6 @@ def write_value(self, value: str): return value + "\0" -class CaArrayConverter(CaConverter): - def descriptor(self, source: str, value: AugmentedValue) -> Descriptor: - return dict(source=source, dtype="array", shape=[len(value)]) - - @dataclass class CaEnumConverter(CaConverter): enum_class: Type[Enum] @@ -90,11 +141,6 @@ def write_value(self, value: Union[Enum, str]): def value(self, value: AugmentedValue): return self.enum_class(value) - def descriptor(self, source: str, value: AugmentedValue) -> Descriptor: - choices = [e.value for e in self.enum_class] - return dict(source=source, dtype="string", shape=[], choices=choices) - - class DisconnectedCaConverter(CaConverter): def __getattribute__(self, __name: str) -> Any: raise NotImplementedError("No PV has been set as connect() has not been called") @@ -113,7 +159,7 @@ def make_converter( # Waveform of strings, check we wanted this if datatype and datatype != Sequence[str]: raise TypeError(f"{pv} has type [str] not {datatype.__name__}") - return CaArrayConverter(pv_dbr, None) + return CaConverter(pv_dbr, None) elif is_array: pv_dtype = get_unique({k: v.dtype for k, v in values.items()}, "dtypes") # This is an array @@ -124,7 +170,7 @@ def make_converter( raise TypeError(f"{pv} has type [{pv_dtype}] not {datatype.__name__}") if dtype != pv_dtype: raise TypeError(f"{pv} has type [{pv_dtype}] not [{dtype}]") - return CaArrayConverter(pv_dbr, None) + return CaConverter(pv_dbr, None) elif pv_dbr == dbr.DBR_ENUM and datatype is bool: # Database can't do bools, so are often representated as enums, CA can do int pv_choices_len = get_unique( diff --git a/tests/epics/test_signals.py b/tests/epics/test_signals.py index 2c00bde508..7c7bafcd8b 100644 --- a/tests/epics/test_signals.py +++ b/tests/epics/test_signals.py @@ -9,17 +9,7 @@ from dataclasses import dataclass from enum import Enum from pathlib import Path -from typing import ( - Any, - Callable, - Dict, - Literal, - Optional, - Sequence, - Tuple, - Type, - TypedDict, -) +from typing import Any, Dict, List, Literal, Optional, Sequence, Tuple, Type, TypedDict from unittest.mock import ANY import numpy as np @@ -156,24 +146,48 @@ class MyEnum(str, Enum): c = "Ccc" -def integer_d(value): - return dict(dtype="integer", shape=[]) - - -def number_d(value): - return dict(dtype="number", shape=[]) - - -def string_d(value): - return dict(dtype="string", shape=[]) +ca_metadata = { + "integer": { + "lower_alarm_limit": 0, + "lower_ctrl_limit": 10, + "lower_disp_limit": 0, + "lower_warning_limit": 0, + "units": "", + "upper_alarm_limit": 0, + "upper_ctrl_limit": 90, + "upper_disp_limit": 100, + "upper_warning_limit": 0, + }, + "number": { + "lower_ctrl_limit": 0.0, + "lower_disp_limit": 0.0, + "precision": 1, + "units": "mm", + "upper_ctrl_limit": 0.0, + "upper_disp_limit": 0.0, + }, + "string": {"timestamp": ANY}, +} -def enum_d(value): - return dict(dtype="string", shape=[], choices=["Aaa", "Bbb", "Ccc"]) +def suffix_to_dtype(suffix: str): + if "float" in suffix: + return "number" + if "int" in suffix: + return "integer" + return "string" -def waveform_d(value): - return dict(dtype="array", shape=[len(value)]) +def descriptor(protocol: str, suffix: str, value: Any): + is_array = isinstance(value, List) + dtype = suffix_to_dtype(suffix) + d = { + "dtype": "array" if is_array else dtype, + "shape": [len(value)] if isinstance(value, List) else [], + } + if protocol == "ca": + d.update(ca_metadata.get(dtype, {})) + return d ls1 = "a string that is just longer than forty characters" @@ -191,21 +205,21 @@ def waveform_d(value): @pytest.mark.parametrize( "datatype, suffix, initial_value, put_value, descriptor", [ - (int, "int", 42, 43, integer_d), - (float, "float", 3.141, 43.5, number_d), - (str, "str", "hello", "goodbye", string_d), - (MyEnum, "enum", MyEnum.b, MyEnum.c, enum_d), - (npt.NDArray[np.int8], "int8a", [-128, 127], [-8, 3, 44], waveform_d), - (npt.NDArray[np.uint8], "uint8a", [0, 255], [218], waveform_d), - (npt.NDArray[np.int16], "int16a", [-32768, 32767], [-855], waveform_d), - (npt.NDArray[np.uint16], "uint16a", [0, 65535], [5666], waveform_d), - (npt.NDArray[np.int32], "int32a", [-2147483648, 2147483647], [-2], waveform_d), - (npt.NDArray[np.uint32], "uint32a", [0, 4294967295], [1022233], waveform_d), - (npt.NDArray[np.int64], "int64a", [-2147483649, 2147483648], [-3], waveform_d), - (npt.NDArray[np.uint64], "uint64a", [0, 4294967297], [995444], waveform_d), - (npt.NDArray[np.float32], "float32a", [0.000002, -123.123], [1.0], waveform_d), - (npt.NDArray[np.float64], "float64a", [0.1, -12345678.123], [0.2], waveform_d), - (Sequence[str], "stra", ["five", "six", "seven"], ["nine", "ten"], waveform_d), + (int, "int", 42, 43), + (float, "float", 3.141, 43.5), + (str, "str", "hello", "goodbye"), + (MyEnum, "enum", MyEnum.b, MyEnum.c), + (npt.NDArray[np.int8], "int8a", [-128, 127], [-8, 3, 44]), + (npt.NDArray[np.uint8], "uint8a", [0, 255], [218]), + (npt.NDArray[np.int16], "int16a", [-32768, 32767], [-855]), + (npt.NDArray[np.uint16], "uint16a", [0, 65535], [5666]), + (npt.NDArray[np.int32], "int32a", [-2147483648, 2147483647], [-2]), + (npt.NDArray[np.uint32], "uint32a", [0, 4294967295], [1022233]), + (npt.NDArray[np.int64], "int64a", [-2147483649, 2147483648], [-3]), + (npt.NDArray[np.uint64], "uint64a", [0, 4294967297], [995444]), + (npt.NDArray[np.float32], "float32a", [0.000002, -123.123], [1.0]), + (npt.NDArray[np.float64], "float64a", [0.1, -12345678.123], [0.2]), + (Sequence[str], "stra", ["five", "six", "seven"], ["nine", "ten"]), # Can't do long strings until https://github.com/epics-base/pva2pva/issues/17 # (str, "longstr", ls1, ls2, string_d), # (str, "longstr2.VAL$", ls1, ls2, string_d), @@ -217,7 +231,6 @@ async def test_backend_get_put_monitor( suffix: str, initial_value: T, put_value: T, - descriptor: Callable[[Any], dict], tmp_path, ): # ca can't support all the types @@ -233,11 +246,21 @@ async def test_backend_get_put_monitor( # With the given datatype, check we have the correct initial value and putting # works await assert_monitor_then_put( - ioc, suffix, descriptor(initial_value), initial_value, put_value, datatype + ioc, + suffix, + descriptor(ioc.protocol, suffix, initial_value), + initial_value, + put_value, + datatype, ) # With datatype guessed from CA/PVA, check we can set it back to the initial value await assert_monitor_then_put( - ioc, suffix, descriptor(put_value), put_value, initial_value, datatype=None + ioc, + suffix, + descriptor(ioc.protocol, suffix, put_value), + put_value, + initial_value, + datatype=None, ) yaml_path = tmp_path / "test.yaml" @@ -251,7 +274,7 @@ async def test_bool_conversion_of_enum(ioc: IOC, suffix: str) -> None: await assert_monitor_then_put( ioc, suffix=suffix, - descriptor=integer_d(True), + descriptor=descriptor(ioc.protocol, "bool", True), initial_value=True, put_value=False, datatype=bool, From b6c56f55f2591fe673cef298f9c968eeb2c579a3 Mon Sep 17 00:00:00 2001 From: Joseph Ware Date: Thu, 14 Mar 2024 15:30:28 +0000 Subject: [PATCH 02/11] Add limits for all signals --- src/ophyd_async/epics/_backend/_aioca.py | 78 ++++++++++------- tests/epics/test_signals.py | 106 +++++++++++++---------- 2 files changed, 108 insertions(+), 76 deletions(-) diff --git a/src/ophyd_async/epics/_backend/_aioca.py b/src/ophyd_async/epics/_backend/_aioca.py index 1cda7c2d2e..60eebc54c3 100644 --- a/src/ophyd_async/epics/_backend/_aioca.py +++ b/src/ophyd_async/epics/_backend/_aioca.py @@ -3,7 +3,7 @@ from dataclasses import dataclass from enum import Enum from math import isnan -from typing import Any, Dict, Optional, Sequence, Type, Union +from typing import Any, Dict, List, Optional, Sequence, Type, Union from aioca import ( FORMAT_CTRL, @@ -40,58 +40,68 @@ dbr.DBR_DOUBLE: "number", dbr.DBR_ENUM: "string", } -_num_dbr = {dbr.DBR_SHORT, dbr.DBR_CHAR, dbr.DBR_LONG, dbr.DBR_FLOAT, dbr.DBR_DOUBLE} +_number_dbr = {dbr.DBR_SHORT, dbr.DBR_CHAR, dbr.DBR_LONG, dbr.DBR_FLOAT, dbr.DBR_DOUBLE} _floating_dbr = {dbr.DBR_FLOAT, dbr.DBR_DOUBLE} +_number_meta = { + "units", + "lower_alarm_limit", + "upper_alarm_limit", + "lower_ctrl_limit", + "upper_ctrl_limit", + "lower_disp_limit", + "upper_disp_limit", + "lower_warning_limit", + "upper_warning_limit", +} + +def _if_has_meta_add_meta( + d: Dict[str, Any], value: AugmentedValue, key: Union[str, List[str]] +): + if isinstance(key, str): + key = [key] + for k in key: + if not hasattr(value, k): + continue + attr = getattr(value, k) + if isinstance(attr, str) or not isnan(attr): + d[k] = attr -def _data_key_from_augmented_value(source: str, value: AugmentedValue) -> Descriptor: + +def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> Descriptor: """Use the return value of get with FORMAT_CTRL to construct a Descriptor describing the signal. See docstring of AugmentedValue for expected value fields by DBR type. Args: value (AugmentedValue): Description of the the return type of a DB record + kwargs: Overrides for the returned values. + e.g. to treat a value as an Enumeration by passing choices Returns: Descriptor: A rich DataKey describing the DB record """ - - if not value.ok: - raise Exception(f"{source} error: {value.errorcode}: {str(value)}") + source = f"ca://{value.name}" + assert value.ok, f"Error reading {source}: {value}" scalar = value.element_count == 1 d = Descriptor( source=source, dtype=dbr_to_dtype[value.datatype] if scalar else "array", + # strictly value.element_count >= len(value) shape=[] if scalar else [len(value)], ) - if value.datatype in _num_dbr: - d["units"] = value.units - if not isnan(value.lower_alarm_limit): - d["lower_alarm_limit"] = value.lower_alarm_limit - if not isnan(value.upper_alarm_limit): - d["upper_alarm_limit"] = value.upper_alarm_limit - if not isnan(value.lower_ctrl_limit): - d["lower_ctrl_limit"] = value.lower_ctrl_limit - if not isnan(value.upper_ctrl_limit): - d["upper_ctrl_limit"] = value.upper_ctrl_limit - if not isnan(value.lower_disp_limit): - d["lower_disp_limit"] = value.lower_disp_limit - if not isnan(value.upper_disp_limit): - d["upper_disp_limit"] = value.upper_disp_limit - if not isnan(value.lower_warning_limit): - d["lower_warning_limit"] = value.lower_warning_limit - if not isnan(value.upper_warning_limit): - d["upper_warning_limit"] = value.upper_warning_limit - - if value.datatype in _floating_dbr and not isnan(value.precision): - d["precision"] = value.precision + if value.datatype in _number_dbr: + _if_has_meta_add_meta(d, value, _number_meta) + if value.datatype in _floating_dbr: + _if_has_meta_add_meta(d, value, "precision") if value.datatype is dbr.DBR_ENUM: d["choices"] = value.enums - if value.datatype is dbr.DBR_STRING and not isnan(value.timestamp): - d["timestamp"] = value.timestamp + if value.datatype is dbr.DBR_STRING: + _if_has_meta_add_meta(d, value, "timestamp") + d.update(kwargs) return d @@ -114,8 +124,8 @@ def reading(self, value: AugmentedValue): alarm_severity=-1 if value.severity > 2 else value.severity, ) - def descriptor(self, source: str, value: AugmentedValue) -> Descriptor: - return _data_key_from_augmented_value(source, value) + def descriptor(self, value: AugmentedValue, **kwargs) -> Descriptor: + return _data_key_from_augmented_value(value, **kwargs) class CaLongStrConverter(CaConverter): @@ -140,6 +150,10 @@ def write_value(self, value: Union[Enum, str]): def value(self, value: AugmentedValue): return self.enum_class(value) + + def descriptor(self, value: AugmentedValue) -> Descriptor: + choices = [e.value for e in self.enum_class] + return super().descriptor(value, choices=choices) class DisconnectedCaConverter(CaConverter): def __getattribute__(self, __name: str) -> Any: @@ -264,7 +278,7 @@ async def _caget(self, format: Format) -> AugmentedValue: async def get_descriptor(self) -> Descriptor: value = await self._caget(FORMAT_CTRL) - return self.converter.descriptor(self.source, value) + return self.converter.descriptor(value) async def get_reading(self) -> Reading: value = await self._caget(FORMAT_TIME) diff --git a/tests/epics/test_signals.py b/tests/epics/test_signals.py index 7c7bafcd8b..d42b94b329 100644 --- a/tests/epics/test_signals.py +++ b/tests/epics/test_signals.py @@ -9,14 +9,23 @@ from dataclasses import dataclass from enum import Enum from pathlib import Path -from typing import Any, Dict, List, Literal, Optional, Sequence, Tuple, Type, TypedDict +from typing import ( + Any, + Dict, + Literal, + Optional, + Sequence, + Tuple, + Type, + TypedDict, +) from unittest.mock import ANY import numpy as np import numpy.typing as npt import pytest from aioca import CANothing, purge_channel_caches -from bluesky.protocols import Reading +from bluesky.protocols import Reading, Descriptor from ophyd_async.core import SignalBackend, T, get_dtype, load_from_yaml, save_to_yaml from ophyd_async.core.utils import NotConnected @@ -116,7 +125,7 @@ async def assert_monitor_then_put( try: # Check descriptor source = f"{ioc.protocol}://{PV_PREFIX}:{ioc.protocol}:{suffix}" - assert dict(source=source, **descriptor) == await backend.get_descriptor() + assert dict(source=source, **descriptor).items() <= (await backend.get_descriptor()).items() # Check initial value await q.assert_updates(pytest.approx(initial_value)) # Put to new value and check that @@ -146,47 +155,55 @@ class MyEnum(str, Enum): c = "Ccc" -ca_metadata = { - "integer": { - "lower_alarm_limit": 0, - "lower_ctrl_limit": 10, - "lower_disp_limit": 0, - "lower_warning_limit": 0, - "units": "", - "upper_alarm_limit": 0, - "upper_ctrl_limit": 90, - "upper_disp_limit": 100, - "upper_warning_limit": 0, - }, - "number": { - "lower_ctrl_limit": 0.0, - "lower_disp_limit": 0.0, - "precision": 1, - "units": "mm", - "upper_ctrl_limit": 0.0, - "upper_disp_limit": 0.0, - }, - "string": {"timestamp": ANY}, +_common_metadata = { + "units": ANY, + "upper_disp_limit": ANY, + "lower_disp_limit": ANY, + "upper_ctrl_limit": ANY, + "lower_ctrl_limit": ANY, } +_metadata = { + "string": {"timestamp": ANY}, + "integer": _common_metadata, + "number": {**_common_metadata, "precision": ANY}, + "enum": {} +} -def suffix_to_dtype(suffix: str): - if "float" in suffix: - return "number" - if "int" in suffix: - return "integer" - return "string" +def descriptor(protocol: str, suffix: str, value: Optional[Any] = None) -> Descriptor: + def get_internal_dtype(suffix: str) -> str: + if "int" in suffix or "bool" in suffix: + return "integer" + if "float" in suffix: + return "number" + if "enum" in suffix: + return "enum" + return "string" + + def get_dtype(suffix: str) -> str: + if suffix.endswith("a"): + return "array" + if "enum" in suffix: + return "string" + return get_internal_dtype(suffix) + + def ca_metadata(dtype: str, internal_dtype: str) -> Dict[str, Any]: + if internal_dtype == "string": + return _metadata[internal_dtype] + if dtype == "array": + return _metadata[dtype] + return + + dtype = get_dtype(suffix) + + d = dict(dtype=dtype, shape=[len(value)] if dtype == "array" else []) + if get_internal_dtype(suffix) == "enum": + d["choices"] = [e.value for e in type(value)] -def descriptor(protocol: str, suffix: str, value: Any): - is_array = isinstance(value, List) - dtype = suffix_to_dtype(suffix) - d = { - "dtype": "array" if is_array else dtype, - "shape": [len(value)] if isinstance(value, List) else [], - } if protocol == "ca": - d.update(ca_metadata.get(dtype, {})) + d.update(_metadata[get_internal_dtype(suffix)]) + return d @@ -203,7 +220,7 @@ def descriptor(protocol: str, suffix: str, value: Any): @pytest.mark.parametrize( - "datatype, suffix, initial_value, put_value, descriptor", + "datatype, suffix, initial_value, put_value", [ (int, "int", 42, 43), (float, "float", 3.141, 43.5), @@ -221,8 +238,8 @@ def descriptor(protocol: str, suffix: str, value: Any): (npt.NDArray[np.float64], "float64a", [0.1, -12345678.123], [0.2]), (Sequence[str], "stra", ["five", "six", "seven"], ["nine", "ten"]), # Can't do long strings until https://github.com/epics-base/pva2pva/issues/17 - # (str, "longstr", ls1, ls2, string_d), - # (str, "longstr2.VAL$", ls1, ls2, string_d), + # (str, "longstr", ls1, ls2), + # (str, "longstr2.VAL$", ls1, ls2), ], ) async def test_backend_get_put_monitor( @@ -274,7 +291,7 @@ async def test_bool_conversion_of_enum(ioc: IOC, suffix: str) -> None: await assert_monitor_then_put( ioc, suffix=suffix, - descriptor=descriptor(ioc.protocol, "bool", True), + descriptor=descriptor(ioc.protocol, "bool"), initial_value=True, put_value=False, datatype=bool, @@ -420,7 +437,8 @@ async def test_pva_table(ioc: IOC) -> None: q = MonitorQueue(backend) try: # Check descriptor - dict(source=backend.source, **descriptor) == await backend.get_descriptor() + dict(source=backend.source, **descriptor).items() <= (await backend.get_descriptor()).items() + # Check initial value await q.assert_updates(approx_table(i)) # Put to new value and check that @@ -485,7 +503,7 @@ async def test_pva_ntdarray(ioc: IOC): "source": backend.source, "dtype": "array", "shape": [2, 3], - } == await backend.get_descriptor() + }.items() <= (await backend.get_descriptor()).items() # Check initial value await q.assert_updates(pytest.approx(i)) await raw_data_backend.put(p.flatten()) From 35eb958e0765c6efea5b16862b4d81ea8ab6ace0 Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Thu, 14 Mar 2024 15:30:28 +0000 Subject: [PATCH 03/11] Revert adding limits to test records, refactor tests --- src/ophyd_async/epics/_backend/_aioca.py | 16 +++++------ tests/epics/test_signals.py | 35 ++++++++---------------- 2 files changed, 20 insertions(+), 31 deletions(-) diff --git a/src/ophyd_async/epics/_backend/_aioca.py b/src/ophyd_async/epics/_backend/_aioca.py index 60eebc54c3..e86b728a5f 100644 --- a/src/ophyd_async/epics/_backend/_aioca.py +++ b/src/ophyd_async/epics/_backend/_aioca.py @@ -3,7 +3,7 @@ from dataclasses import dataclass from enum import Enum from math import isnan -from typing import Any, Dict, List, Optional, Sequence, Type, Union +from typing import Any, Dict, Optional, Sequence, Set, Type, Union from aioca import ( FORMAT_CTRL, @@ -56,10 +56,10 @@ def _if_has_meta_add_meta( - d: Dict[str, Any], value: AugmentedValue, key: Union[str, List[str]] + d: Dict[str, Any], value: AugmentedValue, key: Union[str, Set[str]] ): if isinstance(key, str): - key = [key] + key = {key} for k in key: if not hasattr(value, k): continue @@ -75,7 +75,7 @@ def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> Descripto Args: value (AugmentedValue): Description of the the return type of a DB record - kwargs: Overrides for the returned values. + kwargs: Overrides for the returned values. e.g. to treat a value as an Enumeration by passing choices Returns: @@ -124,8 +124,8 @@ def reading(self, value: AugmentedValue): alarm_severity=-1 if value.severity > 2 else value.severity, ) - def descriptor(self, value: AugmentedValue, **kwargs) -> Descriptor: - return _data_key_from_augmented_value(value, **kwargs) + def descriptor(self, value: AugmentedValue) -> Descriptor: + return _data_key_from_augmented_value(value) class CaLongStrConverter(CaConverter): @@ -150,10 +150,10 @@ def write_value(self, value: Union[Enum, str]): def value(self, value: AugmentedValue): return self.enum_class(value) - + def descriptor(self, value: AugmentedValue) -> Descriptor: choices = [e.value for e in self.enum_class] - return super().descriptor(value, choices=choices) + return _data_key_from_augmented_value(value, choices=choices) class DisconnectedCaConverter(CaConverter): def __getattribute__(self, __name: str) -> Any: diff --git a/tests/epics/test_signals.py b/tests/epics/test_signals.py index d42b94b329..b351de92fe 100644 --- a/tests/epics/test_signals.py +++ b/tests/epics/test_signals.py @@ -9,23 +9,14 @@ from dataclasses import dataclass from enum import Enum from pathlib import Path -from typing import ( - Any, - Dict, - Literal, - Optional, - Sequence, - Tuple, - Type, - TypedDict, -) +from typing import Any, Dict, Literal, Optional, Sequence, Tuple, Type, TypedDict from unittest.mock import ANY import numpy as np import numpy.typing as npt import pytest from aioca import CANothing, purge_channel_caches -from bluesky.protocols import Reading, Descriptor +from bluesky.protocols import Descriptor, Reading from ophyd_async.core import SignalBackend, T, get_dtype, load_from_yaml, save_to_yaml from ophyd_async.core.utils import NotConnected @@ -125,7 +116,10 @@ async def assert_monitor_then_put( try: # Check descriptor source = f"{ioc.protocol}://{PV_PREFIX}:{ioc.protocol}:{suffix}" - assert dict(source=source, **descriptor).items() <= (await backend.get_descriptor()).items() + assert ( + dict(source=source, **descriptor).items() + <= (await backend.get_descriptor()).items() + ) # Check initial value await q.assert_updates(pytest.approx(initial_value)) # Put to new value and check that @@ -163,15 +157,15 @@ class MyEnum(str, Enum): "lower_ctrl_limit": ANY, } -_metadata = { +_metadata: Dict[str, Dict[str, Any]] = { "string": {"timestamp": ANY}, "integer": _common_metadata, "number": {**_common_metadata, "precision": ANY}, - "enum": {} + "enum": {}, } -def descriptor(protocol: str, suffix: str, value: Optional[Any] = None) -> Descriptor: +def descriptor(protocol: str, suffix: str, value=None) -> Descriptor: def get_internal_dtype(suffix: str) -> str: if "int" in suffix or "bool" in suffix: return "integer" @@ -188,13 +182,6 @@ def get_dtype(suffix: str) -> str: return "string" return get_internal_dtype(suffix) - def ca_metadata(dtype: str, internal_dtype: str) -> Dict[str, Any]: - if internal_dtype == "string": - return _metadata[internal_dtype] - if dtype == "array": - return _metadata[dtype] - return - dtype = get_dtype(suffix) d = dict(dtype=dtype, shape=[len(value)] if dtype == "array" else []) @@ -437,7 +424,9 @@ async def test_pva_table(ioc: IOC) -> None: q = MonitorQueue(backend) try: # Check descriptor - dict(source=backend.source, **descriptor).items() <= (await backend.get_descriptor()).items() + dict(source=backend.source, **descriptor).items() <= ( + await backend.get_descriptor() + ).items() # Check initial value await q.assert_updates(approx_table(i)) From da7dfb8cb1b666366e3e5b8a4ff155b79dab163c Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Thu, 14 Mar 2024 15:30:28 +0000 Subject: [PATCH 04/11] Refactor tests for DBR_DOUBLE backing --- tests/epics/test_signals.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/tests/epics/test_signals.py b/tests/epics/test_signals.py index b351de92fe..f6eacb54af 100644 --- a/tests/epics/test_signals.py +++ b/tests/epics/test_signals.py @@ -116,10 +116,7 @@ async def assert_monitor_then_put( try: # Check descriptor source = f"{ioc.protocol}://{PV_PREFIX}:{ioc.protocol}:{suffix}" - assert ( - dict(source=source, **descriptor).items() - <= (await backend.get_descriptor()).items() - ) + assert dict(source=source, **descriptor) == await backend.get_descriptor() # Check initial value await q.assert_updates(pytest.approx(initial_value)) # Put to new value and check that @@ -157,20 +154,28 @@ class MyEnum(str, Enum): "lower_ctrl_limit": ANY, } +_int_metadata = { + "lower_alarm_limit": 0, + "lower_warning_limit": 0, + "upper_alarm_limit": 0, + "upper_warning_limit": 0, +} + _metadata: Dict[str, Dict[str, Any]] = { + "enum": {"timestamp": ANY}, "string": {"timestamp": ANY}, - "integer": _common_metadata, + "integer": {**_int_metadata, **_common_metadata}, "number": {**_common_metadata, "precision": ANY}, - "enum": {}, } def descriptor(protocol: str, suffix: str, value=None) -> Descriptor: def get_internal_dtype(suffix: str) -> str: + # uint32, int64, uint64 all backed by DBR_DOUBLE which has precision + if "float" in suffix or "uint32" in suffix or "int64" in suffix: + return "number" if "int" in suffix or "bool" in suffix: return "integer" - if "float" in suffix: - return "number" if "enum" in suffix: return "enum" return "string" @@ -424,9 +429,7 @@ async def test_pva_table(ioc: IOC) -> None: q = MonitorQueue(backend) try: # Check descriptor - dict(source=backend.source, **descriptor).items() <= ( - await backend.get_descriptor() - ).items() + dict(source=backend.source, **descriptor) == await backend.get_descriptor() # Check initial value await q.assert_updates(approx_table(i)) @@ -492,7 +495,7 @@ async def test_pva_ntdarray(ioc: IOC): "source": backend.source, "dtype": "array", "shape": [2, 3], - }.items() <= (await backend.get_descriptor()).items() + } == await backend.get_descriptor() # Check initial value await q.assert_updates(pytest.approx(i)) await raw_data_backend.put(p.flatten()) From eaf46b5975609849fc2c2c7d1e037af8e7d19c37 Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Thu, 14 Mar 2024 15:30:28 +0000 Subject: [PATCH 05/11] Simplify metadata handling --- src/ophyd_async/epics/_backend/_aioca.py | 30 +++++++++--------------- tests/epics/test_signals.py | 10 ++++---- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/ophyd_async/epics/_backend/_aioca.py b/src/ophyd_async/epics/_backend/_aioca.py index e86b728a5f..6598d7379a 100644 --- a/src/ophyd_async/epics/_backend/_aioca.py +++ b/src/ophyd_async/epics/_backend/_aioca.py @@ -40,9 +40,7 @@ dbr.DBR_DOUBLE: "number", dbr.DBR_ENUM: "string", } -_number_dbr = {dbr.DBR_SHORT, dbr.DBR_CHAR, dbr.DBR_LONG, dbr.DBR_FLOAT, dbr.DBR_DOUBLE} -_floating_dbr = {dbr.DBR_FLOAT, dbr.DBR_DOUBLE} -_number_meta = { +_common_meta = { "units", "lower_alarm_limit", "upper_alarm_limit", @@ -52,20 +50,18 @@ "upper_disp_limit", "lower_warning_limit", "upper_warning_limit", + "precision", + "timestamp", } -def _if_has_meta_add_meta( - d: Dict[str, Any], value: AugmentedValue, key: Union[str, Set[str]] -): - if isinstance(key, str): - key = {key} - for k in key: - if not hasattr(value, k): +def _if_has_meta_add_meta(d: Dict[str, Any], value: AugmentedValue, keys: Set[str]): + for key in keys: + if not hasattr(value, key): continue - attr = getattr(value, k) + attr = getattr(value, key) if isinstance(attr, str) or not isnan(attr): - d[k] = attr + d[key] = attr def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> Descriptor: @@ -85,22 +81,18 @@ def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> Descripto assert value.ok, f"Error reading {source}: {value}" scalar = value.element_count == 1 + dtype = dbr_to_dtype[value.datatype] d = Descriptor( source=source, - dtype=dbr_to_dtype[value.datatype] if scalar else "array", + dtype=dtype if scalar else "array", # strictly value.element_count >= len(value) shape=[] if scalar else [len(value)], ) - if value.datatype in _number_dbr: - _if_has_meta_add_meta(d, value, _number_meta) - if value.datatype in _floating_dbr: - _if_has_meta_add_meta(d, value, "precision") + _if_has_meta_add_meta(d, value, _common_meta) if value.datatype is dbr.DBR_ENUM: d["choices"] = value.enums - if value.datatype is dbr.DBR_STRING: - _if_has_meta_add_meta(d, value, "timestamp") d.update(kwargs) return d diff --git a/tests/epics/test_signals.py b/tests/epics/test_signals.py index f6eacb54af..d83a365834 100644 --- a/tests/epics/test_signals.py +++ b/tests/epics/test_signals.py @@ -155,10 +155,10 @@ class MyEnum(str, Enum): } _int_metadata = { - "lower_alarm_limit": 0, - "lower_warning_limit": 0, - "upper_alarm_limit": 0, - "upper_warning_limit": 0, + "lower_alarm_limit": ANY, + "lower_warning_limit": ANY, + "upper_alarm_limit": ANY, + "upper_warning_limit": ANY, } _metadata: Dict[str, Dict[str, Any]] = { @@ -171,7 +171,7 @@ class MyEnum(str, Enum): def descriptor(protocol: str, suffix: str, value=None) -> Descriptor: def get_internal_dtype(suffix: str) -> str: - # uint32, int64, uint64 all backed by DBR_DOUBLE which has precision + # uint32, [u]int64 backed by DBR_DOUBLE, have precision if "float" in suffix or "uint32" in suffix or "int64" in suffix: return "number" if "int" in suffix or "bool" in suffix: From 235dc1af229730e449ce7e4f02ae880c2414ceb3 Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Thu, 14 Mar 2024 15:30:28 +0000 Subject: [PATCH 06/11] enum_choices instantiate once --- src/ophyd_async/epics/_backend/_aioca.py | 27 ++++++++++++------------ 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/ophyd_async/epics/_backend/_aioca.py b/src/ophyd_async/epics/_backend/_aioca.py index 6598d7379a..736e62a624 100644 --- a/src/ophyd_async/epics/_backend/_aioca.py +++ b/src/ophyd_async/epics/_backend/_aioca.py @@ -1,9 +1,9 @@ import logging import sys -from dataclasses import dataclass +from dataclasses import dataclass, field from enum import Enum from math import isnan -from typing import Any, Dict, Optional, Sequence, Set, Type, Union +from typing import Any, Dict, List, Optional, Sequence, Type, Union from aioca import ( FORMAT_CTRL, @@ -55,15 +55,6 @@ } -def _if_has_meta_add_meta(d: Dict[str, Any], value: AugmentedValue, keys: Set[str]): - for key in keys: - if not hasattr(value, key): - continue - attr = getattr(value, key) - if isinstance(attr, str) or not isnan(attr): - d[key] = attr - - def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> Descriptor: """Use the return value of get with FORMAT_CTRL to construct a Descriptor describing the signal. See docstring of AugmentedValue for expected @@ -89,8 +80,13 @@ def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> Descripto # strictly value.element_count >= len(value) shape=[] if scalar else [len(value)], ) + for key in _common_meta: + if not hasattr(value, key): + continue + attr = getattr(value, key) + if isinstance(attr, str) or not isnan(attr): + d[key] = attr - _if_has_meta_add_meta(d, value, _common_meta) if value.datatype is dbr.DBR_ENUM: d["choices"] = value.enums d.update(kwargs) @@ -133,6 +129,10 @@ def write_value(self, value: str): @dataclass class CaEnumConverter(CaConverter): enum_class: Type[Enum] + choices: List[str] = field(init=False) + + def __post_init__(self): + self.choices = [e.value for e in self.enum_class] def write_value(self, value: Union[Enum, str]): if isinstance(value, Enum): @@ -144,8 +144,7 @@ def value(self, value: AugmentedValue): return self.enum_class(value) def descriptor(self, value: AugmentedValue) -> Descriptor: - choices = [e.value for e in self.enum_class] - return _data_key_from_augmented_value(value, choices=choices) + return _data_key_from_augmented_value(value, choices=self.choices) class DisconnectedCaConverter(CaConverter): def __getattribute__(self, __name: str) -> Any: From cc713d788daadc54ff864ea90b4cca45b3042923 Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Thu, 14 Mar 2024 15:30:28 +0000 Subject: [PATCH 07/11] Descriptor -> DataKey --- src/ophyd_async/epics/_backend/_aioca.py | 28 ++++++++++++------------ tests/epics/test_signals.py | 5 ++--- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/ophyd_async/epics/_backend/_aioca.py b/src/ophyd_async/epics/_backend/_aioca.py index 736e62a624..d4537c2630 100644 --- a/src/ophyd_async/epics/_backend/_aioca.py +++ b/src/ophyd_async/epics/_backend/_aioca.py @@ -16,7 +16,7 @@ caput, ) from aioca.types import AugmentedValue, Dbr, Format -from bluesky.protocols import Descriptor, Dtype, Reading +from bluesky.protocols import DataKey, Dtype, Reading from epicscorelibs.ca import dbr from ophyd_async.core import ( @@ -55,18 +55,18 @@ } -def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> Descriptor: - """Use the return value of get with FORMAT_CTRL to construct a Descriptor +def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> DataKey: + """Use the return value of get with FORMAT_CTRL to construct a DataKey describing the signal. See docstring of AugmentedValue for expected value fields by DBR type. Args: value (AugmentedValue): Description of the the return type of a DB record kwargs: Overrides for the returned values. - e.g. to treat a value as an Enumeration by passing choices + e.g. to force treating a value as an Enum by passing choices Returns: - Descriptor: A rich DataKey describing the DB record + DataKey: A rich DataKey describing the DB record """ source = f"ca://{value.name}" assert value.ok, f"Error reading {source}: {value}" @@ -74,18 +74,17 @@ def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> Descripto scalar = value.element_count == 1 dtype = dbr_to_dtype[value.datatype] - d = Descriptor( + d = DataKey( source=source, dtype=dtype if scalar else "array", # strictly value.element_count >= len(value) shape=[] if scalar else [len(value)], ) for key in _common_meta: - if not hasattr(value, key): - continue - attr = getattr(value, key) - if isinstance(attr, str) or not isnan(attr): - d[key] = attr + if hasattr(value, key): + attr = getattr(value, key) + if isinstance(attr, str) or not isnan(attr): + d[key] = attr if value.datatype is dbr.DBR_ENUM: d["choices"] = value.enums @@ -112,7 +111,7 @@ def reading(self, value: AugmentedValue): alarm_severity=-1 if value.severity > 2 else value.severity, ) - def descriptor(self, value: AugmentedValue) -> Descriptor: + def descriptor(self, value: AugmentedValue) -> DataKey: return _data_key_from_augmented_value(value) @@ -143,7 +142,8 @@ def write_value(self, value: Union[Enum, str]): def value(self, value: AugmentedValue): return self.enum_class(value) - def descriptor(self, value: AugmentedValue) -> Descriptor: + def descriptor(self, value: AugmentedValue) -> DataKey: + # Sometimes DBR_TYPE returns as String, must pass choices still return _data_key_from_augmented_value(value, choices=self.choices) class DisconnectedCaConverter(CaConverter): @@ -267,7 +267,7 @@ async def _caget(self, format: Format) -> AugmentedValue: timeout=None, ) - async def get_descriptor(self) -> Descriptor: + async def get_descriptor(self) -> DataKey: value = await self._caget(FORMAT_CTRL) return self.converter.descriptor(value) diff --git a/tests/epics/test_signals.py b/tests/epics/test_signals.py index d83a365834..7f9bd70032 100644 --- a/tests/epics/test_signals.py +++ b/tests/epics/test_signals.py @@ -16,7 +16,7 @@ import numpy.typing as npt import pytest from aioca import CANothing, purge_channel_caches -from bluesky.protocols import Descriptor, Reading +from bluesky.protocols import DataKey, Reading from ophyd_async.core import SignalBackend, T, get_dtype, load_from_yaml, save_to_yaml from ophyd_async.core.utils import NotConnected @@ -169,7 +169,7 @@ class MyEnum(str, Enum): } -def descriptor(protocol: str, suffix: str, value=None) -> Descriptor: +def descriptor(protocol: str, suffix: str, value=None) -> DataKey: def get_internal_dtype(suffix: str) -> str: # uint32, [u]int64 backed by DBR_DOUBLE, have precision if "float" in suffix or "uint32" in suffix or "int64" in suffix: @@ -430,7 +430,6 @@ async def test_pva_table(ioc: IOC) -> None: try: # Check descriptor dict(source=backend.source, **descriptor) == await backend.get_descriptor() - # Check initial value await q.assert_updates(approx_table(i)) # Put to new value and check that From 4020a0d475cbe51ad77164307aa006101ccb75e7 Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Thu, 14 Mar 2024 15:30:28 +0000 Subject: [PATCH 08/11] Reduce metadata to just precision and units --- src/ophyd_async/epics/_backend/_aioca.py | 9 --------- tests/epics/test_signals.py | 23 ++++------------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/src/ophyd_async/epics/_backend/_aioca.py b/src/ophyd_async/epics/_backend/_aioca.py index d4537c2630..9dd9ad1075 100644 --- a/src/ophyd_async/epics/_backend/_aioca.py +++ b/src/ophyd_async/epics/_backend/_aioca.py @@ -42,16 +42,7 @@ } _common_meta = { "units", - "lower_alarm_limit", - "upper_alarm_limit", - "lower_ctrl_limit", - "upper_ctrl_limit", - "lower_disp_limit", - "upper_disp_limit", - "lower_warning_limit", - "upper_warning_limit", "precision", - "timestamp", } diff --git a/tests/epics/test_signals.py b/tests/epics/test_signals.py index 7f9bd70032..bf7f94ba33 100644 --- a/tests/epics/test_signals.py +++ b/tests/epics/test_signals.py @@ -146,26 +146,11 @@ class MyEnum(str, Enum): c = "Ccc" -_common_metadata = { - "units": ANY, - "upper_disp_limit": ANY, - "lower_disp_limit": ANY, - "upper_ctrl_limit": ANY, - "lower_ctrl_limit": ANY, -} - -_int_metadata = { - "lower_alarm_limit": ANY, - "lower_warning_limit": ANY, - "upper_alarm_limit": ANY, - "upper_warning_limit": ANY, -} - _metadata: Dict[str, Dict[str, Any]] = { - "enum": {"timestamp": ANY}, - "string": {"timestamp": ANY}, - "integer": {**_int_metadata, **_common_metadata}, - "number": {**_common_metadata, "precision": ANY}, + "enum": {}, + "string": {}, + "integer": {"units": ANY}, + "number": {"units": ANY, "precision": ANY}, } From 0a68b82a050e794b25433fa12825962d3097cdbd Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Thu, 14 Mar 2024 15:30:28 +0000 Subject: [PATCH 09/11] Restore lost commits --- src/ophyd_async/epics/_backend/_aioca.py | 25 ++++++++++++++---------- src/ophyd_async/epics/_backend/_p4p.py | 14 ++++++++++--- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/ophyd_async/epics/_backend/_aioca.py b/src/ophyd_async/epics/_backend/_aioca.py index 9dd9ad1075..13e58d029a 100644 --- a/src/ophyd_async/epics/_backend/_aioca.py +++ b/src/ophyd_async/epics/_backend/_aioca.py @@ -2,7 +2,7 @@ import sys from dataclasses import dataclass, field from enum import Enum -from math import isnan +from math import isnan, nan from typing import Any, Dict, List, Optional, Sequence, Type, Union from aioca import ( @@ -46,7 +46,9 @@ } -def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> DataKey: +def _data_key_from_augmented_value( + value: AugmentedValue, *, choices: Optional[List[str]] = None +) -> DataKey: """Use the return value of get with FORMAT_CTRL to construct a DataKey describing the signal. See docstring of AugmentedValue for expected value fields by DBR type. @@ -72,15 +74,12 @@ def _data_key_from_augmented_value(value: AugmentedValue, **kwargs) -> DataKey: shape=[] if scalar else [len(value)], ) for key in _common_meta: - if hasattr(value, key): - attr = getattr(value, key) - if isinstance(attr, str) or not isnan(attr): - d[key] = attr - - if value.datatype is dbr.DBR_ENUM: - d["choices"] = value.enums - d.update(kwargs) + attr = getattr(value, key, nan) + if isinstance(attr, str) or not isnan(attr): + d[key] = attr + if choices is not None: + d["choices"] = choices return d @@ -118,6 +117,11 @@ def write_value(self, value: str): @dataclass class CaEnumConverter(CaConverter): + """To prevent issues when a signal is restarted and returns with different enum + values or orders, we put treat an Enum signal as a DBR_STRING, and cache the + choices on this class. + """ + enum_class: Type[Enum] choices: List[str] = field(init=False) @@ -137,6 +141,7 @@ def descriptor(self, value: AugmentedValue) -> DataKey: # Sometimes DBR_TYPE returns as String, must pass choices still return _data_key_from_augmented_value(value, choices=self.choices) + class DisconnectedCaConverter(CaConverter): def __getattribute__(self, __name: str) -> Any: raise NotImplementedError("No PV has been set as connect() has not been called") diff --git a/src/ophyd_async/epics/_backend/_p4p.py b/src/ophyd_async/epics/_backend/_p4p.py index 0507ff4d32..2f2ceeb84f 100644 --- a/src/ophyd_async/epics/_backend/_p4p.py +++ b/src/ophyd_async/epics/_backend/_p4p.py @@ -2,7 +2,7 @@ import atexit import logging import time -from dataclasses import dataclass +from dataclasses import dataclass, field from enum import Enum from typing import Any, Dict, List, Optional, Sequence, Type, Union @@ -109,7 +109,16 @@ def write_value(self, value): @dataclass class PvaEnumConverter(PvaConverter): + """To prevent issues when a signal is restarted and returns with different enum + values or orders, we put treat an Enum signal as a DBR_STRING, and cache the + choices on this class. + """ + enum_class: Type[Enum] + choices: List[str] = field(init=False) + + def __post_init__(self): + self.choices = [e.value for e in self.enum_class] def write_value(self, value: Union[Enum, str]): if isinstance(value, Enum): @@ -121,8 +130,7 @@ def value(self, value): return list(self.enum_class)[value["value"]["index"]] def descriptor(self, source: str, value) -> Descriptor: - choices = [e.value for e in self.enum_class] - return dict(source=source, dtype="string", shape=[], choices=choices) + return dict(source=source, dtype="string", shape=[], choices=self.choices) class PvaEnumBoolConverter(PvaConverter): From 86198e7a893c0dc4dfad951c17702310e7c8b6c3 Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Thu, 14 Mar 2024 15:30:28 +0000 Subject: [PATCH 10/11] Add explicit bool type --- src/ophyd_async/epics/_backend/_aioca.py | 19 ++++++++++++++++--- src/ophyd_async/epics/_backend/_p4p.py | 8 ++++---- tests/epics/test_signals.py | 9 +++++---- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/ophyd_async/epics/_backend/_aioca.py b/src/ophyd_async/epics/_backend/_aioca.py index 13e58d029a..62bf3e71eb 100644 --- a/src/ophyd_async/epics/_backend/_aioca.py +++ b/src/ophyd_async/epics/_backend/_aioca.py @@ -47,7 +47,10 @@ def _data_key_from_augmented_value( - value: AugmentedValue, *, choices: Optional[List[str]] = None + value: AugmentedValue, + *, + choices: Optional[List[str]] = None, + dtype: Optional[str] = None, ) -> DataKey: """Use the return value of get with FORMAT_CTRL to construct a DataKey describing the signal. See docstring of AugmentedValue for expected @@ -65,7 +68,7 @@ def _data_key_from_augmented_value( assert value.ok, f"Error reading {source}: {value}" scalar = value.element_count == 1 - dtype = dbr_to_dtype[value.datatype] + dtype = dtype or dbr_to_dtype[value.datatype] d = DataKey( source=source, @@ -142,6 +145,16 @@ def descriptor(self, value: AugmentedValue) -> DataKey: return _data_key_from_augmented_value(value, choices=self.choices) +@dataclass +class CaBoolConverter(CaConverter): + + def value(self, value: AugmentedValue) -> bool: + return bool(value) + + def descriptor(self, value: AugmentedValue) -> DataKey: + return _data_key_from_augmented_value(value, dtype="bool") + + class DisconnectedCaConverter(CaConverter): def __getattribute__(self, __name: str) -> Any: raise NotImplementedError("No PV has been set as connect() has not been called") @@ -179,7 +192,7 @@ def make_converter( ) if pv_choices_len != 2: raise TypeError(f"{pv} has {pv_choices_len} choices, can't map to bool") - return CaConverter(dbr.DBR_SHORT, dbr.DBR_SHORT) + return CaBoolConverter(dbr.DBR_SHORT, dbr.DBR_SHORT) elif pv_dbr == dbr.DBR_ENUM: # This is an Enum pv_choices = get_unique( diff --git a/src/ophyd_async/epics/_backend/_p4p.py b/src/ophyd_async/epics/_backend/_p4p.py index 2f2ceeb84f..17d040eb9f 100644 --- a/src/ophyd_async/epics/_backend/_p4p.py +++ b/src/ophyd_async/epics/_backend/_p4p.py @@ -133,12 +133,12 @@ def descriptor(self, source: str, value) -> Descriptor: return dict(source=source, dtype="string", shape=[], choices=self.choices) -class PvaEnumBoolConverter(PvaConverter): +class PvaBoolConverter(PvaConverter): def value(self, value): - return value["value"]["index"] + return bool(value["value"]["index"]) def descriptor(self, source: str, value) -> Descriptor: - return dict(source=source, dtype="integer", shape=[]) + return dict(source=source, dtype="bool", shape=[]) class PvaTableConverter(PvaConverter): @@ -216,7 +216,7 @@ def make_converter(datatype: Optional[Type], values: Dict[str, Any]) -> PvaConve ) if pv_choices_len != 2: raise TypeError(f"{pv} has {pv_choices_len} choices, can't map to bool") - return PvaEnumBoolConverter() + return PvaBoolConverter() elif "NTEnum" in typeid: # This is an Enum pv_choices = get_unique( diff --git a/tests/epics/test_signals.py b/tests/epics/test_signals.py index bf7f94ba33..7fb602d63a 100644 --- a/tests/epics/test_signals.py +++ b/tests/epics/test_signals.py @@ -147,9 +147,8 @@ class MyEnum(str, Enum): _metadata: Dict[str, Dict[str, Any]] = { - "enum": {}, - "string": {}, "integer": {"units": ANY}, + "bool": {"units": ANY}, "number": {"units": ANY, "precision": ANY}, } @@ -159,8 +158,10 @@ def get_internal_dtype(suffix: str) -> str: # uint32, [u]int64 backed by DBR_DOUBLE, have precision if "float" in suffix or "uint32" in suffix or "int64" in suffix: return "number" - if "int" in suffix or "bool" in suffix: + if "int" in suffix: return "integer" + if "bool" in suffix: + return "bool" if "enum" in suffix: return "enum" return "string" @@ -179,7 +180,7 @@ def get_dtype(suffix: str) -> str: d["choices"] = [e.value for e in type(value)] if protocol == "ca": - d.update(_metadata[get_internal_dtype(suffix)]) + d.update(_metadata.get(get_internal_dtype(suffix), {})) return d From ae622b18810215c4244de5b67a60d4a97a05cc3f Mon Sep 17 00:00:00 2001 From: "Ware, Joseph (DLSLtd,RAL,LSCI)" Date: Thu, 14 Mar 2024 15:39:40 +0000 Subject: [PATCH 11/11] Isolate boolean test behaviour --- tests/epics/test_signals.py | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/tests/epics/test_signals.py b/tests/epics/test_signals.py index 7fb602d63a..34d376a078 100644 --- a/tests/epics/test_signals.py +++ b/tests/epics/test_signals.py @@ -265,16 +265,39 @@ async def test_backend_get_put_monitor( @pytest.mark.parametrize("suffix", ["bool", "bool_unnamed"]) -async def test_bool_conversion_of_enum(ioc: IOC, suffix: str) -> None: +async def test_bool_conversion_of_enum(ioc: IOC, suffix: str, tmp_path) -> None: + """Booleans are converted to Short Enumerations with values 0,1 as database does + not support boolean natively. + The flow of test_backend_get_put_monitor Gets a value with a dtype of None: we + cannot tell the difference between an enum with 2 members and a boolean, so + cannot get a DataKey that does not mutate form. + This test otherwise performs the same. + """ + # With the given datatype, check we have the correct initial value and putting + # works + await assert_monitor_then_put( + ioc, + suffix, + descriptor(ioc.protocol, suffix), + True, + False, + bool, + ) + # With datatype guessed from CA/PVA, check we can set it back to the initial value await assert_monitor_then_put( ioc, - suffix=suffix, - descriptor=descriptor(ioc.protocol, "bool"), - initial_value=True, - put_value=False, - datatype=bool, + suffix, + descriptor(ioc.protocol, suffix, suffix), + False, + True, + bool, ) + yaml_path = tmp_path / "test.yaml" + save_to_yaml([{"test": False}], yaml_path) + loaded = load_from_yaml(yaml_path) + assert np.all(loaded[0]["test"] is False) + async def test_error_raised_on_disconnected_PV(ioc: IOC) -> None: if ioc.protocol == "pva":