From 65c9e27b68bb1cec05c6227f2c3aa1382edc75de Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Mon, 27 Jan 2025 12:07:46 -0500 Subject: [PATCH 1/4] Update to offer more helpful error message if str fallback is used with enum --- src/ophyd_async/epics/core/_aioca.py | 28 ++++++++++++++----- src/ophyd_async/epics/testing/_example_ioc.py | 1 + src/ophyd_async/epics/testing/test_records.db | 8 ++++++ tests/epics/signal/test_signals.py | 13 +++++++++ tests/epics/signal/test_yaml_save_ca.yaml | 1 + tests/epics/signal/test_yaml_save_pva.yaml | 1 + 6 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/ophyd_async/epics/core/_aioca.py b/src/ophyd_async/epics/core/_aioca.py index 7b36c532a9..e9d6a4f6fd 100644 --- a/src/ophyd_async/epics/core/_aioca.py +++ b/src/ophyd_async/epics/core/_aioca.py @@ -293,13 +293,27 @@ async def put(self, value: SignalDatatypeT | None, wait: bool): write_value = self.initial_values[self.write_pv] else: write_value = self.converter.write_value(value) - await caput( - self.write_pv, - write_value, - datatype=self.converter.write_dbr, - wait=wait, - timeout=None, - ) + try: + await caput( + self.write_pv, + write_value, + datatype=self.converter.write_dbr, + wait=wait, + timeout=None, + ) + except CANothing as exc: + # If we ran into a write error, check to see if there is a list + # of valid choices, and if the value we tried to write is in that list. + valid_choices = self.converter.metadata.get("choices") + if valid_choices is not None and len(valid_choices) > 0: + if value not in valid_choices: + msg = ( + f"{value} is not a valid choice for {self.write_pv}, " + f"valid choices: {self.converter.metadata.get('choices')}" + ) + raise ValueError(msg) from exc + else: + raise async def get_datakey(self, source: str) -> DataKey: value = await self._caget(self.read_pv, FORMAT_CTRL) diff --git a/src/ophyd_async/epics/testing/_example_ioc.py b/src/ophyd_async/epics/testing/_example_ioc.py index f4a2de7947..5ae8880881 100644 --- a/src/ophyd_async/epics/testing/_example_ioc.py +++ b/src/ophyd_async/epics/testing/_example_ioc.py @@ -44,6 +44,7 @@ class EpicsTestCaDevice(EpicsDevice): enum: A[SignalRW[EpicsTestEnum], PvSuffix("enum")] enum2: A[SignalRW[EpicsTestEnum], PvSuffix("enum2")] subset_enum: A[SignalRW[EpicsTestSubsetEnum], PvSuffix("subset_enum")] + enum_str_fallback: A[SignalRW[str], PvSuffix("enum_str_fallback")] bool_unnamed: A[SignalRW[bool], PvSuffix("bool_unnamed")] partialint: A[SignalRW[int], PvSuffix("partialint")] lessint: A[SignalRW[int], PvSuffix("lessint")] diff --git a/src/ophyd_async/epics/testing/test_records.db b/src/ophyd_async/epics/testing/test_records.db index 082ba6d421..93d97b65e2 100644 --- a/src/ophyd_async/epics/testing/test_records.db +++ b/src/ophyd_async/epics/testing/test_records.db @@ -104,6 +104,14 @@ record(mbbo, "$(device)subset_enum") { field(PINI, "YES") } +record(mbbo, "$(device)enum_str_fallback") { + field(ZRST, "Aaa") + field(ONST, "Bbb") + field(TWST, "Ccc") + field(VAL, "1") + field(PINI, "YES") +} + record(waveform, "$(device)uint8a") { field(NELM, "3") field(FTVL, "UCHAR") diff --git a/tests/epics/signal/test_signals.py b/tests/epics/signal/test_signals.py index 4d6324486e..387fafb096 100644 --- a/tests/epics/signal/test_signals.py +++ b/tests/epics/signal/test_signals.py @@ -406,6 +406,19 @@ async def test_writing_to_ndarray_raises_typeerror(ioc_devices: EpicsTestIocAndD await signal.set(np.zeros((6,), dtype=np.int64)) +async def test_invalid_enum_choice_raises_valueerror( + ioc_devices: EpicsTestIocAndDevices, +): + signal = ioc_devices.ca_device.enum_str_fallback + await signal.connect() + with pytest.raises(ValueError) as exc: + await signal.set("Ddd") + assert "Ddd is not a valid choice for" in str(exc.value) + assert "ca:enum_str_fallback, valid choices: ['Aaa', 'Bbb', 'Ccc']" in str( + exc.value + ) + + @pytest.mark.parametrize("protocol", get_args(Protocol)) async def test_error_raised_on_disconnected_PV( ioc_devices: EpicsTestIocAndDevices, protocol: Protocol diff --git a/tests/epics/signal/test_yaml_save_ca.yaml b/tests/epics/signal/test_yaml_save_ca.yaml index 85e3f55fef..ad01da2d9d 100644 --- a/tests/epics/signal/test_yaml_save_ca.yaml +++ b/tests/epics/signal/test_yaml_save_ca.yaml @@ -1,6 +1,7 @@ bool_unnamed: true enum: Bbb enum2: Bbb +enum_str_fallback: Bbb float32a: [1.9999999949504854e-06, -123.12300109863281] float64a: [0.1, -12345678.123] float_prec_0: 3 diff --git a/tests/epics/signal/test_yaml_save_pva.yaml b/tests/epics/signal/test_yaml_save_pva.yaml index 15b3e18a77..a519f5d18d 100644 --- a/tests/epics/signal/test_yaml_save_pva.yaml +++ b/tests/epics/signal/test_yaml_save_pva.yaml @@ -1,6 +1,7 @@ bool_unnamed: true enum: Bbb enum2: Bbb +enum_str_fallback: Bbb float32a: [1.9999999949504854e-06, -123.12300109863281] float64a: [0.1, -12345678.123] float_prec_0: 3.0 From 20bfaac630e8857fb8bb6e368a4d61ca164e0c99 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Mon, 27 Jan 2025 12:12:40 -0500 Subject: [PATCH 2/4] Should still raise if value in valid choices but write failed --- src/ophyd_async/epics/core/_aioca.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ophyd_async/epics/core/_aioca.py b/src/ophyd_async/epics/core/_aioca.py index e9d6a4f6fd..fb1077b1f3 100644 --- a/src/ophyd_async/epics/core/_aioca.py +++ b/src/ophyd_async/epics/core/_aioca.py @@ -312,6 +312,8 @@ async def put(self, value: SignalDatatypeT | None, wait: bool): f"valid choices: {self.converter.metadata.get('choices')}" ) raise ValueError(msg) from exc + else: + raise else: raise From 9067b19dcfeb346335f1bfddd1c7d1450070e2c0 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Mon, 27 Jan 2025 13:15:03 -0500 Subject: [PATCH 3/4] Update src/ophyd_async/epics/core/_aioca.py Co-authored-by: Dominic Oram --- src/ophyd_async/epics/core/_aioca.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ophyd_async/epics/core/_aioca.py b/src/ophyd_async/epics/core/_aioca.py index fb1077b1f3..114b3b054a 100644 --- a/src/ophyd_async/epics/core/_aioca.py +++ b/src/ophyd_async/epics/core/_aioca.py @@ -305,7 +305,7 @@ async def put(self, value: SignalDatatypeT | None, wait: bool): # If we ran into a write error, check to see if there is a list # of valid choices, and if the value we tried to write is in that list. valid_choices = self.converter.metadata.get("choices") - if valid_choices is not None and len(valid_choices) > 0: + if valid_choices: if value not in valid_choices: msg = ( f"{value} is not a valid choice for {self.write_pv}, " From 207ce36327ee1fa3232dc270e2592ffb0fd0ca55 Mon Sep 17 00:00:00 2001 From: Jakub Wlodek Date: Mon, 27 Jan 2025 13:16:51 -0500 Subject: [PATCH 4/4] Remove unnecessary statements --- src/ophyd_async/epics/core/_aioca.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ophyd_async/epics/core/_aioca.py b/src/ophyd_async/epics/core/_aioca.py index 114b3b054a..d064fa1ad6 100644 --- a/src/ophyd_async/epics/core/_aioca.py +++ b/src/ophyd_async/epics/core/_aioca.py @@ -312,10 +312,8 @@ async def put(self, value: SignalDatatypeT | None, wait: bool): f"valid choices: {self.converter.metadata.get('choices')}" ) raise ValueError(msg) from exc - else: - raise - else: raise + raise async def get_datakey(self, source: str) -> DataKey: value = await self._caget(self.read_pv, FORMAT_CTRL)