From 51ad5a6d7e1ff1b69869c15bcb8475260f55f8cd Mon Sep 17 00:00:00 2001 From: Kieran Klukas Date: Sat, 19 Sep 2026 11:58:36 -0400 Subject: [PATCH 1/4] feat: share the packet format with the Pi instead of duplicating it The firmware defined the packet struct, the checksum, and the frame encoder itself, and the Raspberry Pi defined all three again on its side. Editing one without the other would not break a build, it would just make the car log nonsense. Both ends now use the same definition from the SensorHub library, so the drift is no longer expressible. Costs 22 bytes of flash, 0.07% of a Nano, and no RAM. Frames are byte-identical to before, checked against the old encoder over 200,000 random packets. Adds CI that compiles for the actual board, reports flash and RAM use, and fails if the sketch grows past 80% of flash, since running out of room on a Nano is a real failure mode when someone adds a sensor. Needs HEEV/SensorHub to land first; CI here installs it from main. --- .github/workflows/ci.yml | 65 ++++++++++++++++++++++++++++++++++++++++ carsensordriver.ino | 62 ++++++++++++++++---------------------- 2 files changed, 90 insertions(+), 37 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..2e6b015 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,65 @@ +name: CI + +on: + push: + branches: [ main ] + pull_request: + workflow_dispatch: + +jobs: + compile: + # the firmware is the one piece nobody can test without hardware, so at + # minimum prove it still compiles for the board it actually runs on, and + # report what it costs + name: compile for the Nano + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + path: sketch/carsensordriver + + - name: Install arduino-cli + run: | + curl -fsSL https://raw.githubusercontent.com/arduino/arduino-cli/master/install.sh \ + | sh -s -- -b /usr/local/bin + arduino-cli version + + - name: Install the AVR core and libraries + # DS18B20 needs OneWire, which it does not pull in itself + run: | + arduino-cli core update-index + arduino-cli core install arduino:avr + arduino-cli lib install DS18B20 + arduino-cli lib install OneWire + + - name: Install SensorHub, which supplies the shared packet format + run: | + arduino-cli lib install --git-url https://github.com/HEEV/SensorHub || { + echo "falling back to a direct checkout" + git clone --depth 1 https://github.com/HEEV/SensorHub.git \ + "$(arduino-cli config get directories.user)/libraries/SensorHub" + } + + - name: Compile + run: | + arduino-cli compile \ + --fqbn arduino:avr:nano:cpu=atmega328 \ + --warnings all \ + sketch/carsensordriver 2>&1 | tee /tmp/build.log + + - name: Report flash and RAM use + # a Nano has 30720 bytes of flash and 2048 of RAM. running out is a + # real failure mode when someone adds a sensor, and it is much easier + # to notice here than at an event. + run: grep -E "Sketch uses|Global variables" /tmp/build.log + + - name: Fail if the sketch no longer fits with room to spare + # 80% is the line; below that there is space for another sensor + run: | + used=$(grep -oE "Sketch uses [0-9]+" /tmp/build.log | grep -oE "[0-9]+") + pct=$(( used * 100 / 30720 )) + echo "flash: ${used} bytes, ${pct}% of a Nano" + if [ "$pct" -ge 80 ]; then + echo "over 80% of flash; time to think about what to cut" + exit 1 + fi diff --git a/carsensordriver.ino b/carsensordriver.ino index 0065127..9e13b35 100644 --- a/carsensordriver.ino +++ b/carsensordriver.ino @@ -1,8 +1,25 @@ #include #include -const uint8_t HEADER_1 = 0xAA; -const uint8_t HEADER_2 = 0x55; +/* + * The packet layout, the checksum, and the frame encoder live in the + * SensorHub library, which the Raspberry Pi uses to decode this. Sharing one + * definition is the whole point: the two ends cannot drift apart, because + * there is only one of them to edit. + * + * Install with: + * arduino-cli lib install --git-url https://github.com/HEEV/SensorHub + * + * Only the encoder is linked here, about 90 bytes of flash more than the + * hand-rolled version it replaced. The receiving state machine comes along in + * the same header for whenever the Pi starts commanding the output channels + * on pins 10, 11, and 12. + */ +#include + +/* Keep the local spelling so the call sites below read unchanged. */ +typedef sh_packet_t DataPacket; + //wheel speed constants #define wheelSpeedSensorPin 2 #define numMagnets 1 @@ -31,43 +48,14 @@ const uint8_t radTempAddr[8] = { 0x28, 0xD0, 0xEB, 0x87, 0x00, 0xCA, 0x26, 0x const int cacheTTL[] = {50, 50}; int cacheLife[] = {0, 0}; -// This is a struct with all padding bytes removed, which is used for efficient sending of data over the serial bus. -// unsigned char = 1 byte -// unsigned int = 2 bytes -// float = 4 bytes - - -struct __attribute__((packed)) DataPacket { - 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; -}; - -uint8_t packetChecksum(const uint8_t *data, size_t length) { - uint8_t checksum = 0; - - for (size_t i = 0; i < length; i++) { - checksum ^= data[i]; - } - - return checksum; -} - void sendPacket(const DataPacket &packet) { - const uint8_t *payload = (const uint8_t *)&packet; - uint8_t checksum = packetChecksum(payload, sizeof(DataPacket)); + /* One buffered write rather than four: header, payload, and checksum are + assembled by the shared encoder, so the Pi's decoder and this cannot + disagree about the format. */ + uint8_t frame[SH_FRAME_SIZE]; - Serial.write(HEADER_1); - Serial.write(HEADER_2); - Serial.write(payload, sizeof(DataPacket)); - Serial.write(checksum); + sh_encode_frame(&packet, frame); + Serial.write(frame, sizeof(frame)); } void setup() { From 30777fae5288ef48b50342be3ac7b8d1c84b9b75 Mon Sep 17 00:00:00 2001 From: Kieran Klukas Date: Sat, 19 Sep 2026 12:01:34 -0400 Subject: [PATCH 2/4] ci: install arduino-cli with the official action The install script reads its first positional argument as a version, so passing a bindir made it try to download arduino-cli_-b_Linux_64bit. --- .github/workflows/ci.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2e6b015..66e08ec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,10 +19,13 @@ jobs: path: sketch/carsensordriver - name: Install arduino-cli - run: | - curl -fsSL https://raw.githubusercontent.com/arduino/arduino-cli/master/install.sh \ - | sh -s -- -b /usr/local/bin - arduino-cli version + # the official action rather than the install script: the script + # reads its first positional argument as a version, so passing a + # bindir there makes it fetch arduino-cli_-b_Linux_64bit.tar.gz + uses: arduino/setup-arduino-cli@v2 + + - name: Show the version in use + run: arduino-cli version - name: Install the AVR core and libraries # DS18B20 needs OneWire, which it does not pull in itself From 07908c61bf2c25836d0be1dcd409d9ffaabbcffa Mon Sep 17 00:00:00 2001 From: Kieran Klukas Date: Sat, 19 Sep 2026 12:46:47 -0400 Subject: [PATCH 3/4] feat: send the versioned frame, and report the output pins Follows the wire format change in SensorHub: the frame now carries a version and a length, and the payload has room to grow. The fan and pump pins have been configured as outputs since this was written and their state appeared nowhere in telemetry, so nobody could see or log what the car was doing to itself. They are now reported alongside the inputs. Also adds the sequence number the receiver needs to notice a packet never arrived, and names the output pins so the pinMode calls and the packet cannot drift apart. Costs 292 bytes of flash, taking the sketch from 18% to 19% of a Nano. --- carsensordriver.ino | 73 ++++++++++++++++++++++++++++++++------------- 1 file changed, 53 insertions(+), 20 deletions(-) diff --git a/carsensordriver.ino b/carsensordriver.ino index 9e13b35..d4c7e15 100644 --- a/carsensordriver.ino +++ b/carsensordriver.ino @@ -1,5 +1,6 @@ #include #include +#include /* * The packet layout, the checksum, and the frame encoder live in the @@ -20,6 +21,15 @@ /* Keep the local spelling so the call sites below read unchanged. */ typedef sh_packet_t DataPacket; +/* Output pins, named so the packet's output channels and the pinMode calls + below cannot drift apart. */ +#define RAD_FAN_PIN 10 +#define WATER_PUMP_PIN 11 +#define SPARE_OUT_PIN 12 + +/* Wraps at 65535; the receiver counts gaps in it as dropped packets. */ +static uint16_t sequence = 0; + //wheel speed constants #define wheelSpeedSensorPin 2 #define numMagnets 1 @@ -49,13 +59,18 @@ const int cacheTTL[] = {50, 50}; int cacheLife[] = {0, 0}; void sendPacket(const DataPacket &packet) { - /* One buffered write rather than four: header, payload, and checksum are - assembled by the shared encoder, so the Pi's decoder and this cannot - disagree about the format. */ + /* One buffered write rather than four: header, version, length, payload, + and checksum are all assembled by the shared encoder, so the Pi's + decoder and this cannot disagree about the format. */ uint8_t frame[SH_FRAME_SIZE]; + size_t written = 0; - sh_encode_frame(&packet, frame); - Serial.write(frame, sizeof(frame)); + if (sh_encode_frame(&packet, frame, sizeof(frame), &written) != SH_OK) { + return; /* cannot happen with a correctly sized buffer, but do not + transmit a half-built frame if it ever does */ + } + + Serial.write(frame, written); } void setup() { @@ -68,9 +83,9 @@ void setup() { pinMode(6, INPUT); pinMode(7, INPUT); pinMode(8, INPUT); - pinMode(10, OUTPUT); // Rad Fan Signal Out - pinMode(11, OUTPUT); // Water Pump Signal Out - pinMode(12, OUTPUT); + pinMode(RAD_FAN_PIN, OUTPUT); // Rad Fan Signal Out + pinMode(WATER_PUMP_PIN, OUTPUT); // Water Pump Signal Out + pinMode(SPARE_OUT_PIN, OUTPUT); pinMode(A7, INPUT); pinMode(A2, INPUT); //Attach the A2 pin on the arduino to the OUT pin on the airspeed module @@ -89,18 +104,36 @@ void loop() { float engTemp = updateEngineTemp(); float radTemp = updateRadiatorTemp(); - DataPacket packet = { - speed, - (float)analogRead(A2) - airOffset, - engTemp, - radTemp, - (uint8_t)digitalRead(4), - (uint8_t)digitalRead(5), - (uint8_t)digitalRead(6), - (uint8_t)digitalRead(7), - (uint8_t)digitalRead(8), - (uint16_t)analogRead(A7) - }; + DataPacket packet; + memset(&packet, 0, sizeof(packet)); + + packet.speed = speed; + packet.airspeed = (float)analogRead(A2) - airOffset; + + packet.temps[SH_TEMP_ENGINE] = engTemp; + packet.temps[SH_TEMP_RADIATOR] = radTemp; + // temps[2] and temps[3] are spare. Adding a third probe to the same + // DS18B20 bus is now a firmware change, not a wire format change. + + packet.analog[SH_ANALOG_BATTERY] = (uint16_t)analogRead(A7); + // analog[1..3] spare, for the next sensor that needs an ADC pin. + + sh_set_digital_in(&packet, 0, digitalRead(4)); + sh_set_digital_in(&packet, 1, digitalRead(5)); + sh_set_digital_in(&packet, 2, digitalRead(6)); + sh_set_digital_in(&packet, 3, digitalRead(7)); + sh_set_digital_in(&packet, 4, digitalRead(8)); + + // Report what we drive, not only what we read. The fan and pump pins + // appeared nowhere in telemetry before, so there was no way to see or log + // what the car was doing to itself. + sh_set_digital_out(&packet, 0, digitalRead(RAD_FAN_PIN)); + sh_set_digital_out(&packet, 1, digitalRead(WATER_PUMP_PIN)); + sh_set_digital_out(&packet, 2, digitalRead(SPARE_OUT_PIN)); + + // Wraps at 65535, which the receiver expects. Gaps here are the only way + // the Pi can know a packet never arrived; a checksum cannot tell it. + packet.sequence = sequence++; if (speed > 0.25f || speed == 0.0f) { sendPacket(packet); From 8997be0cf9a65b03bdfc6386875a06e873441035 Mon Sep 17 00:00:00 2001 From: Kieran Klukas Date: Sat, 19 Sep 2026 12:49:26 -0400 Subject: [PATCH 4/4] ci: make a failed compile fail the compile step Without pipefail, tee's exit status hid a broken build and the error turned up two steps later as an empty grep. Also checks that the SensorHub checkout actually contains a usable library. --- .github/workflows/ci.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66e08ec..7a11ff5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,9 +42,21 @@ jobs: git clone --depth 1 https://github.com/HEEV/SensorHub.git \ "$(arduino-cli config get directories.user)/libraries/SensorHub" } + # a clone can succeed while producing something Arduino cannot use, + # so check for the entry point rather than trusting the exit status + lib="$(arduino-cli config get directories.user)/libraries/SensorHub" + if [ ! -f "$lib/src/SensorHub.h" ]; then + echo "SensorHub is present but has no src/SensorHub.h." + echo "If HEEV/SensorHub#2 has not merged yet, that is expected:" + echo "main does not carry the library layout until it lands." + exit 1 + fi - name: Compile run: | + # pipefail, or tee's exit status hides a failed compile and the + # error surfaces two steps later as a confusing empty grep + set -o pipefail arduino-cli compile \ --fqbn arduino:avr:nano:cpu=atmega328 \ --warnings all \