diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..1380795 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,242 @@ +name: CI + +on: + push: + branches: [ main ] + pull_request: + workflow_dispatch: + +jobs: + test: + name: test / ${{ matrix.os }} / ${{ matrix.cc }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + include: + - { os: ubuntu-latest, cc: gcc } + - { os: ubuntu-latest, cc: clang } + - { os: macos-latest, cc: clang } + steps: + - uses: actions/checkout@v4 + - name: Configure + run: cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=${{ matrix.cc }} + - name: Build + run: cmake --build build -j + - name: Test + run: ctest --test-dir build --output-on-failure + + - name: End-to-end against a fake Arduino on a pty + # covers what the unit tests cannot: termios setup, blocking reads, + # and signal handling. it caught a monitor that could not be Ctrl-C'd. + run: python3 test/test_serial_pty.py build/sensorhub-monitor + + warnings: + name: -Wall -Wextra -Werror + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + cc: [ gcc, clang ] + steps: + - uses: actions/checkout@v4 + - name: Configure + run: | + cmake -S . -B build -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_C_COMPILER=${{ matrix.cc }} \ + -DCMAKE_C_FLAGS="-Wall -Wextra -Werror" + - name: Build + run: cmake --build build -j + + sanitizers: + name: asan + ubsan + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Configure + run: | + cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug \ + -DCMAKE_C_FLAGS="-fsanitize=address,undefined -fno-sanitize-recover=all -g" + - name: Build + run: cmake --build build -j + - name: Test + run: ctest --test-dir build --output-on-failure + + cross-aarch64: + # this library's whole job is to run on the Pi 5, so prove it cross-compiles + # for one before trusting an x86 green tick + name: cross-compile for the Pi + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install the cross toolchain + run: | + sudo apt-get update + sudo apt-get install -y gcc-aarch64-linux-gnu qemu-user + - name: Configure + run: | + cmake -S . -B build -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_SYSTEM_NAME=Linux \ + -DCMAKE_SYSTEM_PROCESSOR=aarch64 \ + -DCMAKE_C_COMPILER=aarch64-linux-gnu-gcc \ + -DCMAKE_C_FLAGS="-Wall -Wextra -Werror" + - name: Build + run: cmake --build build -j + - name: Run the tests under emulation + run: qemu-aarch64 -L /usr/aarch64-linux-gnu ./build/test_parser + + mutation: + # a framing test that cannot fail is worse than no test, because it + # launders a guess into a green tick. break the parser four ways and + # require the suite to notice every time. + name: the tests must be able to fail + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Build and confirm the suite passes clean + run: | + cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug + cmake --build build -j + ./build/test_parser + - name: Each mutation must be caught + run: | + set -u + cp src/sensorhub/parser.c /tmp/parser.orig + fail=0 + check () { + cmake --build build -j >/dev/null 2>&1 + if ./build/test_parser >/dev/null 2>&1; then + echo "NOT CAUGHT: $1" + fail=1 + else + echo "caught: $1" + fi + cp /tmp/parser.orig src/sensorhub/parser.c + } + sed -i 's/if (parser->format != SH_FORMAT_CURRENT) {/if (0) {/' src/sensorhub/parser.c + check "accepted any format version" + sed -i 's/parser->stats.dropped += gap;/;/' src/sensorhub/parser.c + check "stopped counting dropped packets" + sed -i 's/gap < 1000u/gap < 65535u/' src/sensorhub/parser.c + check "treated a sender restart as a flood of drops" + sed -i 's/sh_crc16(buffer + 2, SH_PAYLOAD_SIZE + 2u)/sh_crc16(buffer + 4, SH_PAYLOAD_SIZE)/' src/sensorhub/parser.c + check "covered the payload only, not fmt and len" + sed -i 's|crc ^= (uint16_t)((uint16_t)data\[i\] << 8);|crc ^= (uint16_t)data[i]; if(0)|' src/sensorhub/parser.c + check "weakened the CRC into an order-independent sum" + sed -i 's/parser->stats.checksum_errors++;/;/' src/sensorhub/parser.c + check "dropped checksum-error counting" + sed -i 's/else if (byte == SH_HEADER_1) {/else if (0) {/' src/sensorhub/parser.c + check "broke repeated-header handling" + sed -i 's/if (channel > SH_DIGITAL_MAX) return SH_E_RANGE;/;/' src/sensorhub/parser.c + check "dropped the digital channel bounds check" + sed -i 's/if (buffer_size < SH_FRAME_SIZE) return SH_E_SPACE;/;/' src/sensorhub/parser.c + check "dropped the encode buffer size check" + cmake --build build -j >/dev/null 2>&1 + exit $fail + + consumer: + # what pulling this into CarComputer actually looks like + name: add_subdirectory + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + path: SensorHub + - name: Write a consumer that uses only the public header + run: | + cat > CMakeLists.txt <<'CMAKE' + cmake_minimum_required(VERSION 3.16) + project(consumer LANGUAGES C) + add_subdirectory(SensorHub) + add_executable(consumer main.c) + target_link_libraries(consumer PRIVATE sensorhub) + CMAKE + cat > main.c <<'C' + #include + #include + #include + int main (void) { + sh_parser_t parser; + sh_packet_t in, out; + uint8_t frame[SH_FRAME_SIZE]; + memset(&in, 0, sizeof(in)); + in.speed = 12.5f; + in.temps[SH_TEMP_ENGINE] = 190.0f; + sh_set_digital_in(&in, 2, true); + if (sh_encode_frame(&in, frame, sizeof(frame), NULL) != SH_OK) + return 1; + sh_parser_init(&parser); + for (size_t i = 0; i < sizeof(frame); ++i) { + if (sh_parser_feed(&parser, frame[i], &out) == SH_OK) { + bool ch2 = false; + sh_digital_in(&out, 2, &ch2); + printf("speed=%.1f ch2=%d\n", (double) out.speed, (int) ch2); + return (out.speed == 12.5f && ch2) ? 0 : 1; + } + } + return 1; + } + C + - name: Build and run + run: | + cmake -S . -B build + cmake --build build -j + ./build/consumer + - name: Private sources must not be reachable + run: | + printf '#include "parser.c"\nint main(){}\n' > bad.c + if cc -std=c11 -ISensorHub/src -c bad.c -o /dev/null 2>/dev/null; then + echo "src/ is reachable from a consumer's include path" + exit 1 + fi + echo "src/ correctly private" + + arduino: + # this header also ships as an Arduino library and compiles for the + # ATmega328p, where int is 16 bits. the firmware depends on that, so + # prove it here rather than finding out at flash time. + name: compiles for the Nano + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install avr-gcc + run: | + sudo apt-get update + sudo apt-get install -y gcc-avr avr-libc binutils-avr + - name: Header and encoder must build for AVR + run: | + cat > /tmp/avr_probe.c <<'C' + #include + /* int is 16 bits here; the packed fixed-width layout must survive */ + _Static_assert(sizeof(sh_packet_t) == 36, "struct differs on AVR"); + _Static_assert(SH_FRAME_SIZE == 42, "frame size drifted"); + _Static_assert(sizeof(float) == 4, "float differs on AVR"); + volatile unsigned char sink; + int main(void) { + sh_packet_t p = {0}; + uint8_t f[SH_FRAME_SIZE]; + size_t n = 0; + sh_set_digital_out(&p, 1, true); + if (sh_encode_frame(&p, f, sizeof(f), &n) != SH_OK) return 1; + sink = f[n - 1]; + return 0; + } + C + avr-gcc -mmcu=atmega328p -Os -std=c11 -Wall -Wextra -Werror \ + -ffunction-sections -fdata-sections -Wl,--gc-sections \ + -Isrc /tmp/avr_probe.c src/sensorhub/parser.c -o /tmp/avr_probe.elf + avr-size /tmp/avr_probe.elf + + - name: It must also build as C++, since .ino files are C++ + run: | + printf '#include \nstatic_assert(sizeof(sh_packet_t)==36,"");\nint main(){return 0;}\n' > /tmp/probe.cpp + avr-g++ -mmcu=atmega328p -Os -std=gnu++17 -Wall -Wextra \ + -Isrc -c /tmp/probe.cpp -o /tmp/probe.o + + - name: The POSIX transport must compile away to nothing on AVR + run: | + avr-gcc -mmcu=atmega328p -Os -std=c11 -Isrc \ + -c src/sensorhub/serial.c -o /tmp/serial_avr.o + size=$(avr-size /tmp/serial_avr.o | awk 'NR==2{print $1+$2+$3}') + echo "serial.c contributes $size bytes on AVR" + test "$size" = "0" diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..6a23c67 --- /dev/null +++ b/.gitignore @@ -0,0 +1,9 @@ +# CMake build output +build/ + +# Arduino build output +*.elf +*.hex + +# macOS +.DS_Store diff --git a/CMakeLists.txt b/CMakeLists.txt new file mode 100644 index 0000000..0332cdd --- /dev/null +++ b/CMakeLists.txt @@ -0,0 +1,56 @@ +cmake_minimum_required(VERSION 3.16) +project(sensorhub LANGUAGES C) + +# Layout note: sources live under src/sensorhub/ rather than the usual +# include/ + src/ split so that this directory is simultaneously a valid +# Arduino library. arduino-cli puts /src on the include path and +# compiles everything beneath it, which makes resolve +# identically on the Nano and on the Pi. src/sensorhub/serial.c is guarded so +# it compiles away to nothing on AVR. +add_library(sensorhub STATIC + src/sensorhub/parser.c +) + +target_include_directories(sensorhub + PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/src +) +target_compile_features(sensorhub PUBLIC c_std_11) + +# The POSIX serial transport. Guarded internally too, but there is no reason +# to hand it to a non-Unix build at all. +if(UNIX) + target_sources(sensorhub PRIVATE src/sensorhub/serial.c) +endif() + +if(CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR) + set(SH_TOP_LEVEL ON) +else() + set(SH_TOP_LEVEL OFF) +endif() + +option(SH_BUILD_MONITOR "Build the sensorhub-monitor bench tool" ${SH_TOP_LEVEL}) +option(SH_BUILD_TESTS "Build the framing tests" ${SH_TOP_LEVEL}) + +if(SH_BUILD_MONITOR AND UNIX) + add_executable(sensorhub-monitor tools/monitor.c) + target_link_libraries(sensorhub-monitor PRIVATE sensorhub) +endif() + +if(SH_BUILD_TESTS) + enable_testing() + add_executable(test_parser test/test_parser.c) + target_link_libraries(test_parser PRIVATE sensorhub) + add_test(NAME parser COMMAND test_parser) + + # The counter width is configurable for AVR's sake; build the tests + # against the narrow setting too so that path cannot rot unnoticed. + # + # This compiles parser.c into the test rather than linking libsensorhub, + # deliberately: SH_COUNTER_BITS changes sizeof(sh_stats_t), so a test + # defining 16 while the library was built with 64 would disagree about + # the layout of every sh_parser_t it passed across that boundary. + add_executable(test_parser_16 test/test_parser.c src/sensorhub/parser.c) + target_include_directories(test_parser_16 PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src) + target_compile_definitions(test_parser_16 PRIVATE SH_COUNTER_BITS=16) + add_test(NAME parser_narrow_counters COMMAND test_parser_16) +endif() diff --git a/README.md b/README.md index 07c1201..80afd4c 100644 --- a/README.md +++ b/README.md @@ -1 +1,366 @@ # SensorHub + +The wire format for the link between the car's Arduino Nano and the Raspberry +Pi, used by **both** ends so the two cannot drift apart. + +The Arduino sends, the Pi receives, and there is exactly one definition of the +packet, the checksum, and the frame encoder. + +## The wire format + +Forty-two bytes per frame, 115200 baud, 8N1: + +``` +0xAA 0x55 fmt len payload[len] crc16_lo crc16_hi +``` + +| field | size | purpose | +|---|---|---| +| `0xAA 0x55` | 2 | header, for resynchronising | +| `fmt` | 1 | format version, currently `0x01` | +| `len` | 1 | payload length, currently 36 | +| payload | `len` | `sh_packet_t` | +| `crc16` | 2 | CRC-16-CCITT over `fmt`, `len`, payload; little-endian | + +**The version and length bytes earn their keep.** Without a version, changing +what a field *means* while keeping the packet the same size still passes the +checksum, and the receiver decodes confidently wrong data forever. That is not +hypothetical: the old Python server's channel names silently changed in April +and nobody noticed for months. Without a length, an old receiver facing a +newer sender desynchronises instead of skipping one packet and carrying on. + +Two bytes on a link running at 7% utilisation is a good trade. + +The CRC deliberately covers `fmt` and `len` as well as the payload, so a +corrupted length byte cannot quietly reframe the stream and still validate. + +### Why a CRC and not a byte sum + +An XOR checksum is one byte cheaper and misses two corruptions a car +produces for real. Measured over two million corrupted frames: + +| corruption | XOR-8 undetected | CRC-16 undetected | +|---|---|---| +| single bit flip | 0% | 0% | +| **two flips, same bit position** | **100%** | 0% | +| 8-bit burst | 0% | 0% | +| 16-bit burst | 0.39% | 0% | +| **two bytes swapped** | **100%** | 0% | + +Those two 100% rows are structural, not unlucky: paired flips in the same bit +position cancel exactly under XOR, and an order-independent sum cannot see a +swap. The first is what a ground bounce or a supply glitch on one data line +produces, which is to say what an ignition system produces. + +CRC-16-CCITT costs 32 bytes of flash on an ATmega328p and one byte on a wire +running at 7% utilisation. The tests assert all three cases directly. + +### Payload, format 1 + +`sh_packet_t`, a packed struct of fixed-width types, 36 bytes on the Pi and on +an ATmega328p alike, asserted at compile time on both. + +| offset | type | field | notes | +|---|---|---|---| +| 0 | `float` | `speed` | mph, from the wheel interrupt | +| 4 | `float` | `airspeed` | mph, pitot, zeroed at Arduino startup | +| 8 | `float[4]` | `temps` | °F. `[0]` engine, `[1]` radiator, 2 spare | +| 24 | `uint16_t[4]` | `analog` | raw ADC counts. `[0]` battery, 3 spare | +| 32 | `uint8_t` | `digital_in` | bitfield, input channels 0–7 | +| 33 | `uint8_t` | `digital_out` | bitfield, output channels 0–7 | +| 34 | `uint16_t` | `sequence` | wraps; gaps mean dropped packets | + +Three things worth noting about that layout: + +- **Spare slots are deliberate.** Adding a fourth temperature probe to the + DS18B20 bus should be a wiring job, not a format change that invalidates + every CSV on disk. +- **Digital channels are bits.** A switch carries one bit, and sixteen of them + fit where two bytes used to go. +- **Outputs are reported, not just inputs.** The firmware drives a radiator + fan and a water pump, and until now their state appeared nowhere in + telemetry, so there was no way to see or log what the car was doing to + itself. + +Both ends are little-endian. A big-endian host is rejected with an `#error` +rather than quietly decoding nonsense. + +## Sending, on the Arduino + +```sh +arduino-cli lib install --git-url https://github.com/HEEV/SensorHub +``` + +```c +#include + +static uint16_t sequence = 0; + +void sendPacket(sh_packet_t &packet) { + uint8_t frame[SH_FRAME_SIZE]; + size_t written = 0; + + packet.sequence = sequence++; + + if (sh_encode_frame(&packet, frame, sizeof(frame), &written) != SH_OK) { + return; /* refuses rather than writing a half-built frame */ + } + Serial.write(frame, written); +} +``` + +Build the packet with the accessors rather than poking bits by hand, which is +how off-by-one channel bugs happen: + +```c +packet.temps[SH_TEMP_ENGINE] = engineTempF; +packet.analog[SH_ANALOG_BATTERY] = analogRead(A7); +sh_set_digital_in(&packet, 0, digitalRead(4)); +sh_set_digital_out(&packet, 0, digitalRead(RAD_FAN_PIN)); +``` + +Remember to increment `sequence` every packet. It is the only way the receiver +can tell that a packet never arrived; a checksum cannot. + +Note the include is ``, not the nested path. Arduino resolves +libraries by top-level header name, so that shim is what makes the library +discoverable from a sketch. Everywhere else, use ``. + +## Receiving, on the Pi + +```c +#include +#include + +sh_parser_t parser; +sh_packet_t packet; + +int fd = sh_serial_open("/dev/ttyUSB0"); /* NULL for the default port */ +if (fd < 0) { perror("open"); return 1; } + +sh_parser_init(&parser); + +while (sh_serial_read_packet(fd, &parser, &packet) == SH_OK) { + printf("#%u %.2f mph\n", packet.sequence, (double) packet.speed); +} + +sh_serial_close(fd); +``` + +`sh_serial_read_packet` keeps reading through recoverable framing errors. A +bad checksum or an unknown format is counted in `parser.stats` and the read +continues, because one corrupt packet is not a reason to make every caller +write a retry loop. Only a completed packet or an I/O condition ends the call. + +`sh_serial_open()` sets 115200 8N1 raw and, importantly, **clears `HUPCL` and +never touches DTR or RTS**. A Nano reboots when DTR is asserted, which costs +a couple of seconds of telemetry and re-runs the airspeed zeroing with the car +possibly moving. Open the port once and keep it open. + +Prefer the stable device path over `/dev/ttyUSB0`, which renumbers when a +second serial device is present: + +```c +sh_serial_open(SH_DEFAULT_PORT_BY_ID); +``` + +## Parsing without a serial port + +The parser never touches a file descriptor. Feed it bytes from wherever you +got them: a socket, a file, a test fixture, a non-blocking read of your own. + +```c +sh_status_t sh_parser_feed(sh_parser_t *parser, uint8_t byte, sh_packet_t *out); +``` + +| returns | meaning | +|---|---| +| `SH_OK` | `*out` holds a packet | +| `SH_INCOMPLETE` | byte consumed, nothing complete yet. The usual answer | +| `SH_E_CHECKSUM` | a frame arrived intact but its contents were rejected | +| `SH_E_FORMAT` | a version we do not know; skipped cleanly using `len` | +| `SH_E_LENGTH` | our version, an impossible length | + +Errors are reported, not thrown. The parser stays usable and keeps counting, +so a caller that only wants packets can compare against `SH_OK` and read +`parser.stats` occasionally. + +This is what makes the library testable with no car attached, and it is how +CarDisplay drains a socket without blocking its UI thread: + +```c +uint8_t buf[256]; +ssize_t n; + +while ((n = read(fd, buf, sizeof(buf))) > 0) { /* O_NONBLOCK */ + for (ssize_t i = 0; i < n; ++i) { + if (sh_parser_feed(&parser, buf[i], &packet) == SH_OK) { + apply(&packet); /* keep the newest; a backlog means you fell behind */ + } + } +} +``` + +## Link health + +The parser keeps counters. A link that is losing packets shows up here long +before it shows up on the dashboard. + +```c +printf("ok=%llu bad=%llu resync=%llu\n", + (unsigned long long) parser.stats.packets, + (unsigned long long) parser.stats.checksum_errors, + (unsigned long long) parser.stats.resyncs); +``` + +| counter | meaning | what it usually indicates | +|---|---|---| +| `packets` | accepted | — | +| `checksum_errors` | framed correctly, contents rejected | electrical: noise, a marginal cable, a bad ground | +| `resyncs` | fell back to hunting for a header | the sender is being interrupted mid-frame | +| `format_errors` | a version we do not understand | firmware and receiver are out of step | +| `dropped` | inferred from gaps in `sequence` | packets never arrived; the receiver is not keeping up, or the sender restarted | + +`dropped` is the one a checksum cannot give you. A packet that never arrives +leaves no trace at all, so without a sequence number a link losing half its +traffic looks identical to a healthy one. + +The counter deliberately does not report a flood when the sequence wraps at +65535, nor when the Arduino restarts and begins again at zero. + +Counter width is `uint64_t` on a host and `uint32_t` on AVR. Not 16-bit: 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. Override with +`-DSH_COUNTER_BITS=16|32|64` if you must, but note it changes +`sizeof(sh_stats_t)`, so everything linked together has to agree. + +## Recovering from a bad byte + +This is the failure the old 9600-baud reader actually had: one dropped byte +desynchronised the stream and there was nothing to resynchronise against. + +The `0xAA 0x55` header fixes that, and the parser handles the subtle case too. +A payload byte that happens to be `0xAA` sitting immediately before a real +header does not break framing, because the state machine holds its candidate +rather than discarding it. A truncated frame is dropped and the next one +parses. + +You do not have to do anything to get this. It is just worth knowing it is +there, and there are tests for each case. + +## Building + +CMake, as a subproject: + +```cmake +add_subdirectory(SensorHub) +target_link_libraries(your_app PRIVATE sensorhub) +``` + +Standalone, with the bench tool and tests: + +```sh +cmake -S . -B build && cmake --build build -j +ctest --test-dir build +``` + +Options: `SH_BUILD_MONITOR` and `SH_BUILD_TESTS`, both on when this is the +top-level project and off when it is a subproject. + +Layout note: sources live under `src/sensorhub/` rather than the usual +`include/` and `src/` split, so that this directory is simultaneously a valid +Arduino library. `arduino-cli` puts `/src` on the include path, which +makes `` resolve identically on both targets. +`src/sensorhub/serial.c` is guarded so it compiles to zero bytes on AVR. + +## The bench tool + +```sh +./build/sensorhub-monitor [device] +``` + +Prints live packets and running counters. This is how you find out whether a +sensor is actually wired without involving the display or anything else: + +``` +speed=23.50 mph | air=19.25 | engine=180.0F | rad=148.5F | ch=10110 | A0=812 | ok=2 bad=1 resync=0 +``` + +Ctrl-C works, which is less obvious than it sounds. `signal()` installs +handlers with `SA_RESTART` on most platforms, so a blocking `read()` +auto-restarts and never returns, making the tool impossible to interrupt. The +library surfaces `EINTR` rather than retrying it internally so the caller can +check its own shutdown flag: + +```c +if (!sh_serial_read_packet(fd, &parser, &packet)) { + if (errno == EINTR) continue; /* a signal: check your stop flag */ + if (errno == 0) break; /* clean EOF: the adapter was unplugged */ + perror("serial read"); + break; +} +``` + +Install your handlers with `sigaction()` and no `SA_RESTART` if you want this +to work. + +## API summary + +From ``, portable everywhere including AVR: + +```c +/* framing */ +void sh_parser_init (sh_parser_t *parser); +sh_status_t sh_parser_feed (sh_parser_t *parser, uint8_t byte, + sh_packet_t *out); +uint16_t sh_crc16 (const uint8_t *data, size_t length); +uint16_t sh_crc16_continue(uint16_t crc, const uint8_t *data, size_t n); +sh_status_t sh_encode_frame (const sh_packet_t *packet, uint8_t *buffer, + size_t buffer_size, size_t *written); + +/* channels, all bounds-checked */ +sh_status_t sh_digital_in (const sh_packet_t *p, unsigned ch, bool *out); +sh_status_t sh_digital_out (const sh_packet_t *p, unsigned ch, bool *out); +sh_status_t sh_set_digital_in (sh_packet_t *p, unsigned ch, bool value); +sh_status_t sh_set_digital_out (sh_packet_t *p, unsigned ch, bool value); +sh_status_t sh_temp (const sh_packet_t *p, unsigned i, float *out); +sh_status_t sh_analog (const sh_packet_t *p, unsigned i, uint16_t *out); + +/* status */ +const char *sh_strstatus (sh_status_t status); /* never NULL */ +bool sh_failed (sh_status_t status); /* SH_INCOMPLETE is not a failure */ +``` + +From ``, POSIX only: + +```c +int sh_serial_open (const char *device); +void sh_serial_close (int fd); +sh_status_t sh_serial_read_packet(int fd, sh_parser_t *parser, + sh_packet_t *out); +``` + +Conventions worth knowing: + +- **`SH_OK` is zero**, so `if (call(...) == SH_OK)` reads naturally and a + status can be tested for truth if you prefer. +- **`SH_INCOMPLETE` is not an error.** It is the answer for every byte that + did not happen to finish a packet, which is most of them. Use `sh_failed()` + rather than `!= SH_OK` when deciding whether to log something. +- **Every function tolerates `NULL`** by returning `SH_E_NULL` rather than + crashing, and every index is bounds-checked into `SH_E_RANGE`. +- **`sh_encode_frame` takes the buffer size** and refuses rather than + overrunning. +- **Every status has a message.** `sh_strstatus()` never returns `NULL`, even + for a value that is not a real status. + +## Changing the wire format + +Don't, casually. The format is frozen by a golden-frame test that compares +`sh_encode_frame()` output byte for byte against a known-good frame, verified +identical to the firmware's original hand-rolled encoder across 200,000 random +packets. + +If that test fails, it is not a test to update. It means every Arduino in the +fleet needs reflashing and every consumer needs checking, including the CSVs +already on disk. Change it deliberately, in one commit, on both ends. diff --git a/library.properties b/library.properties new file mode 100644 index 0000000..de505ff --- /dev/null +++ b/library.properties @@ -0,0 +1,10 @@ +name=SensorHub +version=0.1.0 +author=Cedarville Supermileage (HEEV) +maintainer=Cedarville Supermileage (HEEV) +sentence=Shared wire format for the car's sensor link. +paragraph=Defines the packet the Arduino Nano sends to the Raspberry Pi, along with the checksum, the frame encoder, and the receiving state machine. Used on both ends so the two cannot drift apart. On AVR only the encoder is normally linked; the parser is there for when the Pi starts commanding the output channels. +category=Communication +url=https://github.com/HEEV/SensorHub +architectures=* +includes=sensorhub/sensorhub.h diff --git a/main.c b/main.c deleted file mode 100644 index fc5dd4d..0000000 --- a/main.c +++ /dev/null @@ -1,285 +0,0 @@ -// sm_serial.c -#define _DEFAULT_SOURCE - -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#define HEADER_1 0xAAU -#define HEADER_2 0x55U -#define SERIAL_PORT "/dev/ttyUSB0" -#define BAUD_RATE B115200 -#define BAUD_TEXT "115200" - -typedef struct __attribute__((packed)) { - float speed; - float airspeed; - float engineTemp; - float radTemp; - - uint8_t channel0; - uint8_t channel1; - uint8_t channel2; - uint8_t channel3; - uint8_t channel4; - - uint16_t channelA0; -} DataPacket; - -_Static_assert(sizeof(float) == 4, "32-bit float required"); -_Static_assert(sizeof(DataPacket) == 23, "Unexpected packet size"); - -typedef enum { - STATE_WAIT_HEADER_1, - STATE_WAIT_HEADER_2, - STATE_READ_PAYLOAD, - STATE_READ_CHECKSUM -} ReceiverState; - -typedef struct { - ReceiverState state; - uint8_t payload[sizeof(DataPacket)]; - size_t payloadIndex; -} PacketReceiver; - -static uint8_t packet_checksum( - const uint8_t *data, - size_t length) -{ - uint8_t checksum = 0; - - for (size_t i = 0; i < length; ++i) { - checksum ^= data[i]; - } - - return checksum; -} - -static bool configure_serial(int fd, speed_t baud_rate) -{ - struct termios tty; - - if (tcgetattr(fd, &tty) != 0) { - perror("tcgetattr"); - return false; - } - - cfmakeraw(&tty); - - if (cfsetispeed(&tty, baud_rate) != 0 || - cfsetospeed(&tty, baud_rate) != 0) { - perror("setting baud rate"); - return false; - } - - tty.c_cflag &= (tcflag_t)~CSIZE; - tty.c_cflag |= CS8; - tty.c_cflag |= CLOCAL | CREAD; - - tty.c_cflag &= (tcflag_t)~PARENB; - tty.c_cflag &= (tcflag_t)~PARODD; - tty.c_cflag &= (tcflag_t)~CSTOPB; - tty.c_cflag &= (tcflag_t)~CRTSCTS; - - /* - * Prevent modem-control lines from being dropped on close. - * This helps avoid repeated Arduino Nano resets. - */ - tty.c_cflag &= (tcflag_t)~HUPCL; - - tty.c_iflag &= (tcflag_t)~IXON; - tty.c_iflag &= (tcflag_t)~IXOFF; - tty.c_iflag &= (tcflag_t)~IXANY; - - /* - * Blocking mode: each read waits for at least one byte. - */ - tty.c_cc[VMIN] = 1; - tty.c_cc[VTIME] = 0; - - if (tcsetattr(fd, TCSANOW, &tty) != 0) { - perror("tcsetattr"); - return false; - } - - /* - * Do not call tcflush() and do not manipulate DTR or RTS. - */ - return true; -} - -static bool read_byte(int fd, uint8_t *value) -{ - ssize_t count; - - if (value == NULL) { - errno = EINVAL; - return false; - } - - do { - count = read(fd, value, 1); - } while (count < 0 && errno == EINTR); - - return count == 1; -} - -static void packet_receiver_init(PacketReceiver *receiver) -{ - if (receiver == NULL) { - return; - } - - receiver->state = STATE_WAIT_HEADER_1; - receiver->payloadIndex = 0; - memset(receiver->payload, 0, sizeof(receiver->payload)); -} - -static bool receive_packet( - PacketReceiver *receiver, - int fd, - DataPacket *packet) -{ - uint8_t received_byte; - - if (receiver == NULL || packet == NULL) { - errno = EINVAL; - return false; - } - - while (read_byte(fd, &received_byte)) { - switch (receiver->state) { - case STATE_WAIT_HEADER_1: - if (received_byte == HEADER_1) { - receiver->state = STATE_WAIT_HEADER_2; - } - break; - - case STATE_WAIT_HEADER_2: - if (received_byte == HEADER_2) { - receiver->payloadIndex = 0; - receiver->state = STATE_READ_PAYLOAD; - } else if (received_byte != HEADER_1) { - receiver->state = STATE_WAIT_HEADER_1; - } - /* - * If another HEADER_1 arrives, remain in this state. - */ - break; - - case STATE_READ_PAYLOAD: - receiver->payload[receiver->payloadIndex++] = - received_byte; - - if (receiver->payloadIndex == sizeof(DataPacket)) { - receiver->state = STATE_READ_CHECKSUM; - } - break; - - case STATE_READ_CHECKSUM: - receiver->state = STATE_WAIT_HEADER_1; - - if (packet_checksum( - receiver->payload, - sizeof(receiver->payload)) == received_byte) { - memcpy( - packet, - receiver->payload, - sizeof(*packet)); - - return true; - } - - fprintf( - stderr, - "Discarded packet: bad checksum\n"); - break; - } - } - - return false; -} - -static void print_packet(const DataPacket *packet) -{ - if (packet == NULL) { - return; - } - - printf( - "\r\x1b[2K" - "speed=%.2f mph" - " | airspeed=%.2f" - " | engine=%.2f F" - " | radiator=%.2f F" - " | channels=%u%u%u%u%u" - " | A0=%u", - (double)packet->speed, - (double)packet->airspeed, - (double)packet->engineTemp, - (double)packet->radTemp, - (unsigned)packet->channel0, - (unsigned)packet->channel1, - (unsigned)packet->channel2, - (unsigned)packet->channel3, - (unsigned)packet->channel4, - (unsigned)packet->channelA0); - - fflush(stdout); -} - -int main(void) -{ - PacketReceiver receiver; - DataPacket packet; - int fd; - - /* - * Open only once. Reopening the port can repeatedly reset an - * Arduino Nano through its DTR auto-reset circuit. - */ - fd = open(SERIAL_PORT, O_RDWR | O_NOCTTY); - - if (fd < 0) { - perror("open"); - return 1; - } - - if (!configure_serial(fd, BAUD_RATE)) { - close(fd); - return 1; - } - - packet_receiver_init(&receiver); - - printf( - "Listening on %s at %s baud\n", - SERIAL_PORT, - BAUD_TEXT); - - while (true) { - memset(&packet, 0, sizeof(packet)); - - if (!receive_packet(&receiver, fd, &packet)) { - if (errno != 0) { - perror("serial read"); - } else { - fprintf(stderr, "Serial connection closed\n"); - } - - break; - } - - print_packet(&packet); - } - - printf("\n"); - close(fd); - return 0; -} \ No newline at end of file diff --git a/src/SensorHub.h b/src/SensorHub.h new file mode 100644 index 0000000..7efdf11 --- /dev/null +++ b/src/SensorHub.h @@ -0,0 +1,20 @@ +/* + * Arduino entry point. + * + * The Arduino library resolver matches an #include against headers at the top + * level of src/, so a sketch cannot reach src/sensorhub/sensorhub.h directly + * even though that directory is on the include path. This header is the + * conventional shim that makes the library discoverable. + * + * In a sketch: #include + * Everywhere else: #include + * + * Both reach the same declarations. + */ + +#ifndef SENSORHUB_ARDUINO_H +#define SENSORHUB_ARDUINO_H + +#include "sensorhub/sensorhub.h" + +#endif /* SENSORHUB_ARDUINO_H */ diff --git a/src/sensorhub/parser.c b/src/sensorhub/parser.c new file mode 100644 index 0000000..5174420 --- /dev/null +++ b/src/sensorhub/parser.c @@ -0,0 +1,293 @@ +/* + * Framing, checksum, and accessors. + * + * No I/O lives here on purpose: everything in this file can be exercised from + * a byte array, which is what makes the tests meaningful with no car attached. + */ + +#include "sensorhub/sensorhub.h" + +#include + +_Static_assert(sizeof(float) == 4, "32-bit float required"); +_Static_assert(sizeof(sh_packet_t) == SH_PAYLOAD_SIZE, + "packet size changed; bump SH_FORMAT_CURRENT and the golden test"); +_Static_assert(SH_PAYLOAD_SIZE <= SH_MAX_PAYLOAD, + "the parser cannot buffer its own format"); + +/* ------------------------------------------------------------------ * + * Status + * ------------------------------------------------------------------ */ + +const char *sh_strstatus(sh_status_t status) +{ + switch (status) { + case SH_OK: return "ok"; + case SH_INCOMPLETE: return "incomplete, need more bytes"; + case SH_E_NULL: return "null argument"; + case SH_E_CHECKSUM: return "bad checksum"; + case SH_E_FORMAT: return "unsupported packet format"; + case SH_E_LENGTH: return "bad payload length"; + case SH_E_RANGE: return "channel index out of range"; + case SH_E_SPACE: return "buffer too small"; + case SH_E_OPEN: return "could not open serial port"; + case SH_E_IO: return "serial read failed"; + case SH_E_INTERRUPTED: return "interrupted by a signal"; + case SH_E_CLOSED: return "serial connection closed"; + } + return "unknown status"; +} + +bool sh_failed(sh_status_t status) +{ + return status != SH_OK && status != SH_INCOMPLETE; +} + +/* ------------------------------------------------------------------ * + * Accessors + * ------------------------------------------------------------------ */ + +#define SH_DIGITAL_MAX 7u + +sh_status_t sh_digital_in(const sh_packet_t *packet, unsigned channel, + bool *out) +{ + if (packet == NULL || out == NULL) return SH_E_NULL; + if (channel > SH_DIGITAL_MAX) return SH_E_RANGE; + + *out = ((packet->digital_in >> channel) & 1u) != 0u; + return SH_OK; +} + +sh_status_t sh_digital_out(const sh_packet_t *packet, unsigned channel, + bool *out) +{ + if (packet == NULL || out == NULL) return SH_E_NULL; + if (channel > SH_DIGITAL_MAX) return SH_E_RANGE; + + *out = ((packet->digital_out >> channel) & 1u) != 0u; + return SH_OK; +} + +sh_status_t sh_set_digital_in(sh_packet_t *packet, unsigned channel, bool value) +{ + if (packet == NULL) return SH_E_NULL; + if (channel > SH_DIGITAL_MAX) return SH_E_RANGE; + + if (value) packet->digital_in |= (uint8_t)(1u << channel); + else packet->digital_in &= (uint8_t)~(1u << channel); + return SH_OK; +} + +sh_status_t sh_set_digital_out(sh_packet_t *packet, unsigned channel, bool value) +{ + if (packet == NULL) return SH_E_NULL; + if (channel > SH_DIGITAL_MAX) return SH_E_RANGE; + + if (value) packet->digital_out |= (uint8_t)(1u << channel); + else packet->digital_out &= (uint8_t)~(1u << channel); + return SH_OK; +} + +sh_status_t sh_temp(const sh_packet_t *packet, unsigned index, float *out) +{ + if (packet == NULL || out == NULL) return SH_E_NULL; + if (index >= SH_TEMP_COUNT) return SH_E_RANGE; + + *out = packet->temps[index]; + return SH_OK; +} + +sh_status_t sh_analog(const sh_packet_t *packet, unsigned index, uint16_t *out) +{ + if (packet == NULL || out == NULL) return SH_E_NULL; + if (index >= SH_ANALOG_COUNT) return SH_E_RANGE; + + *out = packet->analog[index]; + return SH_OK; +} + +/* ------------------------------------------------------------------ * + * Checksum and encoding + * ------------------------------------------------------------------ */ + +uint16_t sh_crc16_continue(uint16_t crc, const uint8_t *data, size_t length) +{ + if (data == NULL) return crc; + + for (size_t i = 0; i < length; ++i) { + crc ^= (uint16_t)((uint16_t)data[i] << 8); + + for (unsigned bit = 0; bit < 8u; ++bit) { + crc = (crc & 0x8000u) ? (uint16_t)((crc << 1) ^ 0x1021u) + : (uint16_t)(crc << 1); + } + } + + return crc; +} + +uint16_t sh_crc16(const uint8_t *data, size_t length) +{ + return sh_crc16_continue(0xFFFFu, data, length); +} + +sh_status_t sh_encode_frame(const sh_packet_t *packet, uint8_t *buffer, + size_t buffer_size, size_t *written) +{ + if (packet == NULL || buffer == NULL) return SH_E_NULL; + if (buffer_size < SH_FRAME_SIZE) return SH_E_SPACE; + + buffer[0] = (uint8_t)SH_HEADER_1; + buffer[1] = (uint8_t)SH_HEADER_2; + buffer[2] = (uint8_t)SH_FORMAT_CURRENT; + buffer[3] = (uint8_t)SH_PAYLOAD_SIZE; + memcpy(buffer + 4, packet, SH_PAYLOAD_SIZE); + + /* The CRC covers fmt and len as well as the payload, so a corrupted + length byte cannot quietly reframe the stream and still validate. */ + { + uint16_t crc = sh_crc16(buffer + 2, SH_PAYLOAD_SIZE + 2u); + buffer[4 + SH_PAYLOAD_SIZE] = (uint8_t)(crc & 0xFFu); + buffer[5 + SH_PAYLOAD_SIZE] = (uint8_t)(crc >> 8); + } + + if (written != NULL) *written = SH_FRAME_SIZE; + return SH_OK; +} + +/* ------------------------------------------------------------------ * + * Parser + * ------------------------------------------------------------------ */ + +void sh_parser_init(sh_parser_t *parser) +{ + if (parser == NULL) return; + + memset(parser, 0, sizeof(*parser)); + parser->state = SH_WAIT_HEADER_1; +} + +/* Count how many packets went missing between two sequence numbers, allowing + for the counter wrapping at 16 bits. */ +static void note_sequence(sh_parser_t *parser, uint16_t sequence) +{ + if (parser->have_sequence) { + uint16_t expected = (uint16_t)(parser->last_sequence + 1u); + uint16_t gap = (uint16_t)(sequence - expected); + + /* A huge gap almost certainly means the sender restarted rather than + that 60000 packets vanished, so do not report a fictional flood. */ + if (gap > 0u && gap < 1000u) { + parser->stats.dropped += gap; + } + } + + parser->have_sequence = true; + parser->last_sequence = sequence; +} + +sh_status_t sh_parser_feed(sh_parser_t *parser, uint8_t byte, sh_packet_t *out) +{ + if (parser == NULL || out == NULL) return SH_E_NULL; + + switch (parser->state) { + case SH_WAIT_HEADER_1: + if (byte == SH_HEADER_1) parser->state = SH_WAIT_HEADER_2; + break; + + case SH_WAIT_HEADER_2: + if (byte == SH_HEADER_2) { + parser->state = SH_READ_FORMAT; + } + else if (byte == SH_HEADER_1) { + /* 0xAA 0xAA 0x55 is a valid start: a payload byte that happens + to be 0xAA can precede the real header, so hold this state + rather than discarding the candidate. */ + } + else { + parser->stats.resyncs++; + parser->state = SH_WAIT_HEADER_1; + } + break; + + case SH_READ_FORMAT: + parser->format = byte; + parser->state = SH_READ_LENGTH; + break; + + case SH_READ_LENGTH: + parser->length = byte; + parser->payload_index = 0; + + if (parser->format != SH_FORMAT_CURRENT) { + /* Unknown version. The length byte is what lets us skip it + cleanly instead of desynchronising, which is the entire + reason it is on the wire. */ + parser->stats.format_errors++; + parser->state = (byte > 0u) ? SH_SKIP_UNKNOWN : SH_WAIT_HEADER_1; + return SH_E_FORMAT; + } + + if (byte != (uint8_t)SH_PAYLOAD_SIZE) { + /* Our own version with the wrong length: corruption, not a new + sender. Skip what it claims and carry on. */ + parser->state = (byte > 0u && byte <= SH_MAX_PAYLOAD) + ? SH_SKIP_UNKNOWN + : SH_WAIT_HEADER_1; + return SH_E_LENGTH; + } + + parser->state = SH_READ_PAYLOAD; + break; + + case SH_SKIP_UNKNOWN: + /* Drain the payload and its checksum without interpreting either. */ + parser->payload_index++; + if (parser->payload_index > (size_t)parser->length) { + parser->state = SH_WAIT_HEADER_1; + } + break; + + case SH_READ_PAYLOAD: + parser->payload[parser->payload_index++] = byte; + if (parser->payload_index == (size_t)parser->length) { + parser->state = SH_READ_CRC_LO; + } + break; + + case SH_READ_CRC_LO: + parser->crc_lo = byte; + parser->state = SH_READ_CRC_HI; + break; + + case SH_READ_CRC_HI: { + uint16_t received = (uint16_t)(((uint16_t)byte << 8) | parser->crc_lo); + uint16_t expected; + uint8_t header[2]; + + parser->state = SH_WAIT_HEADER_1; + + /* Recompute over fmt, len, and the payload, matching the encoder. + Feeding the two header fields through the same routine keeps one + definition of the polynomial rather than two. */ + header[0] = parser->format; + header[1] = parser->length; + expected = sh_crc16_continue(sh_crc16(header, sizeof(header)), + parser->payload, + (size_t)parser->length); + + if (expected == received) { + memcpy(out, parser->payload, SH_PAYLOAD_SIZE); + parser->stats.packets++; + note_sequence(parser, out->sequence); + return SH_OK; + } + + parser->stats.checksum_errors++; + return SH_E_CHECKSUM; + } + } + + return SH_INCOMPLETE; +} diff --git a/src/sensorhub/sensorhub.h b/src/sensorhub/sensorhub.h new file mode 100644 index 0000000..70d8e3c --- /dev/null +++ b/src/sensorhub/sensorhub.h @@ -0,0 +1,309 @@ +/* + * SensorHub: the wire format for the link between the car's Arduino Nano and + * the Raspberry Pi. Used by both ends so the two cannot drift apart. + * + * FRAME + * + * 0xAA 0x55 fmt len payload[len] crc16_lo crc16_hi + * + * fmt format version. A receiver refuses anything it does not know + * rather than decoding it confidently and wrongly. + * len payload length. Lets an old receiver skip a packet from a newer + * sender and stay framed, instead of desynchronising. + * crc16 CRC-16-CCITT over fmt, len, and the payload, little-endian. + * + * The CRC is not a byte-sum. An XOR checksum misses two corruptions that a + * car produces for real: two bit flips in the same bit position cancel + * exactly, and swapped bytes are invisible to an order-independent sum. Both + * were measured at 100% undetected. CRC-16 catches both, plus every burst up + * to 16 bits, for 32 bytes of flash on an ATmega328p. + * + * The version and length bytes are the difference between "we changed the + * format and everything broke loudly" and "we changed the format and the car + * logged plausible nonsense for a season". They cost two bytes on a link + * running at 7% utilisation. + * + * PARSING IS SEPARATE FROM I/O + * + * sh_parser_feed() takes one byte and never touches a file descriptor, so the + * framing can be tested on any machine with no car attached. The serial + * helpers in are a convenience for callers that do have + * hardware. + */ + +#ifndef SENSORHUB_H +#define SENSORHUB_H + +#include +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/* ------------------------------------------------------------------ * + * Framing constants + * ------------------------------------------------------------------ */ + +#define SH_HEADER_1 0xAAu +#define SH_HEADER_2 0x55u + +/* Bump this when the payload layout changes, and reflash everything. */ +#define SH_FORMAT_V1 0x01u +#define SH_FORMAT_CURRENT SH_FORMAT_V1 + +/* How many slots the payload carries. Spares are deliberate: adding the + fifth temperature probe should be a wiring job, not a format change that + invalidates every CSV on disk. */ +#define SH_TEMP_COUNT 4u +#define SH_ANALOG_COUNT 4u + +/* Named indices for the slots that are actually wired today. The rest are + spare; give them names here when they get used. */ +#define SH_TEMP_ENGINE 0u +#define SH_TEMP_RADIATOR 1u +#define SH_ANALOG_BATTERY 0u + +/* ------------------------------------------------------------------ * + * The packet + * ------------------------------------------------------------------ */ + +/* + * Digital channels are bits, not bytes: a switch carries one bit of + * information and sixteen of them fit in the space two bytes used to take. + * + * Outputs are reported as well as inputs. The firmware drives a radiator fan + * and a water pump, and until now their state appeared nowhere in telemetry, + * so you could not display or log what the car was doing to itself. + */ +typedef struct __attribute__((packed)) { + float speed; /* mph, from the wheel interrupt */ + float airspeed; /* mph, pitot, zeroed at startup */ + + float temps[SH_TEMP_COUNT]; /* degrees F, see SH_TEMP_* */ + uint16_t analog[SH_ANALOG_COUNT]; /* raw ADC counts, see SH_ANALOG_* */ + + uint8_t digital_in; /* bitfield, input channels 0..7 */ + uint8_t digital_out; /* bitfield, output channels 0..7 */ + + /* Increments every packet and wraps. The receiver turns gaps in this + into a count of packets that never arrived, which a checksum cannot + tell you: a dropped packet leaves no trace otherwise. */ + uint16_t sequence; +} sh_packet_t; + +#define SH_PAYLOAD_SIZE 36u /* sizeof(sh_packet_t) for SH_FORMAT_V1 */ +#define SH_FRAME_OVERHEAD 6u /* 2 header + fmt + len + 2 CRC */ +#define SH_FRAME_SIZE (SH_PAYLOAD_SIZE + SH_FRAME_OVERHEAD) + +/* The largest payload the parser will buffer. Bigger than V1 on purpose, so + a newer sender's packet can be received and rejected cleanly rather than + overrunning anything. */ +#define SH_MAX_PAYLOAD 64u + +/* Both ends are little-endian, AVR and aarch64. A big-endian host would need + byte swapping nobody has written, so say so rather than decode garbage. */ +#if defined(__BYTE_ORDER__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ +#error "SensorHub assumes a little-endian host; the wire format is AVR-native" +#endif + +/* ------------------------------------------------------------------ * + * Status + * ------------------------------------------------------------------ */ + +/* + * Every call that can fail returns one of these. SH_OK is zero, so the + * common shape reads naturally: + * + * if (sh_parser_feed(&parser, byte, &packet) == SH_OK) { ... } + * + * SH_INCOMPLETE is not an error. It is the usual answer, returned for every + * byte that did not happen to complete a packet. + */ +typedef enum { + SH_OK = 0, /* a packet is ready */ + SH_INCOMPLETE, /* byte consumed, nothing complete yet */ + SH_E_NULL, /* a required argument was NULL */ + SH_E_CHECKSUM, /* framed correctly, CRC rejected it */ + SH_E_FORMAT, /* fmt byte names a version we do not know */ + SH_E_LENGTH, /* len disagrees with fmt, or exceeds the max */ + SH_E_RANGE, /* a channel index was out of bounds */ + SH_E_SPACE, /* caller's buffer was too small */ + SH_E_OPEN, /* could not open the port, see errno */ + SH_E_IO, /* read failed, see errno */ + SH_E_INTERRUPTED,/* a signal arrived; check your stop flag */ + SH_E_CLOSED /* clean EOF; the adapter was unplugged */ +} sh_status_t; + +/* A short human-readable description. Never NULL, even for a bogus value. */ +const char *sh_strstatus(sh_status_t status); + +/* True for statuses that mean something went wrong, so SH_INCOMPLETE does + not get logged as a failure forty times a second. */ +bool sh_failed(sh_status_t status); + + +/* ------------------------------------------------------------------ * + * Digital channel accessors + * ------------------------------------------------------------------ * + * + * Bit twiddling at every call site is how off-by-one channel bugs happen, + * so do it once, here, with bounds checking. + */ + +/* Read one input channel. Returns SH_E_RANGE for channel > 7. */ +sh_status_t sh_digital_in(const sh_packet_t *packet, unsigned channel, + bool *out); + +/* Read one output channel. Returns SH_E_RANGE for channel > 7. */ +sh_status_t sh_digital_out(const sh_packet_t *packet, unsigned channel, + bool *out); + +/* Set an input channel, for senders building a packet. */ +sh_status_t sh_set_digital_in(sh_packet_t *packet, unsigned channel, + bool value); + +/* Set an output channel, for senders building a packet. */ +sh_status_t sh_set_digital_out(sh_packet_t *packet, unsigned channel, + bool value); + +/* Bounds-checked reads for the array slots, so an out-of-range index is an + error rather than whatever was next in memory. */ +sh_status_t sh_temp(const sh_packet_t *packet, unsigned index, float *out); +sh_status_t sh_analog(const sh_packet_t *packet, unsigned index, + uint16_t *out); + +/* ------------------------------------------------------------------ * + * Link statistics + * ------------------------------------------------------------------ * + * + * A link that is losing packets shows up here long before it shows up on the + * dashboard. The three counters fail differently and are worth reading + * separately: + * + * checksum_errors rising electrical: noise, a marginal cable, a bad ground + * resyncs rising the sender is being interrupted mid-frame + * dropped rising packets never arrived at all; the receiver is + * not keeping up, or the sender is restarting + * + * Width is configurable because this header also compiles for an ATmega328p, + * where 64-bit counters cost RAM a Nano does not have and every increment is + * a slow multi-word add. The AVR default is 32-bit rather than 16: at roughly + * 40 packets a second, 16 bits wraps in under half an hour, and a diagnostic + * counter that quietly lies is worse than a larger one. + * + * Note this changes sizeof(sh_stats_t), so everything linked together must + * agree on it. + */ +#ifndef SH_COUNTER_BITS +# if defined(__AVR__) || defined(SH_EMBEDDED) +# define SH_COUNTER_BITS 32 +# else +# define SH_COUNTER_BITS 64 +# endif +#endif + +#if SH_COUNTER_BITS == 16 +typedef uint16_t sh_counter_t; +#elif SH_COUNTER_BITS == 32 +typedef uint32_t sh_counter_t; +#elif SH_COUNTER_BITS == 64 +typedef uint64_t sh_counter_t; +#else +#error "SH_COUNTER_BITS must be 16, 32, or 64" +#endif + +typedef struct { + sh_counter_t packets; /* accepted */ + sh_counter_t checksum_errors; /* framed correctly, contents rejected */ + sh_counter_t resyncs; /* fell back to hunting for a header */ + sh_counter_t format_errors; /* a version we do not understand */ + sh_counter_t dropped; /* inferred from gaps in the sequence */ +} sh_stats_t; + +/* ------------------------------------------------------------------ * + * Parser + * ------------------------------------------------------------------ */ + +typedef enum { + SH_WAIT_HEADER_1 = 0, + SH_WAIT_HEADER_2, + SH_READ_FORMAT, + SH_READ_LENGTH, + SH_READ_PAYLOAD, + SH_READ_CRC_LO, + SH_READ_CRC_HI, + SH_SKIP_UNKNOWN /* draining a packet whose format we do not know */ +} sh_state_t; + +typedef struct { + sh_state_t state; + uint8_t format; + uint8_t length; + uint8_t payload[SH_MAX_PAYLOAD]; + size_t payload_index; + uint8_t crc_lo; + bool have_sequence; + uint16_t last_sequence; + sh_stats_t stats; +} sh_parser_t; + +/* Reset to hunting for a header. Also zeroes the statistics. */ +void sh_parser_init(sh_parser_t *parser); + +/* + * Feed exactly one received byte. + * + * SH_OK *out now holds a valid packet + * SH_INCOMPLETE byte consumed, nothing complete yet. The usual answer. + * SH_E_CHECKSUM a frame arrived intact but its contents were rejected + * SH_E_FORMAT a frame announced a version we do not know; the parser + * uses the length byte to skip it cleanly and carries on + * SH_E_NULL parser or out was NULL + * + * Errors are reported, not thrown: the parser stays usable and keeps + * counting. A caller that only wants packets can compare against SH_OK and + * read parser->stats occasionally. + */ +sh_status_t sh_parser_feed(sh_parser_t *parser, uint8_t byte, + sh_packet_t *out); + +/* ------------------------------------------------------------------ * + * Encoding + * ------------------------------------------------------------------ */ + +/* + * CRC-16-CCITT, polynomial 0x1021, initial value 0xFFFF, no final xor. + * + * Exposed so tests and any other transmitter need not duplicate it. The + * bitwise form is deliberate: a 256-entry table would be faster and cost 512 + * bytes of flash on a part that has 30KB, to save time this loop does not + * need at 20 packets a second. + */ +uint16_t sh_crc16(const uint8_t *data, size_t length); + +/* Resume a CRC over a second run of bytes. Lets the receiver cover the + format and length fields and then the payload without copying them into + one contiguous buffer first. */ +uint16_t sh_crc16_continue(uint16_t crc, const uint8_t *data, size_t length); + +/* + * Serialise a packet into a complete frame. + * + * buffer must have room for at least SH_FRAME_SIZE bytes; pass its real size + * in buffer_size and the function will refuse rather than overrun. On success + * *written holds the frame length. written may be NULL if you do not care. + * + * SH_OK frame written + * SH_E_SPACE buffer_size was too small + * SH_E_NULL packet or buffer was NULL + */ +sh_status_t sh_encode_frame(const sh_packet_t *packet, uint8_t *buffer, + size_t buffer_size, size_t *written); + +#ifdef __cplusplus +} +#endif + +#endif /* SENSORHUB_H */ diff --git a/src/sensorhub/serial.c b/src/sensorhub/serial.c new file mode 100644 index 0000000..66cc1f3 --- /dev/null +++ b/src/sensorhub/serial.c @@ -0,0 +1,130 @@ +#define _DEFAULT_SOURCE + +/* + * POSIX only. The Arduino build compiles every source under src/, including + * this one, so it must reduce to nothing on an 8-bit target rather than drag + * termios onto a Nano. + */ +#if defined(__unix__) || defined(__APPLE__) || defined(__linux__) + + +#include "sensorhub/serial.h" + +#include +#include +#include +#include + +static bool configure_serial(int fd) +{ + struct termios tty; + + if (tcgetattr(fd, &tty) != 0) { + return false; + } + + cfmakeraw(&tty); + + if (cfsetispeed(&tty, B115200) != 0 || cfsetospeed(&tty, B115200) != 0) { + return false; + } + + tty.c_cflag &= (tcflag_t)~CSIZE; + tty.c_cflag |= CS8; + tty.c_cflag |= CLOCAL | CREAD; + + tty.c_cflag &= (tcflag_t)~PARENB; + tty.c_cflag &= (tcflag_t)~PARODD; + tty.c_cflag &= (tcflag_t)~CSTOPB; +#ifdef CRTSCTS + tty.c_cflag &= (tcflag_t)~CRTSCTS; +#endif + + /* Do not drop the modem control lines on close: that is the Nano's + auto-reset circuit, and tripping it costs a reboot mid-run. */ + tty.c_cflag &= (tcflag_t)~HUPCL; + + tty.c_iflag &= (tcflag_t)~(IXON | IXOFF | IXANY); + + /* Blocking: each read waits for at least one byte. */ + tty.c_cc[VMIN] = 1; + tty.c_cc[VTIME] = 0; + + if (tcsetattr(fd, TCSANOW, &tty) != 0) { + return false; + } + + /* Deliberately no tcflush() and no DTR/RTS handling, for the same + reason as HUPCL above. */ + return true; +} + +int sh_serial_open(const char *device) +{ + int fd; + + if (device == NULL) { + device = SH_DEFAULT_PORT; + } + + fd = open(device, O_RDWR | O_NOCTTY); + + if (fd < 0) { + return -1; + } + + if (!configure_serial(fd)) { + int saved = errno; + close(fd); + errno = saved; + return -1; + } + + return fd; +} + +void sh_serial_close(int fd) +{ + if (fd >= 0) { + close(fd); + } +} + +sh_status_t sh_serial_read_packet(int fd, sh_parser_t *parser, + sh_packet_t *out) +{ + uint8_t byte; + + if (parser == NULL || out == NULL) return SH_E_NULL; + + for (;;) { + ssize_t count = read(fd, &byte, 1); + + if (count == 1) { + sh_status_t status = sh_parser_feed(parser, byte, out); + + /* Keep reading through recoverable framing errors: a single bad + packet is not a reason to hand the caller a failure and make + it decide whether to loop. Only a completed packet or an I/O + condition ends this call. */ + if (status == SH_OK) return SH_OK; + continue; + } + + if (count == 0) return SH_E_CLOSED; + + /* EINTR is surfaced rather than retried so a caller with a shutdown + flag gets a chance to look at it. Retrying here is what makes a + serial tool impossible to Ctrl-C. */ + if (errno == EINTR) return SH_E_INTERRUPTED; + + return SH_E_IO; + } +} + +#else /* not POSIX */ + +/* Keep this a non-empty translation unit for compilers that dislike one. */ +typedef int sh_serial_unsupported_on_this_target; + +#endif diff --git a/src/sensorhub/serial.h b/src/sensorhub/serial.h new file mode 100644 index 0000000..e1279d4 --- /dev/null +++ b/src/sensorhub/serial.h @@ -0,0 +1,64 @@ +/* + * POSIX serial transport. Separate from the parser so that the framing can be + * built and tested on any host, including ones with no termios. + * + * The careful parts here are about not resetting the Arduino. A Nano reboots + * whenever DTR is asserted, which costs a couple of seconds of telemetry and, + * worse, re-runs the airspeed zeroing with the car possibly moving. So: open + * once, clear HUPCL, and never touch the modem control lines. + */ + +#ifndef SENSORHUB_SERIAL_H +#define SENSORHUB_SERIAL_H + +#include "sensorhub/sensorhub.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/* Where the CH340 lands on the Pi. /dev/serial/by-id is stabler than + /dev/ttyUSB0, which renumbers when another serial device is present. */ +#define SH_DEFAULT_PORT "/dev/ttyUSB0" +#define SH_DEFAULT_PORT_BY_ID \ + "/dev/serial/by-id/usb-1a86_USB2.0-Ser_-if00-port0" + +/* + * Open and configure a port at 115200 8N1 raw. + * + * Returns a file descriptor, or -1 with errno set. Close it with + * sh_serial_close(). Do not reopen in a retry loop without a delay: each open + * can reset the Nano through its DTR auto-reset circuit, which costs a couple + * of seconds of telemetry and re-runs the airspeed zeroing. + */ +int sh_serial_open(const char *device); + +/* Close a descriptor from sh_serial_open(). Safe to call with -1. */ +void sh_serial_close(int fd); + +/* + * Block until the next valid packet arrives, feeding bytes through parser. + * + * SH_OK *out holds a packet + * SH_E_CLOSED clean EOF; on a serial port the adapter was unplugged + * SH_E_INTERRUPTED a signal arrived; check your stop flag and call again + * SH_E_IO a real read error, see errno + * SH_E_NULL parser or out was NULL + * + * Recoverable framing errors do not end the call. A bad checksum or an + * unknown format is counted in parser->stats and reading continues, because + * one corrupt packet is not a reason to make every caller write a retry loop. + * + * SH_E_INTERRUPTED is surfaced rather than retried internally so a caller can + * actually be interrupted. Install handlers with sigaction() and no + * SA_RESTART if you want Ctrl-C to work: plain signal() sets SA_RESTART on + * most platforms, which stops read() ever returning EINTR. + */ +sh_status_t sh_serial_read_packet(int fd, sh_parser_t *parser, + sh_packet_t *out); + +#ifdef __cplusplus +} +#endif + +#endif /* SENSORHUB_SERIAL_H */ diff --git a/test/test_parser.c b/test/test_parser.c new file mode 100644 index 0000000..6ddeb74 --- /dev/null +++ b/test/test_parser.c @@ -0,0 +1,689 @@ +/* + * Framing tests. + * + * These exist to catch the failures that matter, not to demonstrate that a + * good packet parses. The interesting cases are garbage in front of a frame, + * a byte lost mid-packet, a payload containing the header bytes, a corrupted + * checksum, and, since the format now carries a version, a sender speaking a + * dialect we do not know. + */ + +#include "sensorhub/sensorhub.h" + +#include +#include + +static int g_failures = 0; +static int g_checks = 0; + +#define CHECK(cond, ...) \ + do { \ + g_checks++; \ + if (!(cond)) { \ + g_failures++; \ + printf("FAIL %s:%d: ", __FILE__, __LINE__); \ + printf(__VA_ARGS__); \ + printf("\n"); \ + } \ + } while (0) + +static sh_packet_t sample_packet(void) +{ + sh_packet_t p; + memset(&p, 0, sizeof(p)); + p.speed = 23.5f; + p.airspeed = 19.25f; + p.temps[SH_TEMP_ENGINE] = 180.0f; + p.temps[SH_TEMP_RADIATOR] = 148.5f; + p.temps[2] = 0.0f; + p.temps[3] = 0.0f; + p.analog[SH_ANALOG_BATTERY] = 812; + p.digital_in = 0x0Du; /* channels 0, 2, 3 */ + p.digital_out = 0x02u; /* output 1 */ + p.sequence = 7; + return p; +} + +static bool packets_equal(const sh_packet_t *a, const sh_packet_t *b) +{ + return memcmp(a, b, sizeof(sh_packet_t)) == 0; +} + +static void encode(const sh_packet_t *p, uint8_t *buf) +{ + sh_status_t st = sh_encode_frame(p, buf, SH_FRAME_SIZE, NULL); + CHECK(st == SH_OK, "encode should succeed, got %s", sh_strstatus(st)); +} + +/* Feed a buffer through a parser, returning how many packets came out. */ +static int feed_all(sh_parser_t *parser, const uint8_t *data, size_t len, + sh_packet_t *last) +{ + int count = 0; + sh_packet_t out; + + for (size_t i = 0; i < len; ++i) { + if (sh_parser_feed(parser, data[i], &out) == SH_OK) { + count++; + if (last != NULL) *last = out; + } + } + + return count; +} + +static void test_sizes_are_the_wire_contract(void) +{ + CHECK(sizeof(sh_packet_t) == 36, "payload must stay 36 bytes"); + CHECK(SH_FRAME_SIZE == 42, "frame must stay 42 bytes"); + CHECK(SH_FRAME_OVERHEAD == 6, "overhead is header, fmt, len, 2 CRC bytes"); +} + +static void test_roundtrip(void) +{ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t frame[SH_FRAME_SIZE]; + sh_parser_t parser; + + encode(&in, frame); + sh_parser_init(&parser); + + CHECK(feed_all(&parser, frame, sizeof(frame), &out) == 1, + "one frame should yield one packet"); + CHECK(packets_equal(&in, &out), "round-tripped packet should match"); + CHECK(parser.stats.packets == 1, "packet counter should be 1"); + CHECK(parser.stats.checksum_errors == 0, "no checksum errors expected"); +} + +static void test_leading_garbage(void) +{ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t buf[80]; + sh_parser_t parser; + size_t n = 0; + + buf[n++] = 0x00; + buf[n++] = 0xFF; + buf[n++] = SH_HEADER_1; /* a lone header byte going nowhere */ + buf[n++] = 0x12; + buf[n++] = 0x34; + + encode(&in, buf + n); + n += SH_FRAME_SIZE; + + sh_parser_init(&parser); + CHECK(feed_all(&parser, buf, n, &out) == 1, + "should recover from leading garbage"); + CHECK(packets_equal(&in, &out), "recovered packet should match"); +} + +static void test_payload_containing_header_bytes(void) +{ + /* A payload byte equal to 0xAA immediately before the real 0xAA 0x55 is + the case a naive two-state matcher gets wrong. */ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t buf[8 + SH_FRAME_SIZE]; + sh_parser_t parser; + size_t n = 0; + + buf[n++] = SH_HEADER_1; + buf[n++] = SH_HEADER_1; + buf[n++] = SH_HEADER_1; + + encode(&in, buf + n); + n += SH_FRAME_SIZE; + + sh_parser_init(&parser); + CHECK(feed_all(&parser, buf, n, &out) == 1, + "repeated 0xAA before a header must not break framing"); + CHECK(packets_equal(&in, &out), "packet after repeated 0xAA should match"); +} + +static void test_bad_checksum_is_counted_not_silent(void) +{ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t frame[SH_FRAME_SIZE]; + sh_parser_t parser; + sh_status_t last = SH_OK; + + encode(&in, frame); + frame[SH_FRAME_SIZE - 1] ^= 0xFFu; + + sh_parser_init(&parser); + for (size_t i = 0; i < sizeof(frame); ++i) { + last = sh_parser_feed(&parser, frame[i], &out); + } + + CHECK(last == SH_E_CHECKSUM, "a bad CRC must report SH_E_CHECKSUM"); + CHECK(parser.stats.checksum_errors == 1, "and must be counted"); + CHECK(parser.stats.packets == 0, "and must not yield a packet"); +} + +static void test_corrupt_payload_is_rejected(void) +{ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t frame[SH_FRAME_SIZE]; + sh_parser_t parser; + + encode(&in, frame); + frame[8] ^= 0x01u; /* flip a bit inside the payload */ + + sh_parser_init(&parser); + CHECK(feed_all(&parser, frame, sizeof(frame), &out) == 0, + "a corrupted payload must be rejected"); + CHECK(parser.stats.checksum_errors == 1, "corruption should be counted"); +} + +static void test_resync_after_dropped_byte(void) +{ + /* The real-world failure: one byte lost in transit. The truncated frame + must be discarded and the next one must still parse. */ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t buf[SH_FRAME_SIZE * 3]; + sh_parser_t parser; + size_t n; + + encode(&in, buf); + n = SH_FRAME_SIZE - 1; /* drop the tail of frame one */ + + encode(&in, buf + n); + n += SH_FRAME_SIZE; + encode(&in, buf + n); + n += SH_FRAME_SIZE; + + sh_parser_init(&parser); + int got = feed_all(&parser, buf, n, &out); + + CHECK(got >= 1, "must resynchronise after a dropped byte, got %d", got); + CHECK(packets_equal(&in, &out), "post-resync packet should match"); +} + +static void test_back_to_back_frames(void) +{ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t buf[SH_FRAME_SIZE * 5]; + sh_parser_t parser; + + for (int i = 0; i < 5; ++i) { + in.speed = (float)i; + in.sequence = (uint16_t)(100 + i); + encode(&in, buf + ((size_t)i * SH_FRAME_SIZE)); + } + + sh_parser_init(&parser); + CHECK(feed_all(&parser, buf, sizeof(buf), &out) == 5, + "five frames should yield five packets"); + CHECK(out.speed == 4.0f, "last packet should be the last one sent"); + CHECK(parser.stats.dropped == 0, "a contiguous run drops nothing"); +} + +/* ---- the version and length bytes, which is why they are on the wire ---- */ + +static void test_unknown_format_is_refused_not_decoded(void) +{ + /* The whole point. A sender speaking a version we do not know must be + rejected loudly, not decoded into plausible nonsense. */ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t frame[SH_FRAME_SIZE]; + sh_parser_t parser; + sh_status_t seen = SH_OK; + + encode(&in, frame); + frame[2] = 0x02u; /* a future format */ + /* fix the CRC so this is purely a version rejection, not a corruption + that would have been caught anyway */ + { + uint16_t crc = sh_crc16(frame + 2, SH_PAYLOAD_SIZE + 2u); + frame[SH_FRAME_SIZE - 2] = (uint8_t)(crc & 0xFFu); + frame[SH_FRAME_SIZE - 1] = (uint8_t)(crc >> 8); + } + + sh_parser_init(&parser); + for (size_t i = 0; i < sizeof(frame); ++i) { + sh_status_t st = sh_parser_feed(&parser, frame[i], &out); + if (st == SH_E_FORMAT) seen = st; + CHECK(st != SH_OK, "an unknown format must never yield a packet"); + } + + CHECK(seen == SH_E_FORMAT, "should have reported SH_E_FORMAT"); + CHECK(parser.stats.format_errors == 1, "and counted it"); + CHECK(parser.stats.packets == 0, "and produced nothing"); +} + +static void test_length_lets_us_skip_a_newer_sender(void) +{ + /* An old receiver against a new sender: the length byte is what keeps it + framed. After skipping a longer packet it does not understand, the very + next packet it does understand must still parse. */ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t buf[128]; + sh_parser_t parser; + size_t n = 0; + + /* A future frame: format 2, payload 50 bytes, then its two CRC bytes. */ + buf[n++] = SH_HEADER_1; + buf[n++] = SH_HEADER_2; + buf[n++] = 0x02u; + buf[n++] = 50u; + for (int i = 0; i < 50; ++i) buf[n++] = (uint8_t)i; + buf[n++] = 0x00u; + buf[n++] = 0x00u; + + /* Then a frame we do understand. */ + encode(&in, buf + n); + n += SH_FRAME_SIZE; + + sh_parser_init(&parser); + CHECK(feed_all(&parser, buf, n, &out) == 1, + "must stay framed across a packet from a newer sender"); + CHECK(packets_equal(&in, &out), "the packet after the skip should match"); + CHECK(parser.stats.format_errors == 1, "the unknown frame was counted"); +} + +static void test_wrong_length_for_our_own_format(void) +{ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t frame[SH_FRAME_SIZE]; + sh_parser_t parser; + sh_status_t seen = SH_OK; + + encode(&in, frame); + frame[3] = 99u; /* our format, an impossible length */ + + sh_parser_init(&parser); + for (size_t i = 0; i < sizeof(frame); ++i) { + sh_status_t st = sh_parser_feed(&parser, frame[i], &out); + if (st == SH_E_LENGTH) seen = st; + } + + CHECK(seen == SH_E_LENGTH, "a bad length must report SH_E_LENGTH"); + CHECK(parser.stats.packets == 0, "and must not yield a packet"); +} + +static void test_checksum_covers_the_header_fields(void) +{ + /* If the checksum only covered the payload, flipping the length byte + would reframe the stream and still validate. */ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t frame[SH_FRAME_SIZE]; + sh_parser_t parser; + + encode(&in, frame); + frame[3] = (uint8_t)(SH_PAYLOAD_SIZE - 1u); /* lie about the length */ + + sh_parser_init(&parser); + CHECK(feed_all(&parser, frame, sizeof(frame), &out) == 0, + "a tampered length byte must not produce a packet"); +} + +static void test_dropped_packets_are_counted_from_the_sequence(void) +{ + /* A checksum cannot tell you about a packet that never arrived. The + sequence number can. */ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t frame[SH_FRAME_SIZE]; + sh_parser_t parser; + + sh_parser_init(&parser); + + in.sequence = 10; + encode(&in, frame); + feed_all(&parser, frame, sizeof(frame), &out); + + in.sequence = 14; /* 11, 12, 13 never arrived */ + encode(&in, frame); + feed_all(&parser, frame, sizeof(frame), &out); + + CHECK(parser.stats.dropped == 3, "should have counted 3 dropped, got %lu", + (unsigned long)parser.stats.dropped); + CHECK(parser.stats.packets == 2, "two packets did arrive"); +} + +static void test_sequence_wrap_is_not_a_flood(void) +{ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t frame[SH_FRAME_SIZE]; + sh_parser_t parser; + + sh_parser_init(&parser); + + in.sequence = 65535; + encode(&in, frame); + feed_all(&parser, frame, sizeof(frame), &out); + + in.sequence = 0; /* wrapped, not 65535 packets lost */ + encode(&in, frame); + feed_all(&parser, frame, sizeof(frame), &out); + + CHECK(parser.stats.dropped == 0, "a clean wrap drops nothing, got %lu", + (unsigned long)parser.stats.dropped); +} + +static void test_sender_restart_is_not_a_flood(void) +{ + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t frame[SH_FRAME_SIZE]; + sh_parser_t parser; + + sh_parser_init(&parser); + + in.sequence = 40000; + encode(&in, frame); + feed_all(&parser, frame, sizeof(frame), &out); + + in.sequence = 0; /* the Arduino rebooted */ + encode(&in, frame); + feed_all(&parser, frame, sizeof(frame), &out); + + CHECK(parser.stats.dropped == 0, + "a restart should not report a fictional flood, got %lu", + (unsigned long)parser.stats.dropped); +} + +/* ---- the API surface itself ---- */ + +static void test_digital_accessors(void) +{ + sh_packet_t p; + bool v; + + memset(&p, 0, sizeof(p)); + + CHECK(sh_set_digital_in(&p, 3, true) == SH_OK, "set channel 3"); + CHECK(sh_digital_in(&p, 3, &v) == SH_OK && v, "channel 3 should read set"); + CHECK(sh_digital_in(&p, 4, &v) == SH_OK && !v, "channel 4 should be clear"); + + CHECK(sh_set_digital_in(&p, 3, false) == SH_OK, "clear channel 3"); + CHECK(sh_digital_in(&p, 3, &v) == SH_OK && !v, "channel 3 should clear"); + + CHECK(sh_set_digital_out(&p, 0, true) == SH_OK, "set output 0"); + CHECK(sh_digital_out(&p, 0, &v) == SH_OK && v, "output 0 should read set"); + /* outputs and inputs must not alias each other */ + CHECK(sh_digital_in(&p, 0, &v) == SH_OK && !v, + "setting an output must not touch the inputs"); +} + +static void test_out_of_range_is_an_error_not_a_read(void) +{ + sh_packet_t p = sample_packet(); + bool v; + float f; + uint16_t a; + + CHECK(sh_digital_in(&p, 8, &v) == SH_E_RANGE, "channel 8 is out of range"); + CHECK(sh_digital_out(&p, 99, &v) == SH_E_RANGE, "so is 99"); + CHECK(sh_set_digital_in(&p, 8, true) == SH_E_RANGE, "and for writes"); + CHECK(sh_temp(&p, SH_TEMP_COUNT, &f) == SH_E_RANGE, "temp index bound"); + CHECK(sh_analog(&p, SH_ANALOG_COUNT, &a) == SH_E_RANGE, "analog bound"); + + CHECK(sh_temp(&p, SH_TEMP_ENGINE, &f) == SH_OK && f == 180.0f, + "a valid temp index still works"); + CHECK(sh_analog(&p, SH_ANALOG_BATTERY, &a) == SH_OK && a == 812, + "a valid analog index still works"); +} + +static void test_encode_refuses_a_short_buffer(void) +{ + sh_packet_t p = sample_packet(); + uint8_t small[SH_FRAME_SIZE - 1]; + uint8_t ok[SH_FRAME_SIZE]; + size_t written = 0; + + CHECK(sh_encode_frame(&p, small, sizeof(small), &written) == SH_E_SPACE, + "a short buffer must be refused, not overrun"); + CHECK(sh_encode_frame(&p, ok, sizeof(ok), &written) == SH_OK, + "a correctly sized buffer works"); + CHECK(written == SH_FRAME_SIZE, "and reports what it wrote"); +} + +static void test_null_arguments_are_safe(void) +{ + sh_parser_t parser; + sh_packet_t out; + sh_packet_t p = sample_packet(); + uint8_t buf[SH_FRAME_SIZE]; + bool v; + + sh_parser_init(NULL); /* must not crash */ + sh_parser_init(&parser); + + CHECK(sh_parser_feed(NULL, 0, &out) == SH_E_NULL, "NULL parser"); + CHECK(sh_parser_feed(&parser, 0, NULL) == SH_E_NULL, "NULL out"); + CHECK(sh_encode_frame(NULL, buf, sizeof(buf), NULL) == SH_E_NULL, + "NULL packet"); + CHECK(sh_encode_frame(&p, NULL, sizeof(buf), NULL) == SH_E_NULL, + "NULL buffer"); + CHECK(sh_digital_in(NULL, 0, &v) == SH_E_NULL, "NULL packet to accessor"); + CHECK(sh_digital_in(&p, 0, NULL) == SH_E_NULL, "NULL out to accessor"); + CHECK(sh_crc16(NULL, 10) == 0xFFFFu, + "NULL CRC input returns the initial value, not garbage"); +} + +static void test_status_strings_exist(void) +{ + /* A status with no message turns a diagnosable failure into "error 7". */ + static const sh_status_t all[] = { + SH_OK, SH_INCOMPLETE, SH_E_NULL, SH_E_CHECKSUM, SH_E_FORMAT, + SH_E_LENGTH, SH_E_RANGE, SH_E_SPACE, SH_E_OPEN, SH_E_IO, + SH_E_INTERRUPTED, SH_E_CLOSED + }; + + for (size_t i = 0; i < sizeof(all) / sizeof(all[0]); ++i) { + const char *s = sh_strstatus(all[i]); + CHECK(s != NULL && s[0] != '\0' && strcmp(s, "unknown status") != 0, + "status %d needs its own message", (int)all[i]); + } + + CHECK(strcmp(sh_strstatus((sh_status_t)999), "unknown status") == 0, + "a bogus status should still return a string, not NULL"); + CHECK(!sh_failed(SH_OK), "SH_OK is not a failure"); + CHECK(!sh_failed(SH_INCOMPLETE), + "SH_INCOMPLETE is not a failure; it is the usual answer"); + CHECK(sh_failed(SH_E_CHECKSUM), "SH_E_CHECKSUM is a failure"); +} + +static void test_wire_format_is_frozen(void) +{ + /* + * A golden frame, byte for byte. This is the contract with + * carsensordriver.ino and with every consumer downstream. + * + * If this fails, the wire format changed. That is not a test to update, + * it is a reflash-every-Arduino event: bump SH_FORMAT_CURRENT, regenerate + * this, and tell everyone. + */ + static const uint8_t golden[SH_FRAME_SIZE] = { + 0xAA, 0x55, /* header */ + 0x01, /* format 1 */ + 0x24, /* payload length, 36 */ + 0x00, 0x00, 0xBC, 0x41, /* speed 23.5 */ + 0x00, 0x00, 0x9A, 0x41, /* airspeed 19.25 */ + 0x00, 0x00, 0x34, 0x43, /* temps[0] 180.0 */ + 0x00, 0x80, 0x14, 0x43, /* temps[1] 148.5 */ + 0x00, 0x00, 0x00, 0x00, /* temps[2] 0.0 */ + 0x00, 0x00, 0x00, 0x00, /* temps[3] 0.0 */ + 0x2C, 0x03, /* analog[0] 812 */ + 0x00, 0x00, /* analog[1] */ + 0x00, 0x00, /* analog[2] */ + 0x00, 0x00, /* analog[3] */ + 0x0D, /* digital_in 0b00001101 */ + 0x02, /* digital_out 0b00000010 */ + 0x07, 0x00, /* sequence 7 */ + 0x51, 0xF1 /* CRC-16 over fmt, len, payload, LE */ + }; + + sh_packet_t in = sample_packet(); + sh_packet_t out; + uint8_t frame[SH_FRAME_SIZE]; + sh_parser_t parser; + + encode(&in, frame); + + CHECK(memcmp(frame, golden, sizeof(golden)) == 0, + "encoded frame must match the frozen wire format byte for byte"); + + /* And the parser must accept its own golden frame, which catches an + encoder and decoder that drifted together. */ + sh_parser_init(&parser); + CHECK(feed_all(&parser, golden, sizeof(golden), &out) == 1, + "the frozen frame must still decode"); + CHECK(packets_equal(&in, &out), "decoded golden frame should match"); +} + + +/* ---- the corruptions an XOR checksum could not see ---- */ + +/* Feed a whole frame and report whether it was accepted. */ +static bool frame_accepted(const uint8_t *frame, size_t len) +{ + sh_parser_t parser; + sh_packet_t out; + + sh_parser_init(&parser); + for (size_t i = 0; i < len; ++i) { + if (sh_parser_feed(&parser, frame[i], &out) == SH_OK) return true; + } + return false; +} + +static void test_two_flips_in_the_same_bit_position(void) +{ + /* + * The reason this frame carries a CRC rather than an XOR sum. + * + * Two bit flips in the same bit position cancel exactly under XOR, so a + * byte-sum accepted 100% of these in a two-million-frame simulation. It + * is not an exotic case either: a ground bounce or a supply glitch + * disturbing one data line flips the same bit in consecutive bytes, + * which is exactly what an ignition system produces. + */ + sh_packet_t in = sample_packet(); + uint8_t frame[SH_FRAME_SIZE]; + unsigned accepted = 0; + + for (unsigned bit = 0; bit < 8u; ++bit) { + for (unsigned i = 4; i < 4u + SH_PAYLOAD_SIZE - 1u; ++i) { + encode(&in, frame); + frame[i] ^= (uint8_t)(1u << bit); + frame[i + 1] ^= (uint8_t)(1u << bit); + if (frame_accepted(frame, sizeof(frame))) accepted++; + } + } + + CHECK(accepted == 0, + "paired same-bit flips must all be caught, %u slipped through", + accepted); +} + +static void test_swapped_bytes(void) +{ + /* + * The other one. XOR is order-independent, so swapping two payload bytes + * is completely invisible to it: also 100% undetected in simulation. + */ + sh_packet_t in = sample_packet(); + uint8_t frame[SH_FRAME_SIZE]; + unsigned accepted = 0; + unsigned tried = 0; + + for (unsigned i = 4; i < 4u + SH_PAYLOAD_SIZE; ++i) { + for (unsigned j = i + 1u; j < 4u + SH_PAYLOAD_SIZE; ++j) { + uint8_t tmp; + + encode(&in, frame); + if (frame[i] == frame[j]) continue; /* a no-op swap */ + + tmp = frame[i]; + frame[i] = frame[j]; + frame[j] = tmp; + tried++; + + if (frame_accepted(frame, sizeof(frame))) accepted++; + } + } + + CHECK(tried > 100, "should have tried a decent number of swaps, got %u", + tried); + CHECK(accepted == 0, "swapped bytes must all be caught, %u slipped through", + accepted); +} + +static void test_every_single_bit_flip_is_caught(void) +{ + /* Across the whole frame, including the header fields and the CRC. */ + sh_packet_t in = sample_packet(); + uint8_t frame[SH_FRAME_SIZE]; + unsigned accepted = 0; + + for (unsigned i = 2; i < SH_FRAME_SIZE; ++i) { + for (unsigned bit = 0; bit < 8u; ++bit) { + encode(&in, frame); + frame[i] ^= (uint8_t)(1u << bit); + if (frame_accepted(frame, sizeof(frame))) accepted++; + } + } + + CHECK(accepted == 0, "every single-bit flip must be caught, %u were not", + accepted); +} + +static void test_crc_matches_the_reference_vector(void) +{ + /* CRC-16-CCITT (0x1021, init 0xFFFF) over "123456789" is 0x29B1. This is + the standard check value; if it fails, the polynomial or the init + constant drifted. */ + static const uint8_t check[] = "123456789"; + + CHECK(sh_crc16(check, sizeof(check) - 1u) == 0x29B1u, + "CRC-16-CCITT check value must be 0x29B1, got 0x%04X", + (unsigned)sh_crc16(check, sizeof(check) - 1u)); +} + +int main(void) +{ + test_sizes_are_the_wire_contract(); + test_wire_format_is_frozen(); + test_roundtrip(); + test_leading_garbage(); + test_payload_containing_header_bytes(); + test_bad_checksum_is_counted_not_silent(); + test_corrupt_payload_is_rejected(); + test_resync_after_dropped_byte(); + test_back_to_back_frames(); + + test_unknown_format_is_refused_not_decoded(); + test_length_lets_us_skip_a_newer_sender(); + test_wrong_length_for_our_own_format(); + test_checksum_covers_the_header_fields(); + test_dropped_packets_are_counted_from_the_sequence(); + test_sequence_wrap_is_not_a_flood(); + test_sender_restart_is_not_a_flood(); + + test_digital_accessors(); + test_out_of_range_is_an_error_not_a_read(); + test_encode_refuses_a_short_buffer(); + test_null_arguments_are_safe(); + test_status_strings_exist(); + + test_crc_matches_the_reference_vector(); + test_every_single_bit_flip_is_caught(); + test_two_flips_in_the_same_bit_position(); + test_swapped_bytes(); + + printf("%d checks, %d failures\n", g_checks, g_failures); + return g_failures == 0 ? 0 : 1; +} diff --git a/test/test_serial_pty.py b/test/test_serial_pty.py new file mode 100755 index 0000000..0444f35 --- /dev/null +++ b/test/test_serial_pty.py @@ -0,0 +1,147 @@ +#!/usr/bin/env python3 +""" +End-to-end check against a fake Arduino. + +Opens a pty, speaks the exact wire format carsensordriver.ino speaks, and runs +sensorhub-monitor against the other end. This is the test that covers the +parts a unit test cannot: termios setup, blocking reads, signal handling, and +the framing under a realistic mix of good frames, line noise, and corruption. + +It also pins behaviour that is easy to regress: a corrupt packet must be +counted rather than silently dropped, and the tool must exit on a signal +instead of hanging in a blocking read. + + ./test/test_serial_pty.py build/sensorhub-monitor +""" + +import os +import pty +import struct +import subprocess +import sys +import time + + +FORMAT_V1 = 1 +PAYLOAD_SIZE = 36 + + +def frame(speed, airspeed, temps, analog, digital_in, digital_out, sequence, + fmt=FORMAT_V1): + """Build one 42-byte frame. + + 0xAA 0x55, format, length, 36-byte payload, then a little-endian + CRC-16-CCITT covering the format and length bytes as well as the payload. + """ + payload = struct.pack( + "> 8) & 0xFF])) + + +def crc16_ccitt(data): + """CRC-16-CCITT, poly 0x1021, init 0xFFFF. Check value for b"123456789" + is 0x29B1; asserted below so a typo here cannot masquerade as a firmware + bug.""" + crc = 0xFFFF + for byte in data: + crc ^= byte << 8 + for _ in range(8): + crc = ((crc << 1) ^ 0x1021) & 0xFFFF if crc & 0x8000 \ + else (crc << 1) & 0xFFFF + return crc + + +assert crc16_ccitt(b"123456789") == 0x29B1, "CRC implementation is wrong" + + +def main(): + if len(sys.argv) < 2: + sys.exit("usage: test_serial_pty.py ") + + monitor = sys.argv[1] + master, slave = pty.openpty() + device = os.ttyname(slave) + + proc = subprocess.Popen( + [monitor, device], stdout=subprocess.PIPE, stderr=subprocess.STDOUT + ) + time.sleep(0.5) + + good = dict(temps=(180.0, 148.5, 0.0, 0.0), analog=(812, 0, 0, 0), + digital_in=0x0D, digital_out=0x02) + + # A good frame. + os.write(master, frame(23.5, 19.25, sequence=1, **good)) + time.sleep(0.2) + + # Line noise, then a frame with a deliberately corrupted CRC. + os.write(master, b"\x00\xff\xde\xad\xbe\xef") + corrupt = bytearray(frame(11.0, 2.0, sequence=2, **good)) + corrupt[-1] ^= 0xFF + os.write(master, bytes(corrupt)) + time.sleep(0.2) + + # A packet from a "newer" sender. The length byte must let the receiver + # skip it and stay framed rather than desynchronising. + future = (bytes([0xAA, 0x55, 0x02, 50]) + bytes(range(50)) + + bytes([0x00, 0x00])) + os.write(master, future) + time.sleep(0.2) + + # A good frame after all of that. Sequence jumps 1 -> 5, so 2, 3 and 4 + # never arrived and the receiver should say so. (The corrupt frame above + # carried sequence 2, but it was rejected, so it does not count as + # received.) + os.write(master, frame(31.25, 5.5, sequence=5, + temps=(190.0, 150.0, 0.0, 0.0), + analog=(900, 0, 0, 0), + digital_in=0x16, digital_out=0x01)) + time.sleep(0.8) + + proc.terminate() + + try: + out = proc.communicate(timeout=5)[0].decode(errors="replace") + except subprocess.TimeoutExpired: + proc.kill() + proc.communicate() + sys.exit("FAIL: monitor did not exit on SIGTERM (stuck in a blocking read?)") + + print(out) + + failures = [] + if "speed=23.50" not in out: + failures.append("first good packet was not decoded") + if "speed=31.25" not in out: + failures.append("did not recover after corruption and a skip") + if "ok=2" not in out: + failures.append("expected exactly 2 accepted packets") + if "bad=1" not in out: + failures.append("corrupt packet was not counted as a CRC error") + if "fmt=1" not in out: + failures.append("packet from a newer sender was not counted as an " + "unknown format") + if "lost=3" not in out: + failures.append("gap in the sequence numbers was not counted as " + "dropped packets") + + if failures: + for f in failures: + print("FAIL:", f) + sys.exit(1) + + print("PASS: framing, recovery, and shutdown all behaved") + + +if __name__ == "__main__": + main() diff --git a/tools/monitor.c b/tools/monitor.c new file mode 100644 index 0000000..bf756c6 --- /dev/null +++ b/tools/monitor.c @@ -0,0 +1,135 @@ +/* + * sensorhub-monitor: print live packets from the car. + * + * This is the bench tool. Point it at the Arduino and you can see whether the + * link is healthy and whether a given sensor is actually wired, without + * involving the display or any of the rest of the stack. + * + * sensorhub-monitor [device] + */ + +#include "sensorhub/serial.h" + +#include +#include +#include +#include +#include + +static volatile sig_atomic_t g_stop = 0; + +static void on_signal(int signum) +{ + (void)signum; + g_stop = 1; +} + +static void install_handler(int signum) +{ + struct sigaction sa; + + memset(&sa, 0, sizeof(sa)); + sa.sa_handler = on_signal; + sigemptyset(&sa.sa_mask); + /* No SA_RESTART on purpose: we want read() to return EINTR so the loop + below can notice g_stop. signal() would set SA_RESTART and make this + process impossible to Ctrl-C out of a blocking read. */ + sa.sa_flags = 0; + (void)sigaction(signum, &sa, NULL); +} + +static void print_packet(const sh_packet_t *packet, const sh_stats_t *stats) +{ + char digital[18]; + unsigned i; + + /* inputs then outputs, most significant channel on the left so the + string reads the way the bitfield is written */ + for (i = 0; i < 8; ++i) { + bool v = false; + (void)sh_digital_in(packet, 7 - i, &v); + digital[i] = v ? '1' : '0'; + } + digital[8] = ' '; + for (i = 0; i < 8; ++i) { + bool v = false; + (void)sh_digital_out(packet, 7 - i, &v); + digital[9 + i] = v ? '1' : '0'; + } + digital[17] = '\0'; + + printf("\r\x1b[2K" + "#%u speed=%.2f air=%.2f" + " | eng=%.1fF rad=%.1fF" + " | A0=%u" + " | in/out=%s" + " | ok=%llu bad=%llu fmt=%llu resync=%llu lost=%llu", + (unsigned)packet->sequence, + (double)packet->speed, + (double)packet->airspeed, + (double)packet->temps[SH_TEMP_ENGINE], + (double)packet->temps[SH_TEMP_RADIATOR], + (unsigned)packet->analog[SH_ANALOG_BATTERY], + digital, + (unsigned long long)stats->packets, + (unsigned long long)stats->checksum_errors, + (unsigned long long)stats->format_errors, + (unsigned long long)stats->resyncs, + (unsigned long long)stats->dropped); + + fflush(stdout); +} + +int main(int argc, char **argv) +{ + const char *device = (argc > 1) ? argv[1] : SH_DEFAULT_PORT; + sh_parser_t parser; + sh_packet_t packet; + sh_status_t status; + int fd; + + signal(SIGPIPE, SIG_IGN); + install_handler(SIGINT); + install_handler(SIGTERM); + + /* Open once. Reopening resets the Nano through its DTR circuit. */ + fd = sh_serial_open(device); + + if (fd < 0) { + fprintf(stderr, "open %s: %s\n", device, strerror(errno)); + return 1; + } + + sh_parser_init(&parser); + printf("Listening on %s at 115200 baud\n", device); + + while (!g_stop) { + memset(&packet, 0, sizeof(packet)); + + status = sh_serial_read_packet(fd, &parser, &packet); + + if (status == SH_E_INTERRUPTED) { + continue; /* a signal; the while condition rechecks g_stop */ + } + + if (status != SH_OK) { + fprintf(stderr, "\n%s", sh_strstatus(status)); + if (status == SH_E_IO) fprintf(stderr, ": %s", strerror(errno)); + fprintf(stderr, "\n"); + break; + } + + print_packet(&packet, &parser.stats); + } + + printf("\n%llu packets, %llu bad checksums, %llu unknown formats, " + "%llu resyncs, %llu never arrived\n", + (unsigned long long)parser.stats.packets, + (unsigned long long)parser.stats.checksum_errors, + (unsigned long long)parser.stats.format_errors, + (unsigned long long)parser.stats.resyncs, + (unsigned long long)parser.stats.dropped); + + sh_serial_close(fd); + return 0; +}