-
Notifications
You must be signed in to change notification settings - Fork 40
SoftSignalBackend wrapping arbitrary callables #1280
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.
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
b8888c5
15c67dc
6e90c40
b775f47
5713069
fbcb573
54abd02
f2d0e65
516733e
aa5b931
7207094
ed6214f
718f8fc
23f5fb9
d703fb4
25529e6
b31f2b1
e57dd39
1a640af
d57827c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
| import time | ||
| import typing | ||
| from abc import abstractmethod | ||
| from collections.abc import Sequence | ||
| from collections.abc import Awaitable, Callable, Sequence | ||
| from dataclasses import dataclass | ||
| from functools import lru_cache | ||
| from typing import Any, Generic, get_args | ||
|
|
@@ -29,9 +30,6 @@ | |
|
|
||
|
|
||
| class SoftConverter(Generic[SignalDatatypeT]): | ||
| # This is Any -> SignalDatatypeT because we support coercing | ||
| # value types to SignalDatatype to allow people to do things like | ||
| # SignalRW[Enum].set("enum value") | ||
| @abstractmethod | ||
| def write_value(self, value: Any) -> SignalDatatypeT: ... | ||
|
|
||
|
|
@@ -114,6 +112,10 @@ def make_converter(datatype: type[SignalDatatype]) -> SoftConverter: | |
| raise TypeError(f"Can't make converter for {datatype}") | ||
|
|
||
|
|
||
| Setter = Callable[[Any], SignalDatatypeT | None | Awaitable[SignalDatatypeT | None]] | ||
| Getter = Callable[[], SignalDatatypeT | Awaitable[SignalDatatypeT]] | ||
|
|
||
|
Comment on lines
+117
to
+123
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Disagree, #1280 (comment) would pass the converted value to the setter |
||
|
|
||
| class SoftSignalBackend(SignalBackend[SignalDatatypeT]): | ||
| """An backend to a soft Signal, for test signals see [](#MockSignalBackend). | ||
|
|
||
|
|
@@ -124,6 +126,16 @@ class SoftSignalBackend(SignalBackend[SignalDatatypeT]): | |
| :param units: The units for numeric datatypes. | ||
| :param precision: | ||
| The number of digits after the decimal place to display for a float datatype. | ||
| :param getter: | ||
| Optional callable returning the current device value, called on | ||
| get_value/get_reading and periodically if poll_period is set. | ||
| :param setter: | ||
| Optional callable performing the set action. May return the settled | ||
| value; if it returns None and a getter is configured, the getter is | ||
| called to refresh the cache. | ||
| :param poll_period: | ||
| How often (seconds) to call the getter while a subscription is active. | ||
| Requires getter to be set. | ||
| """ | ||
|
|
||
| def __init__( | ||
|
|
@@ -132,18 +144,51 @@ def __init__( | |
| initial_value: SignalDatatypeT | None = None, | ||
| units: str | None = None, | ||
| precision: int | None = None, | ||
| *, | ||
| getter: Getter[SignalDatatypeT] | None = None, | ||
| setter: Setter[Any] | None = None, | ||
| poll_period: float | None = None, | ||
| ): | ||
| # Create the right converter for the datatype | ||
| if poll_period is not None and getter is None: | ||
| raise ValueError("poll_period requires a getter to be set") | ||
| self.converter = make_converter(datatype or float) | ||
| # Add the extra static metadata to the dictionary | ||
| self.metadata = make_metadata(datatype, units, precision) | ||
| # Create and set the initial value | ||
| self.initial_value = self.converter.write_value(initial_value) | ||
| self.reading: Reading[SignalDatatypeT] | ||
| self.callback: Callback[Reading[SignalDatatypeT]] | None = None | ||
| self._getter = getter | ||
| self._setter = setter | ||
| self._poll_period = poll_period | ||
| self._poll_task: asyncio.Task | None = None | ||
| self.set_value(self.initial_value) | ||
| super().__init__(datatype) | ||
|
|
||
| async def _call_getter(self) -> SignalDatatypeT: | ||
|
burkeds marked this conversation as resolved.
Outdated
|
||
| if self._getter is None: | ||
| raise RuntimeError("No getter configured") | ||
| result = self._getter() | ||
| if isinstance(result, Awaitable): | ||
| result = await result | ||
| return self.converter.write_value(result) | ||
|
burkeds marked this conversation as resolved.
Outdated
burkeds marked this conversation as resolved.
Outdated
|
||
|
|
||
| async def _call_setter(self, value: Any) -> SignalDatatypeT | None: | ||
|
burkeds marked this conversation as resolved.
Outdated
|
||
| if self._setter is None: | ||
| raise RuntimeError("No setter configured") | ||
| result = self._setter(value) | ||
| if isinstance(result, Awaitable): | ||
| result = await result | ||
| return result | ||
|
burkeds marked this conversation as resolved.
Outdated
|
||
|
|
||
| async def _poll(self) -> None: | ||
| if self._poll_period is None: | ||
| raise RuntimeError("No poll_period configured") | ||
| while True: | ||
| await asyncio.sleep(self._poll_period) | ||
| try: | ||
| self.set_value(await self._call_getter()) | ||
| except Exception: | ||
| continue | ||
|
|
||
| def set_value(self, value: SignalDatatypeT): | ||
| """Set the current value, alarm and timestamp.""" | ||
| self.reading = Reading( | ||
|
|
@@ -160,19 +205,32 @@ def source(self, name: str, read: bool) -> str: | |
| async def connect(self, timeout: float): | ||
| pass | ||
|
|
||
| async def put(self, value: SignalDatatypeT | None) -> None: | ||
| async def put(self, value: Any) -> None: | ||
| write_value = self.initial_value if value is None else value | ||
| self.set_value(write_value) | ||
| if self._setter is not None: | ||
| settled = await self._call_setter(write_value) | ||
|
burkeds marked this conversation as resolved.
Outdated
|
||
| if settled is not None: | ||
| self.set_value(self.converter.write_value(settled)) | ||
| elif self._getter is not None: | ||
| self.set_value(await self._call_getter()) | ||
| else: | ||
| self.set_value(self.converter.write_value(write_value)) | ||
|
burkeds marked this conversation as resolved.
Outdated
|
||
| else: | ||
| self.set_value(write_value) | ||
|
Comment on lines
+207
to
+218
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, as per #1280 (comment) |
||
|
|
||
| async def get_datakey(self, source: str) -> DataKey: | ||
| return make_datakey( | ||
| self.datatype or float, self.reading["value"], source, self.metadata | ||
| ) | ||
|
|
||
| async def get_reading(self) -> Reading[SignalDatatypeT]: | ||
| if self._getter is not None: | ||
| self.set_value(await self._call_getter()) | ||
| return self.reading | ||
|
|
||
| async def get_value(self) -> SignalDatatypeT: | ||
| if self._getter is not None: | ||
| self.set_value(await self._call_getter()) | ||
| return self.reading["value"] | ||
|
|
||
| async def get_setpoint(self) -> SignalDatatypeT: | ||
|
|
@@ -184,4 +242,10 @@ def set_callback(self, callback: Callback[Reading[SignalDatatypeT]] | None) -> N | |
| raise RuntimeError("Cannot set a callback when one is already set") | ||
| if callback: | ||
| callback(self.reading) | ||
| if self._poll_period is not None: | ||
| self._poll_task = asyncio.get_event_loop().create_task(self._poll()) | ||
|
burkeds marked this conversation as resolved.
Outdated
|
||
| else: | ||
| if self._poll_task is not None: | ||
| self._poll_task.cancel() | ||
| self._poll_task = None | ||
| self.callback = callback | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -204,3 +204,234 @@ async def test_soft_signal_coerces_numpy_types(): | |
| soft_signal._connector.backend.set_value(np.float64(2.2)) | ||
| assert await soft_signal.get_value() == 2.2 | ||
| assert type(await soft_signal.get_value()) is float | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_getter(): | ||
| store = [42.0] | ||
| backend = SoftSignalBackend(float, getter=lambda: store[0]) | ||
| await backend.connect(timeout=1) | ||
| assert await backend.get_value() == 42.0 | ||
| store[0] = 99.0 | ||
| assert await backend.get_value() == 99.0 | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_async_getter(): | ||
| store = [42.0] | ||
|
|
||
| async def getter(): | ||
| return store[0] | ||
|
|
||
| backend = SoftSignalBackend(float, getter=getter) | ||
| await backend.connect(timeout=1) | ||
| store[0] = 99.0 | ||
| assert await backend.get_value() == 99.0 | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_getter_updates_reading(): | ||
| store = [1.0] | ||
| backend = SoftSignalBackend(float, getter=lambda: store[0]) | ||
| await backend.connect(timeout=1) | ||
| store[0] = 2.0 | ||
| reading = await backend.get_reading() | ||
| assert reading["value"] == 2.0 | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_getter_does_not_affect_setpoint(): | ||
| store = [1.0] | ||
| backend = SoftSignalBackend(float, getter=lambda: store[0]) | ||
| await backend.connect(timeout=1) | ||
| await backend.put(5.0) | ||
| store[0] = 99.0 | ||
| assert await backend.get_setpoint() == 5.0 | ||
| assert await backend.get_value() == 99.0 | ||
|
Comment on lines
+245
to
+246
|
||
|
|
||
|
|
||
| async def test_soft_signal_backend_setter_homogeneous_types(): | ||
| store = [0.0] | ||
| backend = SoftSignalBackend(float, setter=lambda v: store.__setitem__(0, v)) | ||
|
burkeds marked this conversation as resolved.
Outdated
|
||
| await backend.connect(timeout=1) | ||
| await backend.put(7.0) | ||
| assert store[0] == 7.0 | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_setter_heterogeneous_types(): | ||
| counts_written = [] | ||
|
|
||
| def counts_setter(counts: int) -> float: | ||
| counts_written.append(counts) | ||
| return counts * 0.01 | ||
|
|
||
| backend = SoftSignalBackend(float, setter=counts_setter) | ||
| await backend.connect(timeout=1) | ||
| await backend.put(1000) | ||
| assert counts_written == [1000] | ||
| assert await backend.get_value() == pytest.approx(10.0) | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_async_setter_heterogeneous_types(): | ||
| counts_written = [] | ||
|
|
||
| async def counts_setter(counts: int) -> float: | ||
| counts_written.append(counts) | ||
| return counts * 0.01 | ||
|
|
||
| backend = SoftSignalBackend(float, setter=counts_setter) | ||
| await backend.connect(timeout=1) | ||
| await backend.put(500) | ||
| assert counts_written == [500] | ||
| assert await backend.get_value() == pytest.approx(5.0) | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_setter_heterogeneous_none_return_with_getter(): | ||
| store = [0.0] | ||
| commands = [] | ||
|
|
||
| def command_setter(cmd: str) -> None: | ||
| commands.append(cmd) | ||
| store[0] = float(cmd.split("=")[1]) | ||
|
|
||
| backend = SoftSignalBackend(float, setter=command_setter, getter=lambda: store[0]) | ||
| await backend.connect(timeout=1) | ||
| await backend.put("pos=42.5") | ||
| assert commands == ["pos=42.5"] | ||
| assert await backend.get_value() == pytest.approx(42.5) | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_setter_heterogeneous_none_return_without_getter(): | ||
| received = [] | ||
|
|
||
| def int_setter(v: int) -> None: | ||
| received.append(v) | ||
|
|
||
| backend = SoftSignalBackend(float, setter=int_setter) | ||
| await backend.connect(timeout=1) | ||
| await backend.put(42) | ||
| assert received == [42] | ||
| assert await backend.get_value() == pytest.approx(42.0) | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_async_setter(): | ||
| store = [0.0] | ||
|
|
||
| async def setter(v): | ||
| store[0] = v | ||
|
|
||
| backend = SoftSignalBackend(float, setter=setter) | ||
| await backend.connect(timeout=1) | ||
| await backend.put(7.0) | ||
| assert store[0] == 7.0 | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_setter_returns_settled_value(): | ||
| def clamping_setter(v): | ||
| return max(0.0, min(10.0, v)) | ||
|
|
||
| backend = SoftSignalBackend(float, setter=clamping_setter) | ||
| await backend.connect(timeout=1) | ||
| await backend.put(50.0) | ||
| assert await backend.get_value() == 10.0 | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_setter_none_with_getter_refreshes(): | ||
| store = [0.0] | ||
|
|
||
| def setter(v): | ||
| store[0] = v | ||
|
|
||
| backend = SoftSignalBackend(float, setter=setter, getter=lambda: store[0]) | ||
| await backend.connect(timeout=1) | ||
| await backend.put(3.0) | ||
| assert await backend.get_value() == 3.0 | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_setter_none_without_getter_stores_write_value(): | ||
| called_with = [] | ||
|
|
||
| def setter(v): | ||
| called_with.append(v) | ||
|
|
||
| backend = SoftSignalBackend(float, setter=setter) | ||
| await backend.connect(timeout=1) | ||
| await backend.put(6.0) | ||
| assert called_with == [6.0] | ||
| assert await backend.get_value() == 6.0 | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_poll_period_without_getter_raises(): | ||
| with pytest.raises(ValueError, match="poll_period requires a getter"): | ||
| SoftSignalBackend(float, poll_period=0.1) | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_poll_period_updates_via_callback(): | ||
| store = [0.0] | ||
| backend = SoftSignalBackend(float, getter=lambda: store[0], poll_period=0.05) | ||
| await backend.connect(timeout=1) | ||
|
|
||
| updates: asyncio.Queue[Reading] = asyncio.Queue() | ||
| backend.set_callback(updates.put_nowait) | ||
|
|
||
| # Consume the initial callback fired by set_callback | ||
| await updates.get() | ||
|
|
||
| store[0] = 5.0 | ||
| reading = await asyncio.wait_for(updates.get(), timeout=1.0) | ||
| assert reading["value"] == 5.0 | ||
|
|
||
| backend.set_callback(None) | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_poll_task_starts_and_stops(): | ||
| backend = SoftSignalBackend(float, getter=lambda: 0.0, poll_period=0.05) | ||
| await backend.connect(timeout=1) | ||
|
|
||
| updates: asyncio.Queue[Reading] = asyncio.Queue() | ||
| assert backend._poll_task is None | ||
|
|
||
| backend.set_callback(updates.put_nowait) | ||
| assert backend._poll_task is not None | ||
| assert not backend._poll_task.cancelled() | ||
|
|
||
| backend.set_callback(None) | ||
| assert backend._poll_task is None | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_no_poll_without_poll_period(): | ||
| # A getter without poll_period should not start a background task. | ||
| backend = SoftSignalBackend(float, getter=lambda: 0.0) | ||
| await backend.connect(timeout=1) | ||
|
|
||
| updates: asyncio.Queue[Reading] = asyncio.Queue() | ||
| backend.set_callback(updates.put_nowait) | ||
| assert backend._poll_task is None | ||
| backend.set_callback(None) | ||
|
|
||
|
|
||
| async def test_soft_signal_backend_setter_accepts_config_object(): | ||
|
|
||
| class MotorConfig: | ||
| velocity: float | ||
| acceleration: float | ||
| units: str | ||
|
|
||
| configs_received = [] | ||
| store = [0.0] | ||
|
|
||
| def config_setter(config: MotorConfig) -> float: | ||
| configs_received.append(config) | ||
| store[0] = config.velocity | ||
| return config.velocity | ||
|
|
||
| backend = SoftSignalBackend(float, setter=config_setter) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, is it a requirement for
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think I quite understand. How does it promote requiring something? If
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Example: sig = soft_signal_rw(float, setter=config_setter)
await sig.set(32.4) # pyright says ok, runtime says ok
value = await sig.get_value() # value is float
await sig.set(MotorConfig(...)) # pyright says bad, runtime says ok
value = await sig.get_value() # value is floatI don't think I want to promote a pattern where the set argument type is different from the value type
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @coretl What do you suggest then? If we say that the setter can only take one argument of the same type as the value carried by the function, then that strictly limits the types of operations we can reasonably support.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have viewed a few third-party libraries for device control, not enough to constitute as an expert on the matter, but I haven't found so far the situation where an entire data structure is passed down to a function controlling a device. I've seen more use case of APIs with multiple parameters, but in that case using @burkeds have you encountered these situations before?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, we probably need the concept of setpoint and reading:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A possible issue. Currently, the value is not converted until If we put Should we pass the converted write value to the def set_value(self, value: SignalDatatypeT):
"""Set the current value, alarm and timestamp."""
self.reading = Reading(
value=self.converter.write_value(value),
timestamp=time.time(),
alarm_severity=0,
)
if self.callback:
self.callback(self.reading)
async def put(self, value: Any) -> None:
write_value = self.initial_value if value is None else value
if self._setter is not None:
written_value = await maybe_await(self._setter(value))
if written_value is not None:
self.set_value(written_value)
elif self._getter is not None:
await self._update_value_from_getter()
else:
self.set_value(write_value)
else:
self.set_value(write_value)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that's probably best
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the setter is possibly async, it can't go in set_value. That means to pass the converted value to the setter we need to convert the value twice, once in put and once in set_value. I didn't want to do that. Instead, I thought we could pass the unconverted value to the setter (as I am sure a user would expect) and the setpoint is then recorded as the value passed to set_value.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I agree, let's go with an example of a float signal with a setter:
In this example I would suggest that the setpoint is If so then this becomes: async def put(self, value: Any) -> None:
self._setpoint = self.initial_value if value is None else self.converter.write_value(value)
if self._setter is not None:
readback = await maybe_await(self._setter(value))
if readback is not None:
self.set_value(readback)
elif self._getter is not None:
await self._update_value_from_getter()
else:
self.set_value(self._setpoint)
else:
self.set_value(self._setpoint) |
||
| await backend.connect(timeout=1) | ||
|
|
||
| cfg = MotorConfig() | ||
| cfg.velocity = 2.5 | ||
| cfg.acceleration = 0.1 | ||
| cfg.units = "mm/s" | ||
| await backend.put(cfg) | ||
|
|
||
| assert len(configs_received) == 1 | ||
| assert configs_received[0].velocity == 2.5 | ||
| assert configs_received[0].acceleration == 0.1 | ||
| assert configs_received[0].units == "mm/s" | ||
| assert await backend.get_value() == pytest.approx(2.5) | ||
|
burkeds marked this conversation as resolved.
Outdated
|
||
Uh oh!
There was an error while loading. Please reload this page.