-
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 1 commit
b8888c5
15c67dc
6e90c40
b775f47
5713069
fbcb573
54abd02
f2d0e65
516733e
aa5b931
7207094
ed6214f
718f8fc
23f5fb9
d703fb4
25529e6
b31f2b1
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 |
|---|---|---|
|
|
@@ -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 | ||
|
|
||
|
|
||
| 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
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. async def put(self, value: Any) -> None:
write_value = self.initial_value if value is None else value
self._setpoint = write_value
...
async def get_setpoint(self) -> SignalDatatypeT:
# For a soft signal, the setpoint and readback values are the same.
if self._setter is None and self._getter is None:
return self.reading["value"]
else:
return self._setpointSomething like this?
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. |
||
| 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.