Skip to content

feat: surface console printf; unify console + sensor command transports - #97

Draft
boringethan wants to merge 4 commits into
nextfrom
feature/console-usb-printf-debug-flags
Draft

feat: surface console printf; unify console + sensor command transports#97
boringethan wants to merge 4 commits into
nextfrom
feature/console-usb-printf-debug-flags

Conversation

@boringethan

@boringethan boringethan commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Refs #122.

What

Host-side support for console firmware printf-over-USB, plus a transport cleanup: the console and sensor command links now share one async engine.

Commits

  • feat: surface console firmware printf over USB CDC — new omotion/framing.py (shared frame extraction + log-packet demux); MotionConsole.set_debug_flags / get_debug_flags / enable_usb_printf; firmware printf packets (OW_DATA/OW_CMD_ECHO/id=0) are demuxed from responses and surfaced via the logger.
  • refactor: unify console + sensor command transports — extract the sensor's proven async engine into omotion/packet_transport.py::PacketTransport (reader thread + frame dispatch + id-matched send + transport-down cancellation). CommInterface (USB bulk) and MotionUart (serial VCP) both subclass it; the console gains a continuous reader so printf packets never collide with responses. Net −228 lines.
  • refactor: drop CommInterface sync mode — async-only; the sync path was dead in production.
  • refactor: drop console command pacing — the console firmware now re-arms its CDC RX up front, so the host no longer needs the inter-command pace.

Compatibility

This SDK assumes console firmware with the CDC-RX re-arm fix (OpenwaterHealth/openmotion-console-fw#39). Against older console firmware, rapid back-to-back console commands could be dropped.

Testing

  • Pure-software suite green (485 passed; 3 pre-existing unrelated ScanDB-sink failures). CommInterface transport-down contract preserved.
  • Hardware: console printf surfaces to host; 40 rapid back-to-back pings, 0 failures, no pacing; telemetry + both sensors (ping/echo) healthy.

🤖 Generated with Claude Code

boringethan and others added 4 commits June 22, 2026 13:05
@boringethan @claude
feat: surface console firmware printf over USB CDC
ac4cdd3 
The console firmware can now mirror printf() to its USB CDC link as
unsolicited OW_DATA/OW_CMD_ECHO (id=0) packets (DEBUG_FLAG_USB_PRINTF).
Those packets share the link with command responses, so the host must
demux them or the synchronous console reader would mistake a log packet
for a response.

- Add omotion/framing.py: shared frame extraction (extract_frame) and
  log-packet demux (is_log_packet / emit_log_packet) — the one place both
  transports parse the wire protocol and surface firmware printf lines.
- Rework MotionUart.read_packet to reassemble frames and skip/surface log
  packets, keeping a persistent RX buffer so fragments and trailing bytes
  aren't lost; clear_buffer drops it too.
- Refactor CommInterface._process_responses onto the shared helpers
  (behavior-preserving; removes the duplicated demux and dead constants).
- Add MotionConsole.set_debug_flags / get_debug_flags / enable_usb_printf,
  mirroring MotionSensor.
- Add pure-software tests for framing and the console printf demux.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@boringethan @claude
refactor: unify console + sensor command transports on one async engine
36607da 
The console's serial link (MotionUart) was synchronous — it only read the
port while a command was in flight. That can't tolerate the unsolicited
firmware printf packets this branch added: they collided with command
responses, got purged by clear_buffer, and (with the telemetry poller as a
second caller) raced into 20s lock-holding stalls.

Extract the sensor's proven async engine into a shared PacketTransport base
(reader thread + frame dispatch via omotion.framing + id-matched send, with
prompt transport-down cancellation). CommInterface (USB bulk) and MotionUart
(serial VCP) now both subclass it; the console gains a continuous reader, so
printf packets are surfaced by the dispatcher and never collide with
responses. Net -228 lines, and the framing/demux logic lives in one place.

Console-only: pace commands >=15ms apart. The console firmware re-arms its
CDC OUT reception only after sending each response, so a command arriving in
the ~10ms window before that is silently dropped; the old slow synchronous
reader paced this implicitly. Pacing (vs retrying) avoids re-applying
non-idempotent commands. (The sensor's bulk endpoint has no such window.)

Verified on hardware: console printf surfaces to the host with no stalls,
console comms + telemetry healthy, both sensors ping/echo OK. Pure-software
suite green (CommInterface transport-down contract preserved).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@boringethan @claude
refactor: drop CommInterface sync mode — command transports are async…
d5c002e 
…-only

Both transports run the async PacketTransport engine, so the synchronous
send path (and the async_mode flag) was dead in production: MotionComposite
always built CommInterface async. Remove the flag, the sync send branch, and
receive(); simplify PacketTransport (always one response queue + dispatch
thread) and update MotionComposite and the transport-down test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@boringethan @claude
refactor: drop console command pacing — firmware now re-arms CDC RX u…
eff40c6 
…p front

The console firmware (openmotion-console-fw) now snapshots the received frame
and re-arms CDC reception before processing, so it no longer drops rapid
back-to-back commands. The ~15 ms host pacing workaround is no longer needed:
remove MotionUart's throttle and the PacketTransport _pace_before_send /
_mark_send_done hooks.

NOTE: this SDK now assumes console firmware with the CDC RX re-arm fix; against
older firmware, rapid back-to-back console commands could be dropped.

Verified on hardware: 40 rapid pings, 0 failures, no pacing; printf mirroring
and both sensors healthy.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 2, 2026
Open
Closed
Closed
@boringethan
boringethan marked this pull request as draft July 2, 2026 22:48
This was referenced Jul 27, 2026
Closed
Merged
@boringethan

boringethan commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Heads-up before this rebases — this branch would silently re-land a bug that is being fixed on next right now.

write() is carried through essentially verbatim into the new transport shim, and it reproduces #189 (reported as bloodflow-app#391): usb.util.clear_halt does not exist in pyusb, so the EPIPE recovery has never run in any build since 1.5.0. The buggy lines appear here as unchanged context straddling hunks @@ -232,7 +80,7 @@ and @@ -241,140 +89,3 @@ — the only edit inside write() on this branch is a comment reword (_read_loop → the reader loop), so nothing flags them.

Fix landing on next in PR #190. On rebase: take next's version of the whole if e.errno in (32, -9) block wholesale, then re-apply only the comment reword. The conflict is certain but trivial — #190 makes no structural change to that block. Note the fixed version no longer re-sends the packet (the sensor firmware has no id dedup, so a duplicate double-executes the command); please don't reintroduce the re-send when resolving.

If a later revision of this branch moves the raw write into PacketTransport, the fix has to follow it — grep for clear_halt before merging, and move tests/test_comm_clear_halt.py to whatever class ends up owning write().

Separately and independently of the above: this branch reverts _read_loop's timeout check to a literal e.errno in (110, 10060), dropping the is_usb_timeout helper (omotion/USBInterfaceBase.py:11-22) that also handles macOS errno 60 and the USBTimeoutError subclass. That helper arrived in c429baf after this branch was last updated (2026-07-02), so it looks like staleness rather than intent — worth picking up in the same rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@boringethan