From 7a28e343802ba40fd217c1a15fd48ddc58acff49 Mon Sep 17 00:00:00 2001 From: MangaValk Date: Thu, 22 Oct 2026 00:24:14 +0200 Subject: [PATCH] Fix dense images on the Nano and the off image in slow mode - Nano: B data is drawn fast enough for the serial line. A hex digit arrives every 87 us; with a division and four drawPixel() calls per digit a lit digit took about 105 us, so dense frames (all yellow, the panda bitmap) overflowed the 64 byte receive buffer: frames not shown or wrong, "Line too long", frame check repairs that failed the same way. Now the parser counts columns and rows and BROSE9323::drawDots() sets the 4 dots (about 20 us). - ESP32: Power off and the start of the night draw the off image also when slow mode had the power off already (it powers on for it, once) - Tests: drawDots() against drawPixel(), off image in slow mode - Version 2.19.3 Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 6 +++++ docs/how-it-works.md | 3 ++- lib/BROSE9323/BROSE9323.cpp | 19 ++++++++++++++ lib/BROSE9323/BROSE9323.h | 4 +++ src/esp32/config.h | 2 +- src/esp32/logic/display_controller.cpp | 8 ++++-- src/esp32/logic/display_controller.h | 3 +++ src/nano/README.md | 3 ++- src/nano/command_parser.cpp | 22 ++++++++--------- src/nano/command_parser.h | 2 +- test/mocks/BROSE9323.h | 8 ++++++ test/test_driver/test_main.cpp | 33 +++++++++++++++++++++++++ test/test_esp32/test_main.cpp | 34 ++++++++++++++++++++++++++ 13 files changed, 130 insertions(+), 17 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 31e26e0..75d9b10 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,6 +69,12 @@ See README.md for the overview. longer than about 5 ms overflows the 64-byte serial receive buffer; lost bytes shift the rest of a bitmap. That was the "picture moved up" bug. `test_driver` checks the time per `update()` call with a simulated panel. +- The `B` data path must also keep up with the line: a hex digit arrives every 87 µs. + `_bitmapData()` counts columns and rows (no divisions) and sets each digit's 4 dots with + `BROSE9323::drawDots()`. With `p % w`, `p / w` and four `drawPixel()` calls it took about + 105 µs per `F`, so dense images (all yellow, a full picture) lost bytes ("Line too long", + frame check mismatches). Check new per-character work with `avr-objdump` (the mock + clock in the tests doesn't count CPU time). - 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 replies `READY` (at boot), `ERR: ...` and `C xxxx` (checksum of its image, for diff --git a/docs/how-it-works.md b/docs/how-it-works.md index a87ed02..d377f7d 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -29,7 +29,8 @@ Schedule pages and rotation rules with "skip in slow mode" are left out of the rotation (none left in the block: the chosen page). - **23:31–08:00:** the screen is cleared (or shows the off image chosen in the web UI) - and power is cut after 30 seconds. + and power is cut after 30 seconds. When slow mode had the power off already, it is + switched on for this. The same goes for the Power switch. 3. **Home Assistant**: switches, dropdowns and sensors over MQTT, see [Home Assistant](home-assistant.md). 4. **Nano.** The Nano reads commands from the serial line and updates its frame buffer. diff --git a/lib/BROSE9323/BROSE9323.cpp b/lib/BROSE9323/BROSE9323.cpp index e1dcf0f..c951840 100644 --- a/lib/BROSE9323/BROSE9323.cpp +++ b/lib/BROSE9323/BROSE9323.cpp @@ -290,6 +290,25 @@ void BROSE9323::printBuffer(void) } } +void BROSE9323::drawDots(int16_t x, int16_t y, uint8_t bits, bool color) +{ + if (!(bits & 0xF) || y < 0 || y >= height()) + return; + // Never used in direct mode (B commands), so only the buffer changes + _drawing = true; + uint8_t *row = _new_buffer + y * _buffer_width; + for (uint8_t mask = 8; mask; mask >>= 1, x++) + { + if (!(bits & mask) || x < 0 || x >= width()) + continue; + uint8_t bit = 1 << (x & 7); + if (color) + row[x >> 3] |= bit; + else + row[x >> 3] &= ~bit; + } +} + void BROSE9323::_selectColumn(uint8_t col) { if (_active_col == col) diff --git a/lib/BROSE9323/BROSE9323.h b/lib/BROSE9323/BROSE9323.h index 9339f88..a87e8cb 100644 --- a/lib/BROSE9323/BROSE9323.h +++ b/lib/BROSE9323/BROSE9323.h @@ -154,6 +154,10 @@ public: bool refreshing(void) const { return _refreshing; } #endif #ifndef ESPSERIAL + // Draws the set bits of a hex digit of B data into the buffer (bit 3 at x, bit 0 at + // x + 3), like drawPixel() but fast enough for the serial port: dense B lines take + // about 23 us per dot with drawPixel(), and a digit arrives every 87 us + void drawDots(int16_t x, int16_t y, uint8_t bits, bool color); void printBuffer(void); #endif size_t print(const char *ifsh); diff --git a/src/esp32/config.h b/src/esp32/config.h index 0cf2887..73e50fd 100644 --- a/src/esp32/config.h +++ b/src/esp32/config.h @@ -118,7 +118,7 @@ const uint32_t SETUP_AP_DELAY = 20000; // open the AP when WiFi fails thi const uint32_t WIFI_CONNECT_TIMEOUT = 30000; const uint32_t WIFI_RESTART_TIME = 10UL * 60 * 1000; -#define FIRMWARE_VERSION "2.19.2" +#define FIRMWARE_VERSION "2.19.3" // Home Assistant device id (do not change: Home Assistant's entities are linked to it) const uint8_t HA_DEVICE_ID[] = {0x34, 0x86, 0x5d, 0x3a, 0x08, 0x28}; diff --git a/src/esp32/logic/display_controller.cpp b/src/esp32/logic/display_controller.cpp index 7e73fdf..c0c7a26 100644 --- a/src/esp32/logic/display_controller.cpp +++ b/src/esp32/logic/display_controller.cpp @@ -103,8 +103,9 @@ void DisplayController::update(uint32_t now, bool time_valid, const LocalTime &t if (!_power_enabled) { - if (_power.isOn()) + if (_power.isOn() || _off_needed) { + _ensureOn(); _showOnce(Screen::Off); _off_timer.request(now, CLEAR_BEFORE_OFF_TIME + _effect_time); } @@ -129,8 +130,9 @@ void DisplayController::update(uint32_t now, bool time_valid, const LocalTime &t case ScheduleMode::Night: _slow_last = -1; // the morning starts with an update _slow_keep = false; - if (_power.isOn()) + if (_power.isOn() || _off_needed) { + _ensureOn(); _showOnce(Screen::Off); _off_timer.request(now, POWER_OFF_DELAY); } @@ -209,6 +211,7 @@ void DisplayController::_showDynamic(uint32_t now, const LocalTime &time, bool n _dynamic_due = now + wait; } _screen = screen; + _off_needed = true; _shown_hour = time.hour; _shown_minute = time.minute; _shown_revision = _content_revision; @@ -232,5 +235,6 @@ void DisplayController::_showOnce(Screen screen) break; } _screen = screen; + _off_needed = screen != Screen::Off; _shown_revision = _content_revision; } diff --git a/src/esp32/logic/display_controller.h b/src/esp32/logic/display_controller.h index 7e66d37..14b94fc 100644 --- a/src/esp32/logic/display_controller.h +++ b/src/esp32/logic/display_controller.h @@ -109,6 +109,9 @@ private: bool _power_enabled = true; Screen _screen = Screen::Unknown; + // The dots show a frame other than the off screen, also while the power is off (slow + // mode): Power off and night power on to draw the off screen + bool _off_needed = false; int _shown_hour = -1; int _shown_minute = -1; uint32_t _dynamic_due = 0; // millis() when a moving dynamic page is drawn again diff --git a/src/nano/README.md b/src/nano/README.md index 3f47543..71e8a52 100644 --- a/src/nano/README.md +++ b/src/nano/README.md @@ -49,7 +49,8 @@ Arduino core, EEPROM and the display driver are replaced by fakes in - **Nothing blocks.** Every frame (also instant ones, `D1`) is flipped from `update()` in slices of about 1 ms, so `loop()` keeps reading the serial port. A blocking redraw used to overflow the 64-byte receive buffer; the lost bytes shifted the rest of a bitmap - (the picture moved up and left) and nothing repaired it. + (the picture moved up and left) and nothing repaired it. Bitmap data is drawn as fast as + it arrives too (one hex digit every 87 µs); dense images used to be slower than that. - **Every 2 minutes** (`I` command, stored in EEPROM, 0 = never) it flips every dot once more, with one pulse, to fix dots that got stuck. This also runs in slices, only while no frame is being received or shown, and continues where it was after a frame. diff --git a/src/nano/command_parser.cpp b/src/nano/command_parser.cpp index 974e402..f7761c5 100644 --- a/src/nano/command_parser.cpp +++ b/src/nano/command_parser.cpp @@ -182,28 +182,28 @@ bool CommandParser::_bitmapStart() _error(F("Invalid bitmap height (0)")); return false; } - _bitmap_nibble = 0; + _bitmap_col = 0; + _bitmap_row = 0; _bitmap_active = true; return true; } +// Runs for every hex digit while the line arrives (one every 87 us at 115200 baud), so it +// must be quick: no divisions, and drawDots() instead of four drawPixel() calls. Slower +// than that, a dense line (all yellow) overflowed the 64 byte receive buffer. void CommandParser::_bitmapData(char c) { if (!isHex(c)) return; - uint16_t pixel = _bitmap_nibble * 4; - if (pixel >= (uint16_t)_bitmap_w * _bitmap_h) + if (_bitmap_row >= _bitmap_h) return; // more data than the bitmap size, ignore - uint8_t b = hex2dec(c); - for (uint8_t i = 0; i < 4; i++) + _display.drawDots(_bitmap_x + _bitmap_col, _bitmap_y + _bitmap_row, hex2dec(c), _bitmap_color); + _bitmap_col += 4; + if (_bitmap_col >= _bitmap_w) { - if (b & (0x8 >> i)) - { - uint16_t p = pixel + i; - _display.drawPixel(_bitmap_x + p % _bitmap_w, _bitmap_y + p / _bitmap_w, _bitmap_color); - } + _bitmap_col = 0; + _bitmap_row++; } - _bitmap_nibble++; } // Fc (c = 0/1 color) diff --git a/src/nano/command_parser.h b/src/nano/command_parser.h index ebc77b8..e0b8269 100644 --- a/src/nano/command_parser.h +++ b/src/nano/command_parser.h @@ -33,7 +33,7 @@ private: bool _bitmap_active = false; uint8_t _bitmap_x, _bitmap_y, _bitmap_w, _bitmap_h; bool _bitmap_color; - uint16_t _bitmap_nibble; + uint8_t _bitmap_col, _bitmap_row; // where the next hex digit goes, in the bitmap void _runLine(); void _error(const __FlashStringHelper *msg); diff --git a/test/mocks/BROSE9323.h b/test/mocks/BROSE9323.h index a91cd91..b7a90f6 100644 --- a/test/mocks/BROSE9323.h +++ b/test/mocks/BROSE9323.h @@ -27,6 +27,14 @@ public: void begin() { log("begin"); } void display(bool force = false, bool instant = false) { log("display", force, instant); } void drawPixel(int16_t x, int16_t y, uint16_t color) { log(direct ? "directPixel" : "pixel", x, y, color); } + // Recorded as the pixel calls it stands for (the real one is tested in test_driver) + void drawDots(int16_t x, int16_t y, uint8_t bits, bool color) + { + log("dots", x, y, bits); + for (int i = 0; i < 4; i++) + if (bits & (8 >> i)) + drawPixel(x + i, y, color); + } void fillScreen(uint16_t color) { log("fill", color); } void setDirect(bool d) { direct = d; } void setTiming(uint16_t t) { log("timing", t); } diff --git a/test/test_driver/test_main.cpp b/test/test_driver/test_main.cpp index 07b88bb..faace78 100644 --- a/test/test_driver/test_main.cpp +++ b/test/test_driver/test_main.cpp @@ -226,6 +226,38 @@ void test_crc_of_equal_images_is_equal() delete b; } +// The fast path for B data must give the same image as drawPixel(), also at the right edge +// (B widths are rounded up to 4, so x + 3 can be past it) and below the last row +void test_draw_dots_is_like_draw_pixel() +{ + BROSE9323 *a = make(), *b = make(); + a->fillScreen(1); + b->fillScreen(1); + for (int y = 0; y <= H; y++) + for (int x = 0; x < W; x += 4) + { + uint8_t bits = (x * 5 + y * 3) & 0xF; + bool color = (x / 4 + y) % 3 == 0; + a->drawDots(x + 2, y, bits, color); + for (int i = 0; i < 4; i++) + if (bits & (8 >> i)) + b->drawPixel(x + 2 + i, y, color); + } + TEST_ASSERT_EQUAL_HEX16(b->crc(), a->crc()); + for (int x = 0; x < W; x++) + for (int y = 0; y < H; y++) + { + a->drawPixel(x, y, 0); + b->drawPixel(x, y, 0); + } + a->drawDots(-2, 0, 0xF, 1); // dots 0 and 1 of the first row + b->drawPixel(0, 0, 1); + b->drawPixel(1, 0, 1); + TEST_ASSERT_EQUAL_HEX16(b->crc(), a->crc()); + delete a; + delete b; +} + int main() { UNITY_BEGIN(); @@ -237,5 +269,6 @@ int main() RUN_TEST(test_refresh_waits_for_a_running_effect); RUN_TEST(test_interrupted_first_frame_still_reaches_every_dot); RUN_TEST(test_crc_of_equal_images_is_equal); + RUN_TEST(test_draw_dots_is_like_draw_pixel); return UNITY_END(); } diff --git a/test/test_esp32/test_main.cpp b/test/test_esp32/test_main.cpp index 3a42476..5854bbd 100644 --- a/test/test_esp32/test_main.cpp +++ b/test/test_esp32/test_main.cpp @@ -583,6 +583,38 @@ void test_power_switch_off_keeps_display_off_during_the_day() TEST_ASSERT_EQUAL(0, power.on_count); } +// Slow mode had the power off already: the dots still show the clock, so the power goes on +// for the off image, once +void test_power_switch_off_in_slow_mode_draws_the_off_image() +{ + slowMode(true); + run(12, 0, POWER_OFF_DELAY + 200); + TEST_ASSERT_FALSE(power.is_on); + controller->setPowerEnabled(false); + run(12, 1); + TEST_ASSERT_TRUE(power.is_on); + TEST_ASSERT_EQUAL_STRING("off", frames.last().c_str()); + run(12, 1, CLEAR_BEFORE_OFF_TIME + 200); + TEST_ASSERT_FALSE(power.is_on); + run(12, 2, 60000); + TEST_ASSERT_EQUAL(2, power.on_count); + TEST_ASSERT_EQUAL(2, (int)frames.shown.size()); +} + +void test_night_in_slow_mode_draws_the_off_image() +{ + slowMode(true); + run(23, 15, POWER_OFF_DELAY + 200); + TEST_ASSERT_FALSE(power.is_on); + run(23, 31); + TEST_ASSERT_TRUE(power.is_on); + TEST_ASSERT_EQUAL_STRING("off", frames.last().c_str()); + run(23, 31, POWER_OFF_DELAY + 200); + TEST_ASSERT_FALSE(power.is_on); + run(3, 0, 60000); + TEST_ASSERT_EQUAL(2, power.on_count); +} + void test_power_switch_on_again_shows_clock() { run(12, 0); @@ -3270,6 +3302,8 @@ int main() RUN_TEST(test_power_switch_off_clears_and_powers_off_quickly); RUN_TEST(test_power_switch_off_waits_for_the_refresh_effect); RUN_TEST(test_power_switch_off_keeps_display_off_during_the_day); + RUN_TEST(test_power_switch_off_in_slow_mode_draws_the_off_image); + RUN_TEST(test_night_in_slow_mode_draws_the_off_image); RUN_TEST(test_power_switch_on_again_shows_clock); RUN_TEST(test_redraws_after_power_was_switched_off_elsewhere); RUN_TEST(test_no_wifi_is_shown_after_a_minute_without_wifi);