blog.6admin.io
guest@6admin.io:~$
arduino, c, ci-cd, led, pedagogy

Autopsy of an inherited Arduino firmware

Back at a former employer, the original firmware drove little LED strips bolted outside meeting rooms, showing the room’s availability. The board polls GET /lights/<mac> every few seconds, the server answers a single ASCII digit between <0> and <9>, the strip shows a color. Simple product, simple protocol. I did not write it, but I used it as a base.

The board was only half of the system. The other half, on the meeting-room side, was the building’s occupancy server. I, on my own, repurposed that same protocol, for myself alone, with a small Python server bridging my CI pipelines to the board: my CI status light existed in 2019, long before “CI status lights” were a thing. Only the API contract survived, in the docs (GET /lights/<lightid> answering <N>, from <0> rainbow error to <4> red booked-and-busy); the server code itself did not. That is its own lesson: code without a repository does not outlive the job that wrote it.

The code is another matter. I kept a copy of the original sketch (lights.c, 475 lines, identical in three different folders on three different machines, zero version control discipline) and since I have permission to show it, here it is dissected, less to judge a predecessor than to draw reusable lessons from it.

Config for “non-technical people”, security not included

byte serverIP[] = {  198,  51,  100,  23 };  // building server's IP(v4) address
byte forced_mac[] = { 0x00, 0x24, 0x67, 0x3E, 0xFB, 0x8E };

A public server IP and a MAC address, hardcoded in the source, shipped in the “this part contains what non-technical people can change” section. The section also declares use_proxy as TO BE TESTED and two features as TO BE DONE. Every board in the fleet compiled its own copy of the same secrets.

Let me be fair about the fix, because it is not the obvious one. Moving config to runtime was not realistic on a board this constrained and some client sites had to be integrated without DHCP or DNS anyway, on static addressing. What should have happened is not embedding the config in the board but pulling the values out of the source: placeholders in the sketch and a script, or an Ansible role, that renders the .ino with the right values before compiling, the repo keeping the template and not the secrets. A generic bootstrap that fetches its config, yes, but resolved at build time rather than at runtime.

The color table and GRB order

Before anything else, the constants file:

uint32_t red      = strip.Color(  0, 162,   0);  // red
uint32_t darkblue1 = strip.Color(  0,  0,   32); // red

Twelve color constants, six of them commented // red, including the blues. This is the NeoPixel GRB byte order doing its usual damage and nobody ever fixed the comments. truewhite is Color(65, 72, 32): a yellowish tint we sold to the client as “33% white”. And LEDCOUNT 28 carries the comment // 35 for epic failure. Accurate documentation, for once.

The reset left to the watchdog

void RESET() {
   digitalWrite(7, LOW);      // a wire is soldered between pin 7 and RST, on OLD boards
   wdt_enable(WDTO_4S);
   while(1){}                 // wait for it
}

The reset function first shorts pin 7 to the RST pin through a literal soldered wire on old boards, then arms a 4-second watchdog and spins in an infinite loop, waiting for the watchdog to shoot the board. A reset whose API is “die and hope”. The comment // wait for it is doing a lot of work.

Recursion as a retry

When the HTTP connect fails, GET() retries itself recursively, with a growing sleep:

error_count++;
int patience = error_count * 1000;
delay(patience);
return GET();

Eleven levels of recursion with blocking delays, on an AVR with 2.5 KB of RAM, each level holding a stack frame plus a String. It works until the network blinks for a minute and then it either overflows or recovers by accident. There is even an inline apology: at some point there was a "while" instead of an "if".

Twenty seconds without listening

Every success path ends with delay(show_delay * 1000); with show_delay = 20. The board polls once, shows a color, then goes completely deaf for 20 seconds. The meeting room occupancy changes; the light outside the door does not care. Multiply by every case of a ten-way switch, plus a blocking 15-second timeout loop in the reader, plus while (!Serial) at boot: the board spends most of its life not listening.

Returning a local buffer

char inString[32];
...
return inString;   // return the char array with what was enclosed between '<' and '>'

readPage() returns a pointer to a local buffer. The caller stores it in a String fast enough that it usually works, which is the worst kind of bug: the one that works. And if the 15-second timeout expires, the function falls off the end without returning anything: undefined behavior, courtesy of a commented-out safety branch.

String on AVR and heap fragmentation

Every HTTP request is built with request = request + "/lights/"; request = request + macbuffer + .... On AVR, String is heap-allocated and this pattern fragments 2.5 KB of RAM into confetti. This is the single most common Arduino disease of the decade and the fix (fixed buffers, snprintf) was known and ignored.

And it was not theoretical: RAM turned to confetti, the board leaked and fell over at random, a reboot here, a frozen LED strip there, never twice the same symptom, the kind of intermittent failure you never reproduce the day you go looking for it.

Serial, the second display

More than 40 Serial.print calls, including one printing every received character of the HTTP response (Serial.print(c); inline in the read loop), plus a Debug() timestamp after almost every step. With while (!Serial) blocking boot until a terminal opens. The light is a status display for humans; its own status display was the only thing it reliably produced.

The next refactor attempt

The refactor attempt (sketch_nov04a) rebooted via a null function pointer (void (*softReset) (void) = 0; softReset();, the classic undefined-behavior trick), parsed the digit with case 48: through case 57: (ASCII magic numbers, because why use c - '0') and contained the gem case 55: // 7 = blue which sets Color(0,255,0) and comments it // black ! - OFF. The 2019 fleet variant (sketch_3-0_20190328_.ino) kept the recursive GET, the watchdog hang and the dangling return, added EEPROM MAC support and booted with Serial.println("No more Lucky Rainbow !");. Same disease, one more decade of sediment.

Three copies of the same sketch lived on three machines with no shared history. The version control was “whichever folder is on the USB stick”. If you recognize yourself: the fix is git init and it takes one minute.

475 lines of what not to do

  • Blocking delays are lies you tell your scheduler. Nothing else can happen during them; make the loop non-blocking from day one.
  • Never return a local buffer. String, char[], pointer, whatever: the caller owns nothing and you own nothing.
  • Recursion is not a retry loop. On a microcontroller it is a stack overflow with extra steps.
  • The watchdog is for hangs, not a reset API. Rebooting by hanging on purpose works, but it means your reset depends on your failure mode.
  • Hardcoded fleet secrets in source compile into every board. When the board is too constrained for runtime config, the answer is a placeholder template rendered at build time by a script or Ansible, the repo keeping the model and not the values.
  • A single 475-line file does not stay maintainable. Splitting it into several .ino by responsibility is the first move, before fixing a single bug.
  • Comments lie when the protocol confuses people. GRB is not RGB; fix the constant, not the comment.

The redemption story, from this swamp to a non-blocking firmware and a CI pipeline that lights the LEDs, is told in two later episodes: the rewrite and the CI light .