Fix/issue 178 console flakiness - #224
Open
HussainAther wants to merge 17 commits into
Open
Conversation
Co-Authored-By: Claude Fable 5 <[email protected]>
…ish timeout vs cancel (Refs OpenwaterHealth#199) Wires CalibrationOutcome (Task 1) through both workers: - Append outcome field (default CalibrationOutcome.ERROR) to CalibrationResult and TestScanResult; rolled_back field on CalibrationResult for Task 3. - Moved CalibrationOutcome/_resolve_outcome above the result dataclasses (was after TestScanResult) so the dataclass field defaults can reference CalibrationOutcome.ERROR without a forward-reference error. - Add a worker-local timed_out flag, set via nonlocal from each worker's _watchdog, so a watchdog-triggered abort is distinguishable from an operator cancel_calibration() call. - Replace the fragile "canceled" in flash_err substring check with canceled = self._stop_evt.is_set() in both workers' flash-phase error handling. - Finally-block outcome resolution: when timed_out fired, it's authoritative over any phase-boundary check that already stamped a generic "canceled during/after X" message (that message just describes a symptom of the stop_evt the watchdog itself set) -- the error text and outcome unconditionally reflect the timeout. Otherwise the existing any-unaccounted-stop_evt-is-a-cancel backfill still applies. Calls _resolve_outcome (Task 1) rather than reimplementing the ok/passed/canceled/timed_out -> outcome logic inline. - write_result_json gains an optional outcome: str kwarg (default "" so existing callers in test_calibration_workflow_compute.py keep working); both workers pass outcome.value. Both "procedure complete" log lines gain outcome=%s. Tests: test_happy_path_outcome_is_passed, test_watchdog_timeout_outcome_is_timed_out, and a regression test (test_cancel_outcome_is_canceled_not_timed_out) for the old finally-block back-fill that could mislabel a plain cancel as a timeout. Co-Authored-By: Claude Fable 5 <[email protected]>
…t pass (Refs OpenwaterHealth#199) When phase 3 writes the freshly-computed calibration to the console before the phase-4 validation scan has judged it, any terminal outcome other than PASSED (FAILED, CANCELED, TIMED_OUT, ERROR) now leaves the unvalidated calibration on the device. Capture the pre-write calibration, and in the worker's finally block re-write it back (best-effort) whenever the resolved outcome isn't PASSED and a write actually happened. Adds CalibrationResult.rolled_back and the "calibration_rolled_back" manifest key so triage can tell whether the console still holds the write that was attempted. Co-Authored-By: Claude Fable 5 <[email protected]>
… to booleans (Refs OpenwaterHealth#199) CalibrationResult.outcome and TestScanResult.outcome defaulted to CalibrationOutcome.ERROR, so any legacy construction that omits outcome= presented as "error" to the app shim despite healthy ok/passed booleans. Both workers already pass outcome= explicitly, so production behavior is unchanged; only omitting callers (e.g. legacy test fixtures) are affected, and they now correctly fall back to the boolean triple via None. Also fixed the two "procedure complete" log lines to log outcome.value instead of the enum member, which rendered as "CalibrationOutcome.PASSED" on Python 3.12's %s formatting. Co-Authored-By: Claude Fable 5 <[email protected]>
Adds a console-side utility that verifies the packet-protocol clauses of SWREQ-196 / SWREQ-197 / SWREQ-202 by constructing frames directly -- including deliberately malformed ones -- and reporting the response packet type. These clauses cannot be exercised through the application UI, which reports only pass/fail and does not surface packet types or error codes. Checks (--check <name>, or --list): frame-format transmitted frame matches the specified fixed layout tid-echo transaction ID echoed unchanged in the response crc-mismatch corrupted CRC -> OW_BAD_CRC, command not executed no-start/no-end absent start/end byte -> framing error bad-length declared length inconsistent with frame -> rejected oversize declared payload > 2048 -> OW_BAD_PARSE payload-bounds 0-byte and 2048-byte payloads accepted unknown-opcode undefined opcode/packet type -> OW_UNKNOWN reserved-nonzero non-zero reserved byte -> rejected bad-subtarget non-existent sub-target -> rejected ordering responses returned in the order sent command-sweep every defined opcode in each packet type response-set all observed response types within the defined set Each check prints the transmitted frame bytes and the received response type so the console output serves as the verification record. Co-Authored-By: Claude Opus 5 <[email protected]>
…rotocol-test-utility scripts: add protocol_test.py packet-protocol verification utility
Two changes to db_key, both about not letting the clinical encryption path exist where it isn't validated. Refuse macOS outright. macOS is a research-only platform for this product and is never shipped in clinical mode, so the encrypted scan DB has no role there. The Keychain would be a technically adequate backend, but accepting it would mean the encryption path silently works on a platform nobody validates for clinical use. _assert_keystore_platform() raises EncryptionUnavailable on darwin from _keyring(), the single chokepoint every keystore touch routes through (_assert_backend, get_key, import_key, and export_key via get_key). The refusal lands before any keystore write or file read, so nothing can be half-provisioned on an unsupported platform. A macOS research build is unaffected: require_encryption() is False, so db_open takes the plaintext branch and never asks for a key. Pin the backend by module, not class name. The old check was `"WinVault" not in type(kr).__name__ and "Windows" not in type(kr).__module__`, which a plaintext backend defeats simply by naming its class WinVaultKeyring — and keyrings.alt stores secrets in plaintext, so that would silently void the at-rest guarantee the policy exists to enforce. Now matched against an explicit module allowlist, which also rejects keyring.backends.fail (no keystore at all) and keyring.backends.chainer (defers to whatever it discovered, so it can't be verified up front).
…-keychain-backend fix: refuse the scan-db keystore on macOS, and pin the backend by module
…sole (Refs OpenwaterHealth#199) Phase 3 wrote the newly computed calibration to console EEPROM *before* the validation scan could judge it, so every failing run destroyed the previous calibration and then restored it from a copy. Rollback made that survivable but not correct: the console was still mutated on the way through, and a restore that itself failed (USB drop mid-write) left the unvalidated calibration in place with nothing to put back. New phase 2.5 evaluates the calibration scan's own mean and contrast before anything is written. Both are calibration-independent — mean is the raw pixel average, contrast is speckle std/mean — so applying the computed calibration cannot change them, which is what makes it sound to judge them one scan early: a camera too dim here is still too dim in validation. BFI/BVI are excluded because they *are* the calibrated quantities and only become meaningful after the calibration is applied, which is what the validation scan is for. On a clean gate the run proceeds exactly as before — write, validate, verdict. On a failing gate the run stops before the write and asks on_confirm_fn, a new optional start_calibration handler receiving the measured rows so the UI can show what missed rather than prompting blind. Declining, a raising handler, or no handler at all aborts with the console untouched; the no-handler default keeps unattended callers conservative. Consent resumes the normal sequence and is recorded on the result as consented_below_threshold (plus a manifest key). The watchdog is paused across the prompt so operator think time is not charged against max_duration_sec — otherwise a slow answer surfaced as a bogus timeout. Since the handler blocks the worker, it owns its own deadline; the docstring says so. Rollback survives but is now keyed on consent: it fires only for a failure the operator never agreed to (in practice a BFI/BVI or ambient failure at validation, the cases the gate deliberately cannot judge). Undoing a consented write would discard exactly what was approved. Tests: 6 new gate tests (no-handler, decline, consent-keeps-write, silent on a good scan, raising handler, watchdog not charged for think time). The two existing rollback tests forced failure via min_mean=1e9, which the gate now catches first — repointed at BFI so they still exercise the path they were written for. 783 passed (hardware markers deselected). Co-Authored-By: Claude Opus 5 <[email protected]>
…99-calibration-outcome Refs OpenwaterHealth#199. Calibration outcome enum + pre-write gate so a failed run never overwrites the console EEPROM.
The checks only ever spoke to the console UART, so the same packet-protocol verification couldn't be pointed at a sensor. Add a Transport wrapper that carries the identical UartPacket frames over either the console UART or a sensor's COMMS bulk endpoint, selected with --target console|sensor and --side left|right. A sensor in async mode runs a reader thread that would swallow the very responses these checks inspect, so Transport pauses it for the duration and restarts it on exit. Co-Authored-By: Claude Opus 5 <[email protected]>
Prints the console's system odometer (powered-on minutes) and laser odometer (LSYNC pulses, ~40/s of scan) and exits. Written while verifying openmotion-console-fw#50. Co-Authored-By: Claude Fable 5 <[email protected]>
…11-read-odometers feat: add read_odometers.py bench script
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #178.
Summary
test_telemetry_listener_fireswait for two callbacks using athreading.Eventinstead of relying on a fixed sleep windowtry/finallytest_tec_voltage_readto use the public TEC DAC range[-5.0, 5.0]rather than the0.0-3.3 VADC rangeValidation
python -m pytest tests/test_db_key.py -vpython -m pytest -qThe hardware-dependent console tests skip locally because no Open-Motion
console is connected, so the original HIL flake still needs hardware-side
confirmation in CI or on a connected rig.