Share the packet format with the Pi instead of duplicating it

.github/workflows/ci.yml

@@ -0,0 +1,80 @@
+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
+        # 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
+        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"
+          }
+          # 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 \
+            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

carsensordriver.ino

@@ -1,8 +1,35 @@
 #include <DS18B20.h>
 #include <stdint.h>
+#include <string.h>
+
+/*
+ * 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 <SensorHub.h>
+
+/* 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;
 
-const uint8_t HEADER_1 = 0xAA;
-const uint8_t HEADER_2 = 0x55;
 //wheel speed constants
 #define wheelSpeedSensorPin 2
 #define numMagnets 1
@@ -31,43 +58,19 @@ 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, 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;
+
+  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(HEADER_1);
-  Serial.write(HEADER_2);
-  Serial.write(payload, sizeof(DataPacket));
-  Serial.write(checksum);
+  Serial.write(frame, written);
 }
 
 void setup() {
@@ -80,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
 
@@ -101,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);