diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..7a11ff5 --- /dev/null +++ b/.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 diff --git a/carsensordriver.ino b/carsensordriver.ino index 0065127..d4c7e15 100644 --- a/carsensordriver.ino +++ b/carsensordriver.ino @@ -1,8 +1,35 @@ #include #include +#include + +/* + * 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; + +/* 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);