Skip to content

Make the sensor link a shared library both ends can use - #2

Merged
PonderForge merged 4 commits into
mainfrom
feat/shared-packet-library
Sep 26, 2026
Merged

PonderForge merged 4 commits into
mainfrom
feat/shared-packet-library

Conversation

@taciturnaxolotl

Copy link
Copy Markdown
Contributor

Why

The packet the Arduino sends to the Pi was written out twice: once in this repo's receiver, once in the firmware. Nothing stopped the two drifting apart, and a mismatch would not fail a build. The car would just decode garbage, which is the kind of thing you find out at an event.

This turns the receiver into a library the firmware can also use, so there is one definition to edit instead of two.

What changed

  • Parsing is separated from serial I/O. sh_parser_feed() takes one byte and never touches a file descriptor, so framing can be tested on any machine with no car attached. That is what makes the tests worth running in CI at all.
  • The layout doubles as an Arduino library. Sources moved under src/sensorhub/, which arduino-cli puts on the include path, so <sensorhub/sensorhub.h> resolves identically on a Nano and on the Pi. The POSIX serial code compiles away to nothing on AVR, which CI asserts.
  • Stat counter width is configurable. 64-bit on the Pi, 32-bit on AVR. Not 16: at roughly 40 packets a second that wraps in under half an hour, and a diagnostic counter that quietly lies is worse than a larger one.
  • main.c became tools/monitor.c, the bench tool for watching a live link.

Tests

They cover the failure the old 9600-baud reader actually had: one dropped byte desynchronising the stream with nothing to resynchronise against. Also leading garbage, a payload containing the header bytes, corrupt checksums, and split reads.

A golden frame pins the wire format byte for byte. It was verified identical to the firmware's previous hand-rolled encoder across 200,000 random packets before that code was deleted.

There is a CI job that breaks the parser four ways and requires the tests to notice every time. A framing test that cannot fail is worse than no test, because it turns a guess into a green tick.

Other jobs: warnings-as-errors, asan/ubsan, a cross-compile to aarch64 that runs the suite under qemu (an x86 green tick proves nothing about the Pi), an AVR compile, and an add_subdirectory consumer check for how CarComputer will pull this in.

A bug this found

The bench tool could not be interrupted. signal() installs handlers with SA_RESTART, so the blocking read() auto-restarted and the shutdown flag was never checked; Ctrl-C did nothing. Fixed by surfacing EINTR from the library rather than retrying it internally, so the caller decides. The pty-based end-to-end test that caught it is included.

Order

Merge this first. HEEV/SensorController and HEEV/CarDisplay both have PRs that consume this library from main, so their CI stays red until this lands.

The packet format was written out twice, once in the Pi's receiver and
once in the Arduino firmware, with nothing stopping the two from drifting
apart. A mismatch would not fail a build; the car would just decode
garbage, which is the kind of thing you discover at an event.

This turns the receiver into a library that the firmware can also use, so
there is one definition to edit instead of two. Parsing is kept separate
from serial I/O so the framing can be tested on any machine with no car
attached, which is what makes the tests worth running.

The tests cover the failure the old 9600-baud reader actually had: one
dropped byte desynchronising the stream with nothing to resynchronise
against. A golden frame pins the wire format byte for byte, verified
identical to the firmware's previous hand-rolled encoder across 200,000
random packets.

CI includes a job that breaks the parser four ways and requires the tests
to notice every time, because a framing test that cannot fail is worse
than no test: it turns a guess into a green tick.
A same-size packet with a field that changed meaning passes the checksum and
decodes as confident nonsense. That is not hypothetical: the Python server's
channel names silently changed in April and nobody noticed for months.

The frame now carries a format version and a payload length. The version
turns a silent misdecode into a loud refusal. The length lets an old receiver
skip a packet from a newer sender and stay framed instead of desynchronising.
Two bytes on a link running at 7% utilisation.

The payload gains room so the next sensor is a wiring job rather than a
format change that invalidates every CSV on disk: four temperature slots
instead of two named floats, four analog slots instead of one, and digital
channels as bits, which costs less than the bytes they replace. Output
channels are now reported too. The firmware drives a radiator fan and a water
pump whose state appeared nowhere in telemetry, so there was no way to see
what the car was doing to itself.

A sequence number makes dropped packets visible. A checksum cannot tell you
about a packet that never arrived; a link losing half its traffic looked
exactly like a healthy one.

The API reports errors instead of returning a bare bool: a status enum with a
message for every value, bounds-checked channel accessors, and an encoder
that takes its buffer size and refuses rather than overruns.
@taciturnaxolotl

Copy link
Copy Markdown
Contributor Author

Updated: the wire format now carries a version and a length byte from the start, rather than being retrofitted later.

The version byte exists for one specific failure. If the payload changes size, framing desyncs and you notice. But if it stays the same size and a field changes meaning, the checksum still passes and the receiver decodes confidently wrong data. That is exactly what happened on the Python side in April, when columns 5 and 6 silently went from Speed/button to voltage/timer_reset_button and nobody caught it for months.

The length byte lets an old receiver skip a packet from a newer sender and stay framed instead of desynchronising.

Payload also widened while it was cheap to do so, since breaking the format twice is much worse than once:

before now
temperatures 2 named floats 4 slots
analog 1 4 slots
digital 5 bytes, inputs only 8 inputs + 8 outputs, as bits
sequence none uint16_t

Outputs are reported now. The firmware drives a radiator fan and a water pump whose state appeared nowhere in telemetry, so there was no way to see or log what the car was doing to itself.

The sequence number makes dropped packets visible. A checksum cannot tell you about a packet that never arrived, so a link losing half its traffic looked identical to a healthy one.

Costs: frame 26 to 41 bytes, which takes the link from 4.5% to 7% utilisation, and the sketch from 18% to 19% of a Nano.

An XOR sum misses two corruptions a car produces for real, and misses them
completely rather than occasionally. Measured over two million corrupted
frames: two bit flips in the same bit position were 100% undetected, because
they cancel exactly, and two swapped bytes were 100% undetected, because an
order-independent sum cannot see order. The first is what a ground bounce or
a supply glitch on one data line does, which is to say what an ignition
system does.

CRC-16-CCITT catches both, plus every burst up to sixteen bits, where the XOR
sum let 0.39% through. It costs 32 bytes of flash on an ATmega328p and one
byte on a wire running at 7% utilisation.

Both failure modes now have tests that try every paired flip and every byte
swap in the frame and require zero to be accepted. Reverting the CRC to an
order-independent sum lets 280 and 397 of them through respectively, so the
tests fail loudly rather than decoratively.

Done before anything is deployed, so format 1 simply means this from the
start rather than needing a version bump and a reflash later.
@PonderForge

Copy link
Copy Markdown
Member

This looks good, though there's really no point of a sequencer either, since we wanna be like UDP and yeet the packets to the PI. If anything, maybe have like a timer that will trigger a warning if there hasn't been a valid packet in a while.

@PonderForge
PonderForge merged commit a2aee2a into main Sep 26, 2026
10 checks passed
@taciturnaxolotl
taciturnaxolotl deleted the feat/shared-packet-library branch September 26, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants