diff --git a/apps/studio/src/orisys_studio/app_window.py b/apps/studio/src/orisys_studio/app_window.py index e4c5a1a..125bd04 100644 --- a/apps/studio/src/orisys_studio/app_window.py +++ b/apps/studio/src/orisys_studio/app_window.py @@ -28,12 +28,10 @@ from PyQt5.QtWidgets import ( QWidget, ) -from orisys_app_shared.acquisition.constants import ( - FORCE_SAMPLE_RATE_HZ, - LOWPASS_CUTOFF_HZ, -) +from orisys_app_shared.acquisition.constants import FORCE_SAMPLE_RATE_HZ from orisys_app_shared.force import select_display_forces from orisys_app_shared.force.calibration_recorder import CalibrationRecorder +from orisys_app_shared.force.filters import FilterSettings, settings_from_mapping from orisys_app_shared.force.session import SessionStore from orisys_app_shared.paths import recordings_dir from orisys_app_shared.record import RecordConfig @@ -547,10 +545,7 @@ class StudioMainWindow(QMainWindow): ), "frame_id": "monotonic per acquisition session", }, - "filter": { - "lowpass_cutoff_hz": float(LOWPASS_CUTOFF_HZ), - "sample_rate_hz": float(FORCE_SAMPLE_RATE_HZ), - }, + "filter": self._filter_metadata(), "recording_mode": self.toolbar.record_mode(), "video_config": config.as_dict(), } @@ -577,6 +572,61 @@ class StudioMainWindow(QMainWindow): except Exception: # noqa: BLE001 return "unavailable" + def _filter_settings(self) -> FilterSettings: + """The filters the operator asked for, from the preferences file. + + One place reads them, so the dialog that writes them and the session that + runs them cannot drift apart. An empty or unreadable preference is + "nothing filters" — the same default the shipped sensor configs state. + """ + keys = ( + "flow_ema_alpha", + "force_median_window", + "force_lowpass_cutoff_hz", + "force_lowpass_order", + "force_lowpass_sample_rate_hz", + "show_unfiltered", + "preset", + ) + stored = {} + for key in keys: + try: + value = self.prefs.get(f"plot_{key}") + except Exception: # noqa: BLE001 - a broken preference is not fatal + value = None + if value is not None: + stored[key] = value + return settings_from_mapping(stored) + + def _filter_metadata(self) -> dict: + """The filters this session actually ran, for ``session.json``. + + Read from the worker that is running the device, not from the settings + the operator chose: those are two different things whenever the SDK is + older than the request, and the record has to describe the data it holds. + Never raises — a record that cannot name its filters is still a record. + """ + try: + worker = self._active_worker + if worker is None: + worker = self._worker_for(self.sensor_picker.sensor_type()) + except Exception: # noqa: BLE001 + logger.exception("Could not find the worker to ask about the filters") + return FilterSettings().metadata() + try: + state = worker.filter_state() if worker is not None else {} + settings = worker.filter_settings() if worker is not None else None + unsupported = tuple(worker.filters_unsupported()) if worker is not None else () + except Exception: # noqa: BLE001 + logger.exception("Could not read the filter state for the record") + state, settings, unsupported = {}, None, () + block = (settings or FilterSettings()).metadata(state or None) + if unsupported: + # Named, not hidden: the record says which settings the installed SDK + # could not apply, so the numbers can be read for what they are. + block["unsupported_by_sdk"] = list(unsupported) + return block + # ------------------------------------------------------- record channel def _on_packet_recorded(self, packet) -> None: """Runs on the **acquisition thread**. Must stay cheap and Qt-free. @@ -2047,6 +2097,10 @@ class StudioMainWindow(QMainWindow): # it is wired even if the worker was installed by something other than # ``_ensure_workers`` (``add_sink`` is idempotent). self._active_worker.add_sink(self._on_packet_recorded) + # The filters for this session, before it opens the device: the chain runs + # in the SDK, so this is the one moment the choice can be handed over. + # The acquisition thread applies it and records what the device reports. + self._active_worker.set_filter_settings(self._filter_settings()) try: self._active_worker.start(source, config_ref=config_ref) except Exception as exc: diff --git a/apps/studio/src/orisys_studio/ui/force_plots.py b/apps/studio/src/orisys_studio/ui/force_plots.py index 5abfa76..c86409c 100644 --- a/apps/studio/src/orisys_studio/ui/force_plots.py +++ b/apps/studio/src/orisys_studio/ui/force_plots.py @@ -15,7 +15,7 @@ from PyQt5.QtWidgets import ( QVBoxLayout, ) -from orisys_app_shared.acquisition.constants import FORCE_SAMPLE_RATE_HZ, LOWPASS_CUTOFF_HZ +from orisys_app_shared.acquisition.constants import FORCE_SAMPLE_RATE_HZ from orisys_app_shared.force import MetricHistory from orisys_app_shared.force.history import DEFAULT_MAX_FPS from orisys_app_shared.force.source import StoreCurveSource @@ -158,7 +158,12 @@ class ForcePlotsPanel(QFrame): channels=3, window_sec=4.0, sample_rate_hz=FORCE_SAMPLE_RATE_HZ, - lowpass_cutoff_hz=LOWPASS_CUTOFF_HZ, + # No filtering here: the SDK already filtered these readings (median, + # low-pass, flow EMA — whatever the session was configured with) and + # this is the same buffer that goes into record/forces.csv. Filtering + # again would draw a curve the file does not contain, and the + # zero-phase filtfilt this used to do is not even causal. + lowpass_cutoff_hz=0.0, y_axis_min_span=0.5, max_fps=DEFAULT_MAX_FPS, ) diff --git a/apps/studio/tools/ui_check.py b/apps/studio/tools/ui_check.py index 1045bd4..d6d3ce1 100644 --- a/apps/studio/tools/ui_check.py +++ b/apps/studio/tools/ui_check.py @@ -32,6 +32,7 @@ if sys.platform == "win32" and "QT_QPA_FONTDIR" not in os.environ: os.chdir(APP_ROOT) from orisys_app_shared.acquisition.sinks import RecordSinkMixin # noqa: E402 +from orisys_app_shared.force.filters import FilterSettings # noqa: E402 from orisys_app_shared.record import FORCES_HEADER # noqa: E402 FAILURES: list[str] = [] @@ -237,6 +238,22 @@ class _StuckWorker(RecordSinkMixin): #: Zeroing requests, and the confirmation callback the window installs. self.reset_requests = 0 self.on_reset_applied = None + #: Filters handed over before a session, and what the fake device reports + #: as in effect (this stub has no ``configure``, so it reports nothing). + self.filter_settings_value = FilterSettings() + self.filter_state_value: dict = {} + + def set_filter_settings(self, settings) -> None: + self.filter_settings_value = settings + + def filter_settings(self): + return self.filter_settings_value + + def filter_state(self) -> dict: + return dict(self.filter_state_value) + + def filters_unsupported(self) -> tuple[str, ...]: + return () def set_pending_calibration(self, path) -> None: self.calibration_requests.append(str(path)) diff --git a/shared/src/orisys_app_shared/acquisition/__init__.py b/shared/src/orisys_app_shared/acquisition/__init__.py index e60fcbf..219e55b 100644 --- a/shared/src/orisys_app_shared/acquisition/__init__.py +++ b/shared/src/orisys_app_shared/acquisition/__init__.py @@ -1,13 +1,9 @@ """Acquisition helpers.""" -from orisys_app_shared.acquisition.constants import ( - FORCE_SAMPLE_RATE_HZ, - LOWPASS_CUTOFF_HZ, -) +from orisys_app_shared.acquisition.constants import FORCE_SAMPLE_RATE_HZ __all__ = [ "FORCE_SAMPLE_RATE_HZ", - "LOWPASS_CUTOFF_HZ", "FingerAcquisitionWorker", "SdkAcquisitionWorker", "SdkPacket", diff --git a/shared/src/orisys_app_shared/acquisition/constants.py b/shared/src/orisys_app_shared/acquisition/constants.py index 72a8f44..0b6f8cd 100644 --- a/shared/src/orisys_app_shared/acquisition/constants.py +++ b/shared/src/orisys_app_shared/acquisition/constants.py @@ -1,4 +1,6 @@ """Shared acquisition constants (no heavy imports).""" -LOWPASS_CUTOFF_HZ = 10.0 +#: The rate the SDK designs its force low-pass coefficients for until the device +#: measures its own. Also the assumption the plots and a recording's time base +#: are built on, so it stays a constant rather than a per-run guess. FORCE_SAMPLE_RATE_HZ = 30.0 diff --git a/shared/src/orisys_app_shared/acquisition/finger_worker.py b/shared/src/orisys_app_shared/acquisition/finger_worker.py index 3da6ec7..2ef4ded 100644 --- a/shared/src/orisys_app_shared/acquisition/finger_worker.py +++ b/shared/src/orisys_app_shared/acquisition/finger_worker.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging import threading import time from pathlib import Path @@ -9,10 +10,6 @@ from typing import Callable, Optional import numpy as np -from orisys_app_shared.acquisition.constants import ( - FORCE_SAMPLE_RATE_HZ, - LOWPASS_CUTOFF_HZ, -) from orisys_app_shared.acquisition.sinks import RecordSinkMixin # The packet and the geometry come from ``models``, so the finger worker does not # depend on the box worker; only genuinely shared helpers are imported from it. @@ -35,6 +32,9 @@ from orisys_app_shared.acquisition.sdk_worker import ( process_chain, read_forces_from_sensor, ) +from orisys_app_shared.force.filters import ALL_KEYS, FilterSettings, effective_filters + +logger = logging.getLogger(__name__) _EXPORT_REQUIRED = ("mesh.npz", "camera_intrinsics.npz", "world_to_cam.npy") @@ -99,6 +99,32 @@ class FingerAcquisitionWorker(RecordSinkMixin): self._pending_reset: bool = False self._mono_t0: float = 0.0 self._calibration_version: str = "" + #: What the filters should do; set before ``start()`` (see the box worker + #: for the reasoning: one session, one way of filtering). + self._filter_settings: FilterSettings = FilterSettings() + #: ``filters_state()`` of the running session: the effective values. + self._filter_state: dict = {} + #: Keys the installed SDK could not apply. + self._filters_unsupported: tuple[str, ...] = () + + def set_filter_settings(self, settings: Optional[FilterSettings]) -> None: + """Choose the filters for the **next** session.""" + with self._lock: + self._filter_settings = (settings or FilterSettings()).normalised() + + def filter_settings(self) -> FilterSettings: + with self._lock: + return self._filter_settings + + def filter_state(self) -> dict: + """What the running device reports as in effect (``{}`` if it cannot say).""" + with self._lock: + return dict(self._filter_state) + + def filters_unsupported(self) -> tuple[str, ...]: + """Settings the installed SDK could not apply, reported not swallowed.""" + with self._lock: + return tuple(self._filters_unsupported) @property def is_running(self) -> bool: @@ -145,6 +171,10 @@ class FingerAcquisitionWorker(RecordSinkMixin): # zeroing request must not fire into the new session's frames. self._calibration_version = "" self._pending_reset = False + # Nor any filter state: until this session's sensor has been + # configured and asked, there is nothing to report about it. + self._filter_state = {} + self._filters_unsupported = () self._session_id += 1 session = self._session_id self._stop.clear() @@ -325,13 +355,18 @@ class FingerAcquisitionWorker(RecordSinkMixin): return raw_sensor = getattr(runtime.tactile, "raw_sensor", None) if raw_sensor is not None and hasattr(raw_sensor, "configure"): + # The filter chain lives in the SDK; the app says what the + # operator asked for and reads back what the device is doing. + with self._lock: + settings = self._filter_settings try: - raw_sensor.configure( - lowpass_cutoff_hz=LOWPASS_CUTOFF_HZ, - lowpass_sample_rate_hz=FORCE_SAMPLE_RATE_HZ, - ) - except Exception: - pass + unsupported = settings.apply_to(raw_sensor) + except Exception: # noqa: BLE001 - a finger runtime may refuse + logger.exception("Could not apply filter settings to the finger sensor") + unsupported = ALL_KEYS + with self._lock: + self._filter_state = effective_filters(raw_sensor) + self._filters_unsupported = tuple(unsupported) self._status(f"Streaming finger {mode.label()}") # The tactile sensor loads its own bundle calibration while opening; # name those coefficients from the first frame, not the request path. diff --git a/shared/src/orisys_app_shared/acquisition/sdk_worker.py b/shared/src/orisys_app_shared/acquisition/sdk_worker.py index f876d34..983402b 100644 --- a/shared/src/orisys_app_shared/acquisition/sdk_worker.py +++ b/shared/src/orisys_app_shared/acquisition/sdk_worker.py @@ -18,10 +18,8 @@ import numpy as np logger = logging.getLogger(__name__) -from orisys_app_shared.acquisition.constants import ( - FORCE_SAMPLE_RATE_HZ, - LOWPASS_CUTOFF_HZ, -) +from orisys_app_shared.acquisition.constants import FORCE_SAMPLE_RATE_HZ +from orisys_app_shared.force.filters import FilterSettings, effective_filters from orisys_app_shared.acquisition.models import ( FixedCameraMode, SdkPacket, @@ -409,6 +407,44 @@ class SdkAcquisitionWorker(RecordSinkMixin): #: Content hash of the calibration currently in effect. self._calibration_version: str = "" self._frame_id: int = 0 + #: What the filters should do; set before ``start()``. The acquisition + #: thread applies it once the sensor is open, because re-configuring a + #: device that is being read is not safe mid-session. + self._filter_settings: FilterSettings = FilterSettings() + #: ``Sensor.filters_state()`` of the running session: the *effective* + #: values, which are what the dialog shows and the record keeps. + self._filter_state: dict = {} + #: Keys the installed SDK cannot apply (an SDK older than 0.5.2). + self._filters_unsupported: tuple[str, ...] = () + + def set_filter_settings(self, settings: Optional[FilterSettings]) -> None: + """Choose the filters for the **next** session. + + Not applied to a running one on purpose: the filter controls are locked + while recording, so every frame of a session is filtered the same way. + """ + with self._lock: + self._filter_settings = ( + settings or FilterSettings() + ).normalised() + + def filter_settings(self) -> FilterSettings: + with self._lock: + return self._filter_settings + + def filter_state(self) -> dict: + """What the running device reports as in effect. + + ``{}`` means the SDK could not be asked (older than 0.5.2) — which is not + the same as "nothing is filtering" and must not be recorded as if it were. + """ + with self._lock: + return dict(self._filter_state) + + def filters_unsupported(self) -> tuple[str, ...]: + """Settings the installed SDK could not apply, reported not swallowed.""" + with self._lock: + return tuple(self._filters_unsupported) @property def is_running(self) -> bool: @@ -454,6 +490,10 @@ class SdkAcquisitionWorker(RecordSinkMixin): # an old reset request must not fire into the new session's frames. self._calibration_version = "" self._pending_reset = False + # Same for the filter state: until this session's sensor has been + # configured and asked, there is nothing to report about it. + self._filter_state = {} + self._filters_unsupported = () self._session_id += 1 session = self._session_id self._stop.clear() @@ -636,10 +676,14 @@ class SdkAcquisitionWorker(RecordSinkMixin): if self._stop.is_set() or session != self._session_id: self._close_sensor(session) return - sensor.configure( - lowpass_cutoff_hz=LOWPASS_CUTOFF_HZ, - lowpass_sample_rate_hz=FORCE_SAMPLE_RATE_HZ, - ) + # The filter chain lives in the SDK; the app only says what the + # operator asked for and then reads back what the device is doing. + with self._lock: + settings = self._filter_settings + unsupported = settings.apply_to(sensor) + with self._lock: + self._filter_state = effective_filters(sensor) + self._filters_unsupported = tuple(unsupported) # A sensor loads its own bundle calibration while opening, so the # coefficients in effect from the very first frame are already known # here. Without this the record would name the path passed on the diff --git a/shared/src/orisys_app_shared/force/__init__.py b/shared/src/orisys_app_shared/force/__init__.py index 83a5aa5..2509037 100644 --- a/shared/src/orisys_app_shared/force/__init__.py +++ b/shared/src/orisys_app_shared/force/__init__.py @@ -17,6 +17,7 @@ from orisys_app_shared.force.filters import ( FilterSettings, ForceFilterChain, describe_state, + effective_filters, settings_from_mapping, ) from orisys_app_shared.force.history import MetricHistory, PlotState, lowpass_filter @@ -49,6 +50,7 @@ __all__ = [ "SessionStore", "build_force_calibration_v2", "describe_state", + "effective_filters", "export_force_calibration_v2", "lowpass_filter", "meta_from_sensor_json", diff --git a/shared/src/orisys_app_shared/force/filters.py b/shared/src/orisys_app_shared/force/filters.py index 6d844f9..921b868 100644 --- a/shared/src/orisys_app_shared/force/filters.py +++ b/shared/src/orisys_app_shared/force/filters.py @@ -60,6 +60,9 @@ CONFIGURE_PARAMETERS: dict[str, str] = { "force_lowpass_sample_rate_hz": "lowpass_sample_rate_hz", } +#: Every key above, in chain order — "nothing could be applied" is this tuple. +ALL_KEYS: tuple[str, ...] = tuple(CONFIGURE_PARAMETERS) + #: A sensible cutoff to offer when the operator switches the low-pass on. SUGGESTED_LOWPASS_HZ = 10.0 @@ -227,7 +230,7 @@ class FilterSettings: try: parameters = inspect.signature(sensor.configure).parameters except (AttributeError, TypeError, ValueError): # pragma: no cover - exotic fake - return tuple(CONFIGURE_PARAMETERS) + return ALL_KEYS return tuple( key for key, parameter in CONFIGURE_PARAMETERS.items() @@ -353,6 +356,24 @@ def settings_from_mapping(data: Optional[Mapping[str, Any]]) -> FilterSettings: ).normalised() +def effective_filters(sensor: object) -> dict[str, Any]: + """What a sensor reports as in effect, for a record or for the dialog. + + ``{}`` means the question could not be asked — an SDK older than 0.5.2 has no + ``filters_state()`` — which is different from "nothing is filtering" and has + to stay different in the record. + """ + reader = getattr(sensor, "filters_state", None) + if not callable(reader): + return {} + try: + state = reader() + except Exception: # noqa: BLE001 - metadata must never break a session + logger.exception("Could not read the sensor's filter state") + return {} + return dict(state) if isinstance(state, Mapping) else {} + + # --------------------------------------------------------------------------- # Reference implementation: kept so the tests can prove this app understands the # SDK's chain exactly. Nothing in the app calls it. diff --git a/shared/tests/test_force_filters.py b/shared/tests/test_force_filters.py index 58ae275..b62b21b 100644 --- a/shared/tests/test_force_filters.py +++ b/shared/tests/test_force_filters.py @@ -246,6 +246,68 @@ def test_settings_round_trip_through_a_preferences_mapping(): assert settings_from_mapping(None).active() is False +# ------------------------------------------------- the worker's filter contract +def test_a_worker_reports_no_filter_state_until_a_session_has_one(): + """``{}`` means "not asked yet", which is not the same as "nothing filters".""" + from orisys_app_shared.acquisition.sdk_worker import SdkAcquisitionWorker + + worker = SdkAcquisitionWorker() + assert worker.filter_state() == {} + assert worker.filters_unsupported() == () + worker.set_filter_settings(FilterSettings(median_window=3)) + assert worker.filter_settings().median_window == 3 + # Clearing the preference is "nothing filters", not "keep the last choice". + worker.set_filter_settings(None) + assert worker.filter_settings() == FilterSettings() + assert worker.filter_settings().active() is False + + +def test_effective_filters_says_nothing_when_the_sdk_cannot_answer(): + from orisys_app_shared.force.filters import effective_filters + + assert effective_filters(object()) == {} + + class Broken: + def filters_state(self): + raise RuntimeError("no") + + assert effective_filters(Broken()) == {} + + +def test_the_settings_reach_a_real_sensor_and_come_back_as_the_truth(): + """The whole point of this module, against the SDK itself. + + No hardware: the SDK's external-frame mode constructs a Sensor from its own + bundle, which is exactly what the acquisition worker does before applying + the session's filters. + """ + orisys = pytest.importorskip("orisys") + from orisys_app_shared.force.filters import effective_filters + + sensor = orisys.Sensor("external", config_name="box", verbose=False) + try: + assert sensor.filters_state()["any_stage_applied"] is False + unsupported = FilterSettings( + flow_ema_alpha=0.5, median_window=3, lowpass_hz=10.0 + ).apply_to(sensor) + assert unsupported == (), "the installed SDK is missing filter settings" + state = effective_filters(sensor) + assert state["measurement"]["flow_ema_alpha"] == 0.5 + assert state["readout"]["force_median_window"] == 3 + assert state["readout"]["force_lowpass_applied"] is True + read_back = FilterSettings.from_filters_state(state) + assert read_back.describe() == "flowEMA(0.5) + median(3) + lowpass(10Hz,order2)" + # ...and the unfiltered pair is available for the second buffer. + triplets = sensor.force_triplets() + assert set(triplets) == {"filtered", "unfiltered"} + assert set(triplets["unfiltered"]) == {"raw", "newton"} + # Turning everything off is expressible, and the device agrees. + FilterSettings().apply_to(sensor) + assert effective_filters(sensor)["any_stage_applied"] is False + finally: + sensor.disconnect() + + # ------------------------------------------- the reference chain (SDK parity) def test_a_disabled_chain_is_a_pass_through(): chain = ForceFilterChain(FilterSettings())