Rewrite Nano serial parser as line based, fix protocol bugs

- Commands run on '\n' instead of busy-waiting for a byte count, which
  fixes s/F/T waiting for the next command and P drawing 0xFF garbage
- Invalid bitmaps no longer leak their data as commands; bitmap data is
  streamed so full-screen bitmaps still fit
- Store flip time as uint16 with magic marker, validate 50-2000 us
- Overflow-safe millis() handling for the periodic refresh
- Run the full refresh only after D or when serial is idle, so incoming
  data is not lost while the redraw blocks
- Put the boot refresh measurement behind MEASURE_REFRESH
- Fix DEBUG hex output and use F() strings to save RAM
- Driver: direct mode now updates the old buffer instead of re-setting
  the new one

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CLAUDE.md

@@ -31,13 +31,21 @@ See README.md for the overview.
   Add new secrets to `secrets.example.h` as placeholders.
 - Hardware is not available here. Builds can be checked, but runtime behaviour can't.
 
-## Known quirks in the serial protocol (Nano `src/nano/main.cpp`)
+## Nano serial parser (`src/nano/main.cpp`)
 
-- `T` waits for more than 6 bytes before parsing, but `T225\n` has only 4 after the `T`.
+- Line based and non-blocking: bytes are collected into a 64-character buffer and a
-  It completes only when the next command arrives.
+  command runs when `\n` arrives. Bitmap (`B`) pixel data is the exception. It is drawn
-- The `T` value is stored with `EEPROM.update` in one byte, so values above 255 µs are
+  while it arrives, so it can be longer than the buffer. Keep `loop()` free of busy-waits.
-  truncated after a reboot.
+- A full redraw (`display(true)`) blocks for about 0.5 s, which overflows the 64-byte
-- `P` (print text) busy-waits for a `\n` and has no timeout.
+  serial receive buffer. Only run it when no data is expected: right after a `D`, or after
+  2 s with no serial data. The ESP sends one frame per second, ending with `D`.
+- Flip time is stored in EEPROM as a magic word at address 0 and a `uint16_t` at
+  address 2. Values outside 50–2000 µs are rejected.
+- The Nano doesn't send an acknowledgement. Whether the Nano's TX is wired to the ESP's RX
+  is unknown, so don't make the ESP wait for replies.
+- To test the parser without hardware, compile `main.cpp` with g++ (Strawberry Perl
+  provides it at `C:\Strawberry\c\bin`) and mock headers for `Arduino.h`, `EEPROM.h` and
+  `BROSE9323.h`.
 
 ## Git
 

lib/BROSE9323/BROSE9323.cpp

@@ -151,22 +151,11 @@ void BROSE9323::drawPixel(int16_t x, int16_t y, uint16_t color)
 
     _strobe();
 #if !defined(__AVR_ATmega168P__) && !defined(__AVR_ATmega168PB__) && !defined(__AVR_ATmega168__)
+    // The dot is flipped now, so the next display() does not have to flip it again
     if (color)
-    {
+      _old_buffer[y * _buffer_width + x / 8] |= 1 << (x & 7);
-      if (_new_buffer[y * _buffer_width + x / 8] & (1 << (x & 7)))
-      {
-        return;
-      }
-      _new_buffer[y * _buffer_width + x / 8] |= 1 << (x & 7);
-    }
     else
-    {
+      _old_buffer[y * _buffer_width + x / 8] &= ~(1 << (x & 7));
-      if (~_new_buffer[y * _buffer_width + x / 8] & (1 << (x & 7)))
-      {
-        return;
-      }
-      _new_buffer[y * _buffer_width + x / 8] &= ~(1 << (x & 7));
-    }
 #endif
   }
 #endif

src/nano/README.md

@@ -8,18 +8,33 @@ logic.
 
 ## Behaviour
 
-- **Boot:** the Nano flips the whole display on and then off, then prints the time a full
+- **Boot:** the Nano loads the flip time from EEPROM and flips the whole display on and
-  redraw took.
+  then off.
 - **`display()`:** compares the new buffer with the last one that was shown and strobes
   only the dots that changed.
-- **Every 2 minutes** (`REFRESH_TIME`) it forces a full redraw to fix dots that got stuck.
+- **Every 2 minutes** (`REFRESH_TIME`) it does a full redraw to fix dots that got stuck.
-- **Flip pulse length** (µs) is set with the `T` command and stored in EEPROM, so it is
+  A full redraw blocks for about half a second, which is long enough to overflow the
-  kept after a reboot.
+  64-byte serial buffer. So it only runs right after a `D` (the end of a frame, when the
+  ESP is quiet) or when no serial data has arrived for 2 seconds.
+- **Flip pulse length** (µs, 50–2000) is set with the `T` command and stored in EEPROM,
+  so it is kept after a reboot. The default is 200 µs.
+
+### Debug options
+
+Uncomment these at the top of `main.cpp`:
+
+| Define | Effect |
+|--------|--------|
+| `DEBUG` | Print unknown command bytes on the serial port |
+| `MEASURE_REFRESH` | At boot, time a full redraw and print the duration in ms |
 
 ## Serial protocol
 
-Each command is one ASCII line. Coordinates are 2-digit **hex**. Color is `0` (black) or
+Each command is one ASCII line that **ends with `\n`** (`\r` is ignored). The Nano
-`1` (yellow). The ESP32 driver (`ESPSERIAL`) only uses `s`, `S`, `F`, `D` and `T`.
+collects a whole line before it runs the command. If a line is broken or unknown, it is
+skipped and the next line starts clean. Coordinates are 2-digit **hex**. Color is `0`
+(black) or `1` (yellow). The ESP32 driver (`ESPSERIAL`) only uses `s`, `S`, `F`, `D` and
+`T`.
 
 | Cmd | Format | Meaning |
 |-----|--------|---------|
@@ -27,10 +42,12 @@ Each command is one ASCII line. Coordinates are 2-digit **hex**. Color is `0` (b
 | `S` | `SXXYYc` | Set pixel and flip it right away (direct mode) |
 | `F` | `Fc` | Fill the whole buffer |
 | `D` | `D` | Push the buffer to the display (changed dots only) |
-| `P` | `PXXYYcsTEXT` | Print text at (XX,YY), color `c`, size `s`. `\n` in TEXT starts a new line. |
+| `P` | `PXXYYcsTEXT` | Print text at (XX,YY), color `c`, size `s`. A literal `\n` (backslash, n) in TEXT starts a new line. The whole line can be at most 64 characters. |
-| `B` | `BXXYYWWHHcHEX...` | Draw a bitmap. WW must be a multiple of 4. Pixel data is one hex digit per 4 pixels. |
+| `B` | `BXXYYWWHHcHEX...` | Draw a bitmap. WW must be a multiple of 4. Pixel data is one hex digit per 4 pixels, row by row. The data is drawn while it arrives, so it is not limited to 64 characters. |
 | `R` | `R` | Print the buffer as ASCII art on the serial port |
-| `T` | `Tnnn` | Set the flip pulse length in µs (decimal), stored in EEPROM |
+| `T` | `Tnnn` | Set the flip pulse length in µs (decimal, 50–2000), stored in EEPROM |
+
+Invalid commands get a reply that starts with `ERR:` on the Nano's TX pin.
 
 Example (what the ESP32 sends for one pixel and a refresh):
 

src/nano/main.cpp

@@ -6,40 +6,42 @@
 
 #include <EEPROM.h>
 
-BROSE9323 display(84, 19, 28, 200);
-
-uint32_t refresh_at;
-const uint32_t REFRESH_TIME = 120000L; // 600000L // 10 min
-
-uint16_t _flip_time = 200;
-unsigned int address  = 0;
-unsigned int address2 = 1;
-
 //#define DEBUG
-void setup()
+// Print how long a full redraw takes (in ms) at startup
-{
+//#define MEASURE_REFRESH
-	Serial.begin(115200);
-	Serial.println("Hello World!");
 
-	uint16_t read1 = EEPROM.read(address);
+// Flip pulse length in us, stored in EEPROM and changed with the T command
-	uint16_t read2 = EEPROM.read(address2);
+const uint16_t FLIP_TIME_DEFAULT = 200;
+const uint16_t FLIP_TIME_MIN = 50;
+const uint16_t FLIP_TIME_MAX = 2000;
 
-	if(read1 == read2)
+// EEPROM layout: magic marker (marks the stored value as valid), then the flip time
-		_flip_time = read1;
+const int EEPROM_MAGIC_ADDR = 0;
+const int EEPROM_FLIP_TIME_ADDR = 2;
+const uint16_t EEPROM_MAGIC = 0xF11D;
 
-	display.setTiming(_flip_time);
+// Full redraw interval, fixes dots that did not flip
+const uint32_t REFRESH_TIME = 120000UL; // 2 min
+// Serial line quiet for this long: safe to do a due full redraw without waiting for a D
+const uint32_t IDLE_TIME = 2000UL;
 
-	display.begin();
+// Longest command line (bitmap data is streamed and does not count)
-	delay(1000);
+const uint8_t LINE_MAX = 64;
 
-	display.fillScreen(1);
+BROSE9323 display(84, 19, 28, FLIP_TIME_DEFAULT);
-	display.display();
+
-	display.fillScreen(0);
+char line[LINE_MAX + 1];
-	uint32_t t1 = millis();
+uint8_t line_len = 0;
-	display.display();
+bool discard_line = false; // skip everything until the next '\n'
-	uint32_t t2 = millis();
+
-	Serial.println(t2 - t1);
+// Bitmap (B) pixel data is drawn while it arrives instead of being buffered
-}
+bool bitmap_active = false;
+uint8_t bitmap_x, bitmap_y, bitmap_w, bitmap_h;
+bool bitmap_color;
+uint16_t bitmap_nibble;
+
+uint32_t last_refresh;
+uint32_t last_rx;
 
 uint8_t hex2dec(char c)
 {
@@ -52,182 +54,263 @@ uint8_t hex2dec(char c)
 	return 0;
 }
 
-void loop()
+bool isHex(char c)
 {
-	if (Serial.available())
+	return (c >= '0' && c <= '9') || (c >= 'A' && c <= 'F') || (c >= 'a' && c <= 'f');
+}
+
+uint8_t hex2byte(const char *s)
+{
+	return (hex2dec(s[0]) << 4) | hex2dec(s[1]);
+}
+
+void printError(const __FlashStringHelper *msg)
+{
+	Serial.print(F("ERR: "));
+	Serial.println(msg);
+}
+
+uint16_t loadFlipTime()
+{
+	uint16_t magic, t;
+	EEPROM.get(EEPROM_MAGIC_ADDR, magic);
+	EEPROM.get(EEPROM_FLIP_TIME_ADDR, t);
+	if (magic != EEPROM_MAGIC || t < FLIP_TIME_MIN || t > FLIP_TIME_MAX)
+		return FLIP_TIME_DEFAULT;
+	return t;
+}
+
+void saveFlipTime(uint16_t t)
+{
+	// put() only writes bytes that changed
+	EEPROM.put(EEPROM_FLIP_TIME_ADDR, t);
+	EEPROM.put(EEPROM_MAGIC_ADDR, EEPROM_MAGIC);
+}
+
+// Force a full redraw every REFRESH_TIME. Only called when no data is expected,
+// because the redraw blocks long enough to overflow the 64 byte serial buffer.
+void refreshIfDue()
+{
+	if (millis() - last_refresh >= REFRESH_TIME)
 	{
-		char cmd = Serial.read();
+		display.display(true);
+		last_refresh = millis();
+	}
+}
+
+// BxxyywwhhcBITMAP (xx/ww = 8 bit hex x position/width; yy/hh = 8 bit hex y position/height;
+// c = 0/1 color; BITMAP = one hex digit per 4 pixels, row by row, width must be a multiple of 4)
+bool startBitmap()
+{
+	bitmap_x = hex2byte(line + 1);
+	bitmap_y = hex2byte(line + 3);
+	bitmap_w = hex2byte(line + 5);
+	bitmap_h = hex2byte(line + 7);
+	bitmap_color = line[9] != '0';
+	if (bitmap_w == 0 || bitmap_w % 4)
+	{
+		printError(F("Invalid bitmap width (not dividable by 4)"));
+		return false;
+	}
+	if (bitmap_h == 0)
+	{
+		printError(F("Invalid bitmap height (0)"));
+		return false;
+	}
+	bitmap_nibble = 0;
+	bitmap_active = true;
+	return true;
+}
+
+void bitmapData(char c)
+{
+	if (!isHex(c))
+		return;
+	uint16_t pixel = bitmap_nibble * 4;
+	if (pixel >= (uint16_t)bitmap_w * bitmap_h)
+		return; // more data than the bitmap size, ignore
+	uint8_t b = hex2dec(c);
+	for (uint8_t i = 0; i < 4; i++)
+	{
+		if (b & (0x8 >> i))
+		{
+			uint16_t p = pixel + i;
+			display.drawPixel(bitmap_x + p % bitmap_w, bitmap_y + p / bitmap_w, bitmap_color);
+		}
+	}
+	bitmap_nibble++;
+}
+
+// Handle one complete command line (without the '\n')
+void processLine()
+{
+	if (line_len == 0)
+		return;
+	line[line_len] = '\0';
+
+	char cmd = line[0];
 	switch (cmd)
 	{
-		// Ignore CR
+	// Bitmap header shorter than 10 characters (complete headers are handled in loop())
-		case 0x0A:
-			break;
-		// Draw Bitmap (must be n x 8 wide)
 	case 'b':
-		case 'B': //BxxyywwhhcBITMAP (xx/ww = 8 bit hex x position/width; yy/hh = 8 bit y position/height; c = 0/1 color; BITMAP = bytes as hex)
+	case 'B':
-		{
+		printError(F("Incomplete bitmap header"));
-			// May lead to buffer overflow, you may need to segment the bitmap into multiple frames
-			// Or just use the ser pixel function
-			while (Serial.available() < 9)
-			{
-				delay(1);
-			}
-			uint8_t x = (hex2dec(Serial.read()) << 4) | hex2dec(Serial.read());
-			uint8_t y = (hex2dec(Serial.read()) << 4) | hex2dec(Serial.read());
-			uint8_t w = (hex2dec(Serial.read()) << 4) | hex2dec(Serial.read());
-			if (w % 4)
-			{
-				Serial.println("ERR: Invalid bitmap width (not dividable by 4)");
-				Serial.flush();
 		break;
-			}
-			uint8_t h = (hex2dec(Serial.read()) << 4) | hex2dec(Serial.read());
-			if (h == 0)
-			{
-				Serial.println("ERR: Invalid bitmap height (0)");
-				Serial.flush();
-				break;
-			}
-			uint8_t c = Serial.read() - '0';
-			uint8_t b = 0;
-			for (uint8_t _y = 0; _y < h; _y++)
-			{
-				for (uint8_t _x = 0; _x < w; _x++)
-				{
-					if ((_x & 0x3) == 0)
-					{
-						if (!Serial.available())
-						{
-							Serial.println("WARN: OUTOFMEM");
-						}
-						while (!Serial.available())
-						{
-							delay(1);
-						}
-						b = hex2dec(Serial.read());
-					}
-					if (b & (1 << (0x3 - (_x & 0x3))))
-					{
-						display.drawPixel(x + _x, y + _y, c);
-					}
-				}
-			}
-			break;
-		}
 	// Display
 	case 'd':
-		case 'D': //D
+	case 'D': // D
-		{
 		display.display();
+		// The ESP sends D at the end of a frame, so the line is quiet now
+		refreshIfDue();
 		break;
-		}
 	// Fill Screen
 	case 'f':
-		case 'F': //Fc (c = 0/1 color)
+	case 'F': // Fc (c = 0/1 color)
+		if (line_len < 2)
 		{
-			while (Serial.available() <= 2)
+			printError(F("Missing color"));
-			{
-				delay(1);
-			}
-			display.fillScreen(Serial.read() - '0');
 			break;
 		}
+		display.fillScreen(line[1] != '0');
+		break;
 	// Print Text
 	case 'p':
-		case 'P': // PxxyycsTEXT (xx = 8 bit hex x position; yy = 8 bit y position; c = 0/1 color; s = 1/2 size; TEXT = string, newline = escaped \\n)
+	case 'P': // PxxyycsTEXT (xx = 8 bit hex x position; yy = 8 bit hex y position; c = 0/1 color; s = 1-9 size; TEXT = string, newline = escaped \n)
 	{
-			delay(5);
+		if (line_len < 7)
-			while (Serial.available() <= 6)
 		{
-				delay(1);
+			printError(F("Incomplete print command"));
-			}
-			uint8_t x = (hex2dec(Serial.read()) << 4) | hex2dec(Serial.read());
-			uint8_t y = (hex2dec(Serial.read()) << 4) | hex2dec(Serial.read());
-			uint8_t c = Serial.read() - '0';
-			display.setCursor(x, y);
-			display.setTextColor(c, !c);
-			display.setTextSize(Serial.read() - '0');
-			while (true)
-			{
-				if (!Serial.available())
-				{
-					delay(1);
-				}
-				if (Serial.peek() == '\n')
 			break;
-				if (Serial.peek() == '\r')
+		}
+		bool color = line[5] != '0';
+		display.setCursor(hex2byte(line + 1), hex2byte(line + 3));
+		display.setTextColor(color, !color);
+		display.setTextSize(line[6] - '0');
+		for (uint8_t i = 7; i < line_len; i++)
 		{
-					Serial.read();
+			if (line[i] == '\\' && line[i + 1] == 'n')
-					continue;
-				};
-				char c = Serial.read();
-				if (c == '\\' && Serial.peek() == 'n')
 			{
 				display.println();
-					Serial.read();
+				i++;
 			}
 			else
 			{
-					display.write(c);
+				display.write(line[i]);
 			}
 		}
-			return;
+		break;
 	}
 	// Return buffer (show pixels on console)
 	case 'r':
-		case 'R': //R
+	case 'R': // R
-		{
 		display.printBuffer();
 		Serial.flush();
 		break;
-		}
+	// Set Pixel (S = flip immediately, s = buffer only)
-		// Set Pixel
 	case 'S':
-			display.setDirect(true);
+	case 's': // Sxxyyc (xx = 8 bit hex x position; yy = 8 bit hex y position; c = 0/1 color)
-		case 's': // Sxxyyc (xx = 8 bit hex x position; y = 8 bit y position; c = 0/1 color)
+		if (line_len < 6)
 		{
-			while (Serial.available() <= 6)
+			printError(F("Incomplete pixel command"));
-			{
-				delay(1);
-			}
-			uint8_t x = (hex2dec(Serial.read()) << 4) | hex2dec(Serial.read());
-			uint8_t y = (hex2dec(Serial.read()) << 4) | hex2dec(Serial.read());
-			bool color = hex2dec(Serial.read());
-			display.drawPixel(x, y, color);
-			display.setDirect(false);
 			break;
 		}
+		display.setDirect(cmd == 'S');
+		display.drawPixel(hex2byte(line + 1), hex2byte(line + 3), hex2dec(line[5]));
+		display.setDirect(false);
+		break;
 	// Set Timing
 	case 'T':
-		case 't': // Tttt (ttt = decimal timing in us)
+	case 't': // Tttt (ttt = decimal flip time in us)
 	{
-			while (Serial.available() <= 6)
+		long t = atol(line + 1);
+		if (t < FLIP_TIME_MIN || t > FLIP_TIME_MAX)
 		{
-				delay(1);
+			printError(F("Flip time out of range (50-2000 us)"));
+			break;
 		}
-			uint16_t t = atoi(Serial.readStringUntil('\n').c_str());
-
-  			EEPROM.update(address, t);
-  			EEPROM.update(address2, t);
-
 		display.setTiming(t);
+		saveFlipTime(t);
 		break;
 	}
 	default:
 #ifdef DEBUG
 		Serial.print("0x");
 		Serial.print(cmd >> 4, HEX);
-			Serial.print(cmd & 3, HEX);
+		Serial.print(cmd & 0xF, HEX);
 		Serial.println(" - command not known.");
 		Serial.flush();
 #endif
 		break;
 	}
+}
+
+void setup()
+{
+	Serial.begin(115200);
+
+	display.setTiming(loadFlipTime());
+	display.begin();
+
+#ifdef MEASURE_REFRESH
+	delay(1000);
+
+	display.fillScreen(1);
+	display.display();
+	display.fillScreen(0);
+	uint32_t t1 = millis();
+	display.display();
+	uint32_t t2 = millis();
+	Serial.println(t2 - t1);
+#endif
+
+	last_refresh = millis();
+	last_rx = millis();
+}
+
+void loop()
+{
+	while (Serial.available())
+	{
+		char c = Serial.read();
+		last_rx = millis();
+
+		if (bitmap_active)
+		{
+			if (c == '\n')
+				bitmap_active = false;
+			else
+				bitmapData(c);
+			continue;
+		}
+
+		if (c == '\n')
+		{
+			if (!discard_line)
+				processLine();
+			line_len = 0;
+			discard_line = false;
+			continue;
 		}
-	// Refresh content every REFRESH_TIME (by default 2 min)
+		if (c == '\r' || discard_line)
-	if (refresh_at < millis())
+			continue;
+		if (line_len >= LINE_MAX)
 		{
-		refresh_at = millis() + REFRESH_TIME;
+			printError(F("Line too long"));
-		display.display(true);
+			discard_line = true;
+			continue;
+		}
+		line[line_len++] = c;
+
+		// Bitmap header complete: draw the pixel data as it arrives
+		if (line_len == 10 && (line[0] == 'b' || line[0] == 'B'))
+		{
+			line_len = 0;
+			if (!startBitmap())
+				discard_line = true;
+		}
 	}
+
+	// No frames coming in (e.g. ESP idle): do the full redraw now if it is due
+	if (millis() - last_rx >= IDLE_TIME)
+		refreshIfDue();
 }