diff --git a/CLAUDE.md b/CLAUDE.md index aec3ac4..5da2714 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,7 +38,7 @@ CI's separate `boot` job is what verifies `src/main.py` actually starts against `controls(odrv)` takes a **live `fibre` remote object**, so it is dynamically typed (`Any`). Every attribute it reads (`odrv.axis0.motor.current_control.Iq_measured`, `odrv.axis0.controller.config.pos_gain`, …) mirrors the ODrive 0.5.x/0.6.x object tree. -Every property read is a blocking USB round-trip on the event loop, so keep timers lean. +Every property read is a blocking USB round-trip, so after the panel is built all device traffic goes through the single `_usb` worker in `controls.py` (`_Mirror` for bound values, `_on_usb` for timer reads, `_send` for writes and commands) — never read or write the device on the event loop, and keep timers lean. When you change the UI, mirror the same attribute path in `tools/mock_odrive.py` so the hardware-free tests keep covering it. ## Page structure (NiceGUI 3.x) diff --git a/src/controls.py b/src/controls.py index e98e5de..9afaef5 100644 --- a/src/controls.py +++ b/src/controls.py @@ -12,18 +12,27 @@ from __future__ import annotations +import asyncio +import importlib +import itertools import logging from collections.abc import Callable +from concurrent.futures import Future, ThreadPoolExecutor from datetime import datetime from typing import Any from nicegui import ui -from nicegui.events import ValueChangeEventArguments from odrive.pyfibre import fibre from odrive.utils import dump_errors log = logging.getLogger('odrive_gui') +# ``import odrive`` loads libfibre a second time, as top-level ``fibre``, and the device objects +# come from that copy: its ObjectLostError and EmptyInterface are other classes than ours. +_device_fibre = importlib.import_module('fibre') # what odrive's own discovery thread imports +LOST_ERRORS = (fibre.ObjectLostError, _device_fibre.ObjectLostError) +EMPTY_INTERFACES = (fibre.libfibre.EmptyInterface, _device_fibre.libfibre.EmptyInterface) + MODES: dict[int, str] = { 0: 'Voltage', 1: 'Torque', @@ -41,6 +50,8 @@ 7: 'Mirror', } +IDLE = 1 + STATES: dict[int, str] = { 0: 'Undefined', 1: 'Idle', @@ -55,7 +66,88 @@ def _lost(obj: Any) -> bool: which every attribute read raises ``AttributeError``. The panel's timers can fire in the short window before the discovery loop deletes the panel, so they check first. """ - return obj.__class__ is fibre.libfibre.EmptyInterface + return obj.__class__ in EMPTY_INTERFACES + + +def is_lost(e: BaseException) -> bool: + """Whether ``e`` means "the device left the bus" rather than a bug in our code. + + fibre reports a loss in two ways: a read that fails on the bus raises + ``ObjectLostError``, but once libfibre's lost-object callback has run -- on *its own* + thread, so at any point, even between two reads of a synchronous panel build -- the + object's class is swapped to ``EmptyInterface`` and every attribute read raises a + plain ``AttributeError``. Only an ``AttributeError`` on such a lost object counts; + a typo'd attribute path in ``controls()`` still surfaces. + """ + if isinstance(e, LOST_ERRORS): + return True + return isinstance(e, AttributeError) and isinstance(e.obj, EMPTY_INTERFACES) + + +# Every device read or write is a blocking USB round-trip. After the panel is built, all of +# them run on this one worker, in the order they were issued: the event loop never waits for +# the bus, and a command can never overtake a setting the user changed before it. +_usb = ThreadPoolExecutor(max_workers=1, thread_name_prefix='odrive-usb') +_write_order = itertools.count() + + +def _send(obj: Any, fn: Callable[[], Any]) -> None: + """Queue ``fn()`` (a write or command) on the USB worker; log it if it fails.""" + + def done(future: Future) -> None: + e = future.exception() + if e is not None and not (is_lost(e) or _lost(obj)): + log.error('ODrive write failed', exc_info=e) + + _usb.submit(fn).add_done_callback(done) + + +async def _on_usb(obj: Any, fn: Callable[[], Any]) -> Any: + """``fn()`` on the USB worker; ``None`` if ``obj``'s device left the bus meanwhile. + + ``fn`` may read through sub-objects of ``obj``. libfibre tears a lost device down + children first, so such a read can fail while ``obj`` still looks alive: ``is_lost`` + recognises it by the object the ``AttributeError`` names. + """ + try: + return await asyncio.get_running_loop().run_in_executor(_usb, fn) + except Exception as e: + if is_lost(e) or _lost(obj): + return None + raise + + +class _Mirror: + """A local copy of the attributes widgets bind to on one fibre object. + + NiceGUI cannot be notified by a fibre object, so it polls every bound attribute each + 0.1 s -- on the event loop. Widgets bind to this mirror instead: reading it is free, + ``sync()`` refreshes it on the USB worker, and a widget's change is written through there. + """ + + def __init__(self, obj: Any) -> None: + object.__setattr__(self, '_obj', obj) + object.__setattr__(self, '_values', {}) + object.__setattr__(self, '_written', {}) # name -> _write_order of its last write + + def __getattr__(self, name: str) -> Any: + if name not in self._values: + self._values[name] = getattr(self._obj, name) # first read, while building the panel + return self._values[name] + + def __setattr__(self, name: str, value: Any) -> None: + self._values[name] = value + self._written[name] = next(_write_order) + _send(self._obj, lambda: setattr(self._obj, name, value)) + + async def sync(self) -> None: + names = list(self._values) + start = next(_write_order) + values = await _on_usb(self._obj, lambda: [getattr(self._obj, name) for name in names]) + for name, value in zip(names, values or [], strict=False): + # a write queued after this read started is newer than what the read saw + if self._written.get(name, -1) < start: + self._values[name] = value def controls(odrv: Any) -> None: @@ -70,7 +162,7 @@ def controls(odrv: Any) -> None: def reboot() -> None: try: odrv.reboot() - except fibre.ObjectLostError: + except LOST_ERRORS: # the device drops off the USB bus mid-reboot; that specific loss is expected. # do NOT touch `odrv` here — a live read on the just-disconnected device raises again. log.info('ODrive %x rebooting (connection dropped as expected)', serial) @@ -81,15 +173,20 @@ def reboot() -> None: _chip(f'HW {odrv.hw_version_major}.{odrv.hw_version_minor}.{odrv.hw_version_variant}') _chip(f'FW {odrv.fw_version_major}.{odrv.fw_version_minor}.{odrv.fw_version_revision}{" (dev)" if odrv.fw_version_unreleased else ""}') voltage = ui.label().classes('text-lg font-medium text-primary') - ui.timer(1.0, lambda: _lost(odrv) or voltage.set_text(f'{odrv.vbus_voltage:.2f} V')) + + async def update_voltage() -> None: + if (vbus := await _on_usb(odrv, lambda: odrv.vbus_voltage)) is not None: + voltage.set_text(f'{vbus:.2f} V') + + ui.timer(1.0, update_voltage) with ui.row().classes('gap-1'): # wrap in a lambda (like the original): passing the bare fibre RemoteFunction makes # NiceGUI introspect its signature, which is brittle on the C-backed proxy. - ui.button(icon='save', on_click=lambda: odrv.save_configuration()).props('dense').tooltip('Save configuration') - ui.button(icon='bug_report', on_click=lambda: dump_errors(odrv, hasattr(odrv, 'clear_errors'))).props('dense').tooltip( - 'Dump and clear errors' - ) - ui.button(icon='restart_alt', on_click=reboot).props('dense').tooltip('Reboot ODrive') + ui.button(icon='save', on_click=lambda: _send(odrv, lambda: odrv.save_configuration())).props('dense').tooltip('Save configuration') + ui.button(icon='bug_report', on_click=lambda: _send(odrv, lambda: dump_errors(odrv, hasattr(odrv, 'clear_errors')))).props( + 'dense' + ).tooltip('Dump and clear errors') + ui.button(icon='restart_alt', on_click=lambda: _send(odrv, reboot)).props('dense').tooltip('Reboot ODrive') with ui.row().classes('gap-4 items-stretch p-4'): for index, axis in enumerate([odrv.axis0, odrv.axis1]): @@ -117,39 +214,67 @@ def _create_axis_column(index: int, axis: Any) -> None: power = ui.label() # access via lambda, not `on_click=axis.clear_errors`: the method is optional on # older firmware, so it must not be read until the button (if shown) is clicked. - button = ui.button(icon='bug_report', color='negative', on_click=lambda: axis.clear_errors()).props('dense').tooltip('Clear errors') + button = ( + ui.button(icon='bug_report', color='negative', on_click=lambda: _send(axis, lambda: axis.clear_errors())) + .props('dense') + .tooltip('Clear errors') + ) button.set_visibility(hasattr(axis, 'clear_errors')) - ctr_cfg = axis.controller.config - mtr_cfg = axis.motor.config - enc_cfg = axis.encoder.config - trp_cfg = axis.trap_traj.config + mirrors = [_Mirror(obj) for obj in (axis, axis.controller.config, axis.motor.config, axis.encoder.config, axis.trap_traj.config)] + axis_state, ctr_cfg, mtr_cfg, enc_cfg, trp_cfg = mirrors cc = axis.motor.current_control - def update_power() -> None: - if _lost(axis): - return - power.set_text(f'{cc.Iq_measured * cc.v_current_control_integral_q:.1f} W') + async def sync_mirrors() -> None: + for mirror in mirrors: + await mirror.sync() + show_state(axis_state.current_state) + + async def update_power() -> None: + if (watts := await _on_usb(axis, lambda: cc.Iq_measured * cc.v_current_control_integral_q)) is not None: + power.set_text(f'{watts:.1f} W') - # every property read is a blocking USB round-trip on the event loop, so keep the - # 10 Hz path to the two values that must feel live; the error flag only enables a - # button and can follow at 1 Hz. + async def update_error() -> None: + if (error := await _on_usb(axis, lambda: axis.error)) is not None: + button.set_enabled(error != 0) + + # every property read is a USB round-trip, so keep the 10 Hz path to the two values + # that must feel live; the error flag only enables a button and can follow at 1 Hz. ui.timer(0.1, update_power) - ui.timer(1.0, lambda: _lost(axis) or button.set_enabled(axis.error != 0)) - - def request_state(e: ValueChangeEventArguments) -> None: - # Only a *user* choice becomes a request. The toggle also changes when the - # binding below mirrors a new current_state into it (then value == current_state) - # or coerces a state outside STATES (calibration, homing) to None — neither may be - # written back: a two-way bind did exactly that and e.g. re-requested CLOSED_LOOP - # for an axis that came up in it, which makes 0.5.x firmware leave and re-enter - # closed loop (a brief disarm of a live motor). - if e.value is not None and e.value != axis.current_state: - axis.requested_state = e.value + ui.timer(0.1, sync_mirrors) + ui.timer(1.0, update_error) + + def request_state(state: int) -> None: + if state != IDLE and state == axis_state.current_state: + return # re-requesting e.g. CLOSED_LOOP would briefly disarm the motor (see below) + _send(axis, lambda: setattr(axis, 'requested_state', state)) + + # The axis state is one button per state, not a toggle: a click on Idle must reach the + # device, but a toggle drops a click on the option it already shows -- and the state + # shown can be a refresh old (the firmware has not switched yet), so that click can be + # the Idle that stops a motor. A toggle's value changes are no better a trigger: they + # also fire when a refresh shows the device's state, and writing that back re-requested + # e.g. CLOSED_LOOP for an axis already in it, which makes 0.5.x firmware leave and + # re-enter closed loop (a brief disarm of a live motor). So clicks request, refreshes show, + # and a click on any other state the axis is already in is dropped: at worst a stale + # state swallows a click that starts nothing. + state_buttons: dict[int, ui.button] = {} + shown: list[int | None] = [None] + + def show_state(state: int) -> None: + if state == shown[0]: + return + shown[0] = state + for value, state_button in state_buttons.items(): + state_button.props(remove='flat unelevated color') + state_button.props('unelevated color=primary' if value == state else 'flat') with ui.row().classes('gap-2'): mode = ui.toggle(MODES).bind_value(ctr_cfg, 'control_mode') - ui.toggle(STATES, on_change=request_state).bind_value_from(axis, 'current_state') + with ui.element('div').classes('q-btn-toggle row no-wrap inline'): + for value, label in STATES.items(): + state_buttons[value] = ui.button(label, on_click=lambda value=value: request_state(value)).props(remove='round').props('no-caps') + show_state(axis_state.current_state) with ui.row().classes('gap-4 items-start'): for value, name, attr, icons in _MOTION_INPUTS: @@ -210,7 +335,8 @@ def _motion_input(axis: Any, name: str, attr: str, icons: tuple[str, str, str]) field = ui.number(f'input {name.lower()}', value=0) def send(sign: int) -> None: - setattr(axis.controller, attr, sign * _field_value(field)) + value = sign * _field_value(field) + _send(axis, lambda: setattr(axis.controller, attr, value)) with ui.row().classes('w-full justify-around gap-0'): for icon, sign in zip(icons, (-1, 0, 1), strict=True): @@ -225,12 +351,13 @@ def _live_plots(axis: Any) -> None: plots: list[tuple[ui.checkbox, ui.line_plot, Callable[[], list[list[float]]]]] = [] timer: ui.timer | None = None - def push() -> None: + async def push() -> None: now = datetime.now() - for check, plot, sample in plots: - if check.value: - # line_plot uses a datetime x-axis (matplotlib handles it); the stub types x as float. - plot.push([now], sample()) # type: ignore[list-item] + shown = [(plot, sample) for check, plot, sample in plots if check.value] + samples = await _on_usb(axis, lambda: [sample() for _, sample in shown]) + for (plot, _), values in zip(shown, samples or [], strict=False): + # line_plot uses a datetime x-axis (matplotlib handles it); the stub types x as float. + plot.push([now], values) # type: ignore[list-item] def update_timer() -> None: nonlocal timer diff --git a/src/main.py b/src/main.py index 0ef6b2e..8ea7ffc 100755 --- a/src/main.py +++ b/src/main.py @@ -13,9 +13,8 @@ import odrive from nicegui import app, ui -from odrive.pyfibre import fibre -from controls import controls +from controls import controls, is_lost from theme import apply_theme, header logging.basicConfig(level=logging.WARNING, format='%(asctime)s %(levelname)s %(message)s') @@ -30,21 +29,6 @@ devices: dict[int, Any] = {} -def is_lost(e: Exception) -> bool: - """Whether ``e`` means "the device left the bus" rather than a bug in our code. - - fibre reports a loss in two ways: a read that fails on the bus raises - ``ObjectLostError``, but once libfibre's lost-object callback has run -- on *its own* - thread, so at any point, even between two reads of a synchronous panel build -- the - object's class is swapped to ``EmptyInterface`` and every attribute read raises a - plain ``AttributeError``. Only an ``AttributeError`` on such a lost object counts; - a typo'd attribute path in ``controls()`` still surfaces. - """ - if isinstance(e, fibre.ObjectLostError): - return True - return isinstance(e, AttributeError) and isinstance(e.obj, fibre.libfibre.EmptyInterface) - - @ui.refreshable def device_panels() -> None: """One full-width panel per connected device, or a placeholder if there is none. diff --git a/tests/test_main.py b/tests/test_main.py index d8de470..65bce0a 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -8,13 +8,34 @@ """ import asyncio +import threading import pytest -from mock_odrive import arrive_mid_scan, lose_mock_odrive, make_mock_odrive, set_connected_devices +from mock_odrive import arrive_mid_scan, lose_mock_odrive, make_mock_odrive, object_lost_error, set_connected_devices from nicegui import ui from nicegui.testing import User -from controls import STATES +from controls import STATES, _usb + + +def state_buttons(user: User) -> dict[int, ui.button]: + return {state: user.find(kind=ui.button, content=label).elements.pop() for state, label in STATES.items()} + + +def pick(user: User, state: int) -> None: + """Click the axis-state button for ``state``.""" + user.find(kind=ui.button, content=STATES[state]).click() + + +def shown_state(user: User) -> int | None: + """The state the axis-state buttons highlight.""" + return next((state for state, button in state_buttons(user).items() if button.props.get('color') == 'primary'), None) + + +async def drain() -> None: + """Wait until every device write queued so far has run on the USB worker.""" + await asyncio.wrap_future(_usb.submit(lambda: None)) + pytestmark = pytest.mark.nicegui_main_file('src/main.py') @@ -92,33 +113,33 @@ async def test_every_client_sees_the_devices(user: User, create_user) -> None: await other.should_see('SN 3333') -async def test_state_toggle_only_writes_user_choices(user: User) -> None: - """The axis-state toggle mirrors ``current_state`` but must not echo it back as a - request: only a state picked in the UI is written to ``requested_state``.""" - dev = make_mock_odrive(two_axes=False) # one axis -> exactly one state toggle on the page +async def test_state_buttons_show_the_device_state_and_only_clicks_request_one(user: User) -> None: + """The highlighted state follows ``current_state``, but only a click writes + ``requested_state`` -- a refresh never echoes the device's state back as a request.""" + dev = make_mock_odrive(two_axes=False) # one axis -> exactly one set of state buttons axis = dev.axis0 axis.current_state = 8 # came up in CLOSED_LOOP_CONTROL axis.requested_state = 'untouched' set_connected_devices([dev]) await user.open('/') await user.should_see('Axis 0') - await asyncio.sleep(0.3) # a few binding refresh cycles + await asyncio.sleep(0.3) # a few refreshes assert axis.requested_state == 'untouched' # rendering did not request anything + assert shown_state(user) == 8 - state_toggle = next(t for t in user.find(ui.toggle).elements if t.options == STATES) - assert state_toggle.value == 8 - state_toggle.set_value(1) # the user picks Idle + pick(user, 1) # the user clicks Idle + await drain() assert axis.requested_state == 1 axis.requested_state = 'untouched' - axis.current_state = 1 # the firmware followed; the toggle mirrors it … + axis.current_state = 1 # the firmware followed; the buttons show it … await asyncio.sleep(0.3) - assert state_toggle.value == 1 + assert shown_state(user) == 1 assert axis.requested_state == 'untouched' # … without writing it back - axis.current_state = 3 # a state the toggle has no option for (calibration) … + axis.current_state = 3 # a state without a button (calibration) … await asyncio.sleep(0.3) - assert state_toggle.value is None - assert axis.requested_state == 'untouched' # … is not written back as 0 either + assert shown_state(user) is None + assert axis.requested_state == 'untouched' # … is not written back either async def test_stop_with_cleared_input_field_still_writes_zero(user: User) -> None: @@ -132,4 +153,146 @@ async def test_stop_with_cleared_input_field_still_writes_zero(user: User) -> No field = next(n for n in user.find(ui.number).elements if n.props['label'] == 'input torque') field.set_value(None) # the user cleared the box user.find(ui.button).trigger('click') # includes the torque stop button (visible buttons only) + await drain() assert dev.axis0.controller.input_torque == 0.0 + + +async def test_gain_edits_reach_the_device_and_device_changes_reach_the_ui(user: User) -> None: + """The widgets bind to local mirrors, so both directions must still get through: an + edit is written to the device (and not undone by a refresh that read the old value + in the meantime), and a change made on the device shows up in the UI.""" + device = make_mock_odrive(two_axes=False) # one axis -> exactly one pos_gain field + set_connected_devices([device]) + await user.open('/') + await user.should_see('pos_gain') + field = next(n for n in user.find(ui.number).elements if n.props.get('label') == 'pos_gain') + config = device.axis0.controller.config + + field.set_value(42.0) + await asyncio.sleep(1.2) + assert config.pos_gain == 42.0 + assert field.value == 42.0 + + config.pos_gain = 7.0 # changed behind the UI's back (odrivetool, another client) + await asyncio.sleep(1.2) + assert field.value == 7.0 + + +def _record_state_requests(axis, follow: bool = True) -> list[tuple[int, float]]: + """Record every ``requested_state`` write, with the ``current_lim`` the device had then.""" + seen: list[tuple[int, float]] = [] + + def _request(self, state: int) -> None: + seen.append((state, self.motor.config.current_lim)) + if follow: + self.current_state = state # the firmware follows at once + + axis.__class__ = type( + 'RecordingAxis', + (type(axis),), + {'requested_state': property(lambda self: seen[-1][0] if seen else 1, _request)}, + ) + return seen + + +async def test_a_quick_loop_then_idle_is_not_dropped(user: User) -> None: + """Loop then Idle before any refresh: both are requested, in order.""" + dev = make_mock_odrive(two_axes=False) + seen = _record_state_requests(dev.axis0) + set_connected_devices([dev]) + await user.open('/') + await user.should_see('Axis 0') + gate = threading.Event() + _usb.submit(gate.wait) # hold the worker: no refresh can run in between + pick(user, 8) + pick(user, 1) + gate.set() + await drain() + assert [state for state, _ in seen] == [8, 1] + + +async def test_a_command_does_not_overtake_a_setting_changed_before_it(user: User) -> None: + """Lowering ``current_lim`` and then entering closed loop must reach the device in that + order, even while the USB worker is busy: the limit is the motor's protection.""" + dev = make_mock_odrive(two_axes=False) + seen = _record_state_requests(dev.axis0) + set_connected_devices([dev]) + await user.open('/') + await user.should_see('current_lim') + limit = next(n for n in user.find(ui.number).elements if n.props.get('label') == 'current_lim') + gate = threading.Event() + _usb.submit(gate.wait) # the worker is busy (a slow read, a refresh) + limit.set_value(1.0) + pick(user, 8) + gate.set() + await drain() + assert seen == [(8, 1.0)] + + +async def test_idle_is_requested_even_while_idle_is_the_state_shown(user: User) -> None: + """The user clicks Loop; refreshes still read Idle because the firmware has not switched + yet, so Idle is what is shown; then the firmware enters Loop. The user's click on the + Idle shown must still be requested -- a toggle dropped it (on the server and in the + browser alike) and left the axis in closed loop.""" + dev = make_mock_odrive(two_axes=False) + seen = _record_state_requests(dev.axis0, follow=False) + set_connected_devices([dev]) + await user.open('/') + await user.should_see('Axis 0') + pick(user, 8) + await asyncio.sleep(0.3) # refreshes still read Idle + assert shown_state(user) == 1 + gate = threading.Event() + _usb.submit(gate.wait) # no refresh shows the switch before the click + dev.axis0.current_state = 8 # now the firmware switches + pick(user, 1) + gate.set() + await drain() + assert [state for state, _ in seen] == [8, 1] + + +async def test_a_click_on_the_state_the_axis_is_already_in_is_not_requested(user: User) -> None: + """Re-requesting CLOSED_LOOP for an axis already in it makes 0.5.x firmware leave and + re-enter closed loop (a brief disarm of a live motor), so a second click on Loop is + dropped. Idle is still requested, whatever the state shown.""" + dev = make_mock_odrive(two_axes=False) + dev.axis0.current_state = 8 + seen = _record_state_requests(dev.axis0, follow=False) + set_connected_devices([dev]) + await user.open('/') + await user.should_see('Axis 0') + assert shown_state(user) == 8 + pick(user, 8) + pick(user, 1) + await drain() + assert [state for state, _ in seen] == [1] + + +async def test_a_sub_object_lost_before_its_axis_is_not_an_error(user: User) -> None: + """libfibre tears a lost device down children first, on its own thread, so a timer can + read through an already lost sub-object (the current controller for the power label) + while the axis it checks still looks alive. That read is a lost device, not a bug.""" + dev = make_mock_odrive(two_axes=False) + set_connected_devices([dev]) + await user.open('/') + await user.should_see('Axis 0') + lose_mock_odrive(dev.axis0.motor.current_control) + await asyncio.sleep(0.3) # several 10 Hz power-label updates + + +async def test_a_device_dropping_off_mid_save_or_reboot_is_not_an_error(user: User) -> None: + """This firmware reboots on save, and reboot drops the device: both calls then raise the + device's ``ObjectLostError``. That comes from ``odrive``'s second, top-level copy of + libfibre, not ``odrive.pyfibre.fibre``, and must not be logged as a failed write.""" + dev = make_mock_odrive(two_axes=False) + + def lost() -> None: + raise object_lost_error() + + dev.save_configuration = lost + dev.reboot = lost + set_connected_devices([dev]) + await user.open('/') + await user.should_see('Axis 0') + user.find(ui.button).trigger('click') # includes save and reboot; any ERROR log fails the test + await drain() diff --git a/tests/test_render.py b/tests/test_render.py index 1da92b4..bfab7e7 100644 --- a/tests/test_render.py +++ b/tests/test_render.py @@ -8,10 +8,14 @@ """ import asyncio +import threading +from mock_odrive import READS from nicegui import ui from nicegui.testing import User +from controls import _Mirror, _usb + async def test_panel_renders(user: User) -> None: await user.open('/') @@ -61,3 +65,37 @@ async def test_live_plots_and_telemetry_read_the_mock(user: User) -> None: for check in checks: check.set_value(False) await asyncio.sleep(0.1) # the per-axis plot timers are torn down without error + + +async def test_live_values_are_not_read_on_the_event_loop(user: User) -> None: + """Every device read is a blocking USB round-trip, so after the panel is built none may + happen on the event loop: widgets bind to local mirrors that a worker thread refreshes. + Bound straight to the fibre objects instead, NiceGUI's binding loop polls each of them + on the loop every 0.1 s (two calibrated axes: ~440 USB reads per second).""" + await user.open('/') + await user.should_see('Axis 1') + loop_thread = threading.current_thread().name + before = READS.copy() + await asyncio.sleep(1.2) # several binding refreshes and mirror syncs, every 10 Hz/1 Hz timer + assert READS[loop_thread] == before[loop_thread] + assert READS['odrive-usb_0'] > before['odrive-usb_0'] # the live values are still read, on the worker + + +async def test_a_refresh_that_read_before_an_edit_does_not_undo_it(user: User, mock_odrive) -> None: + """A refresh reads on the USB worker while the user may edit the same value: the read + sees the old device value, and applying it would snap the widget back -- whose change + handler then writes the old value to the device too. Holding the worker makes the + read-queued-before-the-write order deterministic.""" + config = mock_odrive.axis0.controller.config + mirror = _Mirror(config) + assert mirror.pos_gain == 20.0 # registers pos_gain for refreshes + gate = threading.Event() + _usb.submit(gate.wait) + sync = asyncio.create_task(mirror.sync()) + await asyncio.sleep(0.01) # the refresh's read is now queued behind the gate + mirror.pos_gain = 42.0 # and the write behind it + gate.set() + await sync + await asyncio.sleep(0.05) + assert mirror.pos_gain == 42.0 + assert config.pos_gain == 42.0 diff --git a/tools/mock_odrive.py b/tools/mock_odrive.py index 8e262c6..0234a4d 100644 --- a/tools/mock_odrive.py +++ b/tools/mock_odrive.py @@ -15,27 +15,46 @@ from __future__ import annotations +import collections import concurrent.futures import math +import os import sys +import threading import time import types _T0 = time.time() +# device attribute reads per thread name, so tests can tell which thread talks to the "bus" +READS: collections.Counter[str] = collections.Counter() +# a real read is a USB round-trip; MOCK_ODRIVE_READ_MS makes every mock read cost one too +READ_LATENCY = float(os.environ.get('MOCK_ODRIVE_READ_MS', '0')) / 1000 + def _wave(period: float, amp: float, offset: float = 0.0) -> float: return offset + amp * math.sin(2 * math.pi * (time.time() - _T0) / period) -class _Config: +class _Recorded: + """Counts every public attribute read (see ``READS``), each costing ``READ_LATENCY``.""" + + def __getattribute__(self, name: str): + if not name.startswith('_'): + READS[threading.current_thread().name] += 1 + if READ_LATENCY: + time.sleep(READ_LATENCY) + return object.__getattribute__(self, name) + + +class _Config(_Recorded): """A plain settable namespace -- NiceGUI bindings get/set attributes here.""" def __init__(self, **kw: object) -> None: self.__dict__.update(kw) -class _CurrentControl: +class _CurrentControl(_Recorded): Iq_setpoint = 0.4 Id_setpoint = 0.0 @@ -52,7 +71,7 @@ def v_current_control_integral_q(self) -> float: return _wave(3.0, 0.2, 1.1) -class _Thermistor: +class _Thermistor(_Recorded): """The FET thermistor, mirroring ``odrv.axisN.motor.fet_thermistor``. Exposes only ``.temperature`` — deliberately its own class rather than aliasing the @@ -66,18 +85,18 @@ def temperature(self) -> float: return _wave(20.0, 4.0, 32.0) -class _MockAxis: +class _MockAxis(_Recorded): def __init__(self, calibrated: bool = True, control_mode: int = 2) -> None: self.error = 0 self.requested_state = 1 self.current_state = 1 - self.motor = types.SimpleNamespace( + self.motor = _Config( is_calibrated=calibrated, current_control=_CurrentControl(), # lives on the motor only, as on hardware config=_Config(current_lim=10.0, current_control_bandwidth=1000.0, torque_lim=float('inf'), requested_current_range=60.0), fet_thermistor=_Thermistor(), ) - self.controller = types.SimpleNamespace( + self.controller = _Config( input_torque=0.0, input_vel=0.0, input_pos=0.0, @@ -98,13 +117,13 @@ def __init__(self, calibrated: bool = True, control_mode: int = 2) -> None: ), ) self.encoder = _Encoder() - self.trap_traj = types.SimpleNamespace(config=_Config(vel_limit=2.0, accel_limit=0.5, decel_limit=0.5)) + self.trap_traj = _Config(config=_Config(vel_limit=2.0, accel_limit=0.5, decel_limit=0.5)) def clear_errors(self) -> None: self.error = 0 -class _Encoder: +class _Encoder(_Recorded): """Encoder with a settable config and animated position/velocity estimates.""" def __init__(self) -> None: @@ -128,7 +147,7 @@ def make_mock_odrive(serial: int = 0x208E39855253, two_axes: bool = True, contro ax0 = _MockAxis(calibrated=True, control_mode=control_mode) ax1 = _MockAxis(calibrated=two_axes, control_mode=control_mode) - class _Dev: + class _Dev(_Recorded): serial_number = serial hw_version_major, hw_version_minor, hw_version_variant = 3, 6, 56 fw_version_major, fw_version_minor, fw_version_revision = 0, 5, 6 @@ -195,6 +214,11 @@ def install_odrive_stub() -> None: fibre.ObjectLostError = type('ObjectLostError', (Exception,), {}) # type: ignore[attr-defined] fibre.libfibre = types.SimpleNamespace(EmptyInterface=type('EmptyInterface', (), {})) pyfibre.fibre = fibre + # ``import odrive`` also loads libfibre as top-level ``fibre``, and the device objects come + # from that copy, so its classes are not ``odrive.pyfibre.fibre``'s -- keep them distinct here. + device_fibre = types.ModuleType('fibre') + device_fibre.ObjectLostError = type('ObjectLostError', (Exception,), {}) # type: ignore[attr-defined] + device_fibre.libfibre = types.SimpleNamespace(EmptyInterface=type('EmptyInterface', (), {})) # type: ignore[attr-defined] utils = types.ModuleType('odrive.utils') utils.dump_errors = lambda odrv, clear=False: None # type: ignore[attr-defined] odrive.pyfibre = pyfibre # type: ignore[attr-defined] @@ -205,6 +229,7 @@ def install_odrive_stub() -> None: 'odrive.pyfibre': pyfibre, 'odrive.pyfibre.fibre': fibre, 'odrive.utils': utils, + 'fibre': device_fibre, } ) @@ -229,10 +254,15 @@ def lose_mock_odrive(device: object) -> None: """Make ``device`` behave like a fibre object whose USB connection was lost. Mirrors ``RemoteObject._destroy()``: the instance's class is swapped to - ``EmptyInterface``, so every attribute read from now on raises ``AttributeError``. + ``EmptyInterface`` (of the top-level ``fibre`` copy, as on hardware), so every attribute + read from now on raises ``AttributeError``. """ - fibre = sys.modules['odrive.pyfibre.fibre'] - device.__class__ = fibre.libfibre.EmptyInterface # type: ignore[attr-defined] + device.__class__ = sys.modules['fibre'].libfibre.EmptyInterface # type: ignore[attr-defined] + + +def object_lost_error() -> Exception: + """The ``ObjectLostError`` a real device raises when it drops off the bus mid-call.""" + return sys.modules['fibre'].ObjectLostError('the object disappeared') # type: ignore[attr-defined] def reset_odrive_stub() -> None: @@ -264,7 +294,7 @@ def arrive_mid_scan(during: object, arriving: object) -> dict: def read_once(self) -> int: type(self).serial_number = serial # fire on the first read only - scan = sys._getframe(1) # the frame whose read sprang the trap + scan = sys._getframe(2) # the frame whose read sprang the trap (1 is _Recorded.__getattribute__) report['fired_mid_scan'] = scan.f_code.co_name == 'discovery_loop' and serial not in scan.f_globals.get('devices', {}) set_connected_devices([during, arriving]) return serial