diff --git a/src/graphics/draw/NotificationRenderer.cpp b/src/graphics/draw/NotificationRenderer.cpp index 9aa70fd81..d71ca6d08 100644 --- a/src/graphics/draw/NotificationRenderer.cpp +++ b/src/graphics/draw/NotificationRenderer.cpp @@ -84,7 +84,7 @@ static inline graphics::NotificationRenderer::BannerFont parseFontTagPrefix(cons { // Tags must be at the start of the line: // [S] small, [M] medium, [L] large - if (p && p[0] == '[' && p[2] == ']' && p[1] != '\0') { + if (p && p[0] == '[' && p[1] != '\0' && p[2] == ']') { char t = p[1]; if (t == 'S') { p += 3; @@ -136,6 +136,26 @@ static inline uint8_t effectiveLineHeightForBannerLine(graphics::NotificationRen return (height > 3) ? (height - 3) : height; } +const char *graphics::NotificationRenderer::resolveBannerLine(uint16_t lineIndex, const char *rawLine, BannerFont &lineFont) +{ + lineFont = BANNER_FONT_DEFAULT; + bool tagAware = (current_notification_type == notificationTypeEnum::text_banner || + current_notification_type == notificationTypeEnum::pairing_pin) && + alertBannerOptions == 0; + if (!tagAware) + return rawLine; + if (lineIndex < alertBannerLineCount) { + lineFont = alertBannerLineFonts[lineIndex]; + return alertBannerLines[lineIndex]; + } + // The parsed-line cache doesn't cover this line (the banner text was stored without a + // re-parse, or a draw raced the parse from another task): strip the tag here too, so it + // acts as a font change and never renders as literal text - the BLE pair PIN banner + // prefixes its PIN line with [M]. + lineFont = parseFontTagPrefix(rawLine); + return rawLine; +} + void graphics::NotificationRenderer::parseBannerMessageWithFonts(const char *message) { alertBannerLineCount = 0; @@ -845,9 +865,6 @@ void NotificationRenderer::drawNotificationBox(OLEDDisplay *display, OLEDDisplay BannerFont lineFonts[totalLines] = {}; uint8_t lineEffectiveHeights[totalLines] = {0}; const char *renderLines[totalLines] = {0}; - bool useTaggedBannerFonts = (current_notification_type == notificationTypeEnum::text_banner || - current_notification_type == notificationTypeEnum::pairing_pin) && - alertBannerOptions == 0; if (maxWidth != 0) is_picker = true; @@ -860,12 +877,8 @@ void NotificationRenderer::drawNotificationBox(OLEDDisplay *display, OLEDDisplay uint16_t widestLineWithBars = 0; while (lines[lineCount] != nullptr) { - const char *renderText = lines[lineCount]; BannerFont lineFont = BANNER_FONT_DEFAULT; - if (useTaggedBannerFonts && lineCount < alertBannerLineCount) { - renderText = alertBannerLines[lineCount]; - lineFont = alertBannerLineFonts[lineCount]; - } + const char *renderText = resolveBannerLine(lineCount, lines[lineCount], lineFont); renderLines[lineCount] = renderText; lineFonts[lineCount] = lineFont; lineEffectiveHeights[lineCount] = effectiveLineHeightForBannerLine(lineFont); @@ -879,10 +892,10 @@ void NotificationRenderer::drawNotificationBox(OLEDDisplay *display, OLEDDisplay if (current_notification_type == notificationTypeEnum::node_picker) { char measureBuffer[64] = {0}; - strncpy(measureBuffer, lines[lineCount], std::min(lineLengths[lineCount], sizeof(measureBuffer) - 1)); + strncpy(measureBuffer, renderText, std::min(lineLengths[lineCount], sizeof(measureBuffer) - 1)); lineWidths[lineCount] = UIRenderer::measureStringWithEmotes(display, measureBuffer); } else { - lineWidths[lineCount] = display->getStringWidth(lines[lineCount], lineLengths[lineCount], true); + lineWidths[lineCount] = display->getStringWidth(renderText, lineLengths[lineCount], true); } // Consider extra width for signal bars on lines that contain "Signal:" diff --git a/src/graphics/draw/NotificationRenderer.h b/src/graphics/draw/NotificationRenderer.h index 4febe07fc..d4b7781d5 100644 --- a/src/graphics/draw/NotificationRenderer.h +++ b/src/graphics/draw/NotificationRenderer.h @@ -38,6 +38,10 @@ class NotificationRenderer static uint8_t alertBannerLineCount; static BannerFont alertBannerLineFonts[MAX_LINES + 1]; static void parseBannerMessageWithFonts(const char *message); + // Decide what text and font a banner line actually renders with: parsed (tag-stripped) + // line if the cache covers it, otherwise the raw line with any leading font tag stripped + // on the fly. Exposed for unit tests. + static const char *resolveBannerLine(uint16_t lineIndex, const char *rawLine, BannerFont &lineFont); static void resetBanner(); static void drawBannercallback(OLEDDisplay *display, OLEDDisplayUiState *state); static void drawAlertBannerOverlay(OLEDDisplay *display, OLEDDisplayUiState *state); diff --git a/test/native-suite-count b/test/native-suite-count index c739b42c4..ea90ee319 100644 --- a/test/native-suite-count +++ b/test/native-suite-count @@ -1 +1 @@ -44 +45 diff --git a/test/test_banner_font_tags/test_main.cpp b/test/test_banner_font_tags/test_main.cpp new file mode 100644 index 000000000..4033c2785 --- /dev/null +++ b/test/test_banner_font_tags/test_main.cpp @@ -0,0 +1,176 @@ +// Regression tests for the alert-banner font-tag pipeline ([S]/[M]/[L] line prefixes). +// +// The BLE pairing banner (src/platform/nrf52/NRF52Bluetooth.cpp) sends +// "Bluetooth\nPIN\n[M]" with notification type pairing_pin. The [M] prefix is a +// font-change tag, never text: it must be stripped by parseBannerMessageWithFonts and, +// crucially, must also be stripped when a draw resolves a line the parsed cache doesn't +// cover (the shape of the historical bug where the pairing PIN rendered a literal "[M]"). +#include "MeshTypes.h" // Include BEFORE TestUtil.h (provides NodeNum, etc.) +#include "TestUtil.h" // initializeTestEnvironment() +#include + +#if HAS_SCREEN // Same guard as the module under test + +#include "graphics/draw/NotificationRenderer.h" +#include + +using graphics::NotificationRenderer; +using graphics::notificationTypeEnum; + +static const char *BLE_PIN_MESSAGE = "Bluetooth\nPIN\n[M]123 456"; + +// Reset every static the tests touch, so each case starts from a known state. +void setUp(void) +{ + NotificationRenderer::alertBannerMessage[0] = '\0'; + NotificationRenderer::parseBannerMessageWithFonts(""); + NotificationRenderer::alertBannerOptions = 0; + NotificationRenderer::current_notification_type = notificationTypeEnum::none; +} + +void tearDown(void) {} + +// Simulate Screen::showOverlayBanner storing and parsing a banner message. +static void showBanner(const char *message, notificationTypeEnum type, uint8_t options = 0) +{ + strncpy(NotificationRenderer::alertBannerMessage, message, 255); + NotificationRenderer::alertBannerMessage[255] = '\0'; + NotificationRenderer::parseBannerMessageWithFonts(NotificationRenderer::alertBannerMessage); + NotificationRenderer::alertBannerOptions = options; + NotificationRenderer::current_notification_type = type; +} + +// --- parseBannerMessageWithFonts --- + +void test_pairing_message_parses_and_strips_medium_tag() +{ + showBanner(BLE_PIN_MESSAGE, notificationTypeEnum::pairing_pin); + + TEST_ASSERT_EQUAL_UINT8(3, NotificationRenderer::alertBannerLineCount); + TEST_ASSERT_EQUAL_STRING("Bluetooth", NotificationRenderer::alertBannerLines[0]); + TEST_ASSERT_EQUAL_STRING("PIN", NotificationRenderer::alertBannerLines[1]); + TEST_ASSERT_EQUAL_STRING("123 456", NotificationRenderer::alertBannerLines[2]); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_DEFAULT, NotificationRenderer::alertBannerLineFonts[0]); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_DEFAULT, NotificationRenderer::alertBannerLineFonts[1]); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_MEDIUM, NotificationRenderer::alertBannerLineFonts[2]); +} + +void test_small_and_large_tags_parse() +{ + showBanner("[S]small\n[L]large", notificationTypeEnum::text_banner); + + TEST_ASSERT_EQUAL_UINT8(2, NotificationRenderer::alertBannerLineCount); + TEST_ASSERT_EQUAL_STRING("small", NotificationRenderer::alertBannerLines[0]); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_SMALL, NotificationRenderer::alertBannerLineFonts[0]); + TEST_ASSERT_EQUAL_STRING("large", NotificationRenderer::alertBannerLines[1]); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_LARGE, NotificationRenderer::alertBannerLineFonts[1]); +} + +void test_unknown_tag_is_kept_as_text() +{ + showBanner("[X]hello", notificationTypeEnum::text_banner); + + TEST_ASSERT_EQUAL_STRING("[X]hello", NotificationRenderer::alertBannerLines[0]); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_DEFAULT, NotificationRenderer::alertBannerLineFonts[0]); +} + +void test_tag_not_at_line_start_is_kept_as_text() +{ + showBanner("PIN [M]x", notificationTypeEnum::text_banner); + + TEST_ASSERT_EQUAL_STRING("PIN [M]x", NotificationRenderer::alertBannerLines[0]); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_DEFAULT, NotificationRenderer::alertBannerLineFonts[0]); +} + +void test_tag_only_line_yields_empty_text_with_font() +{ + showBanner("[L]", notificationTypeEnum::text_banner); + + TEST_ASSERT_EQUAL_STRING("", NotificationRenderer::alertBannerLines[0]); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_LARGE, NotificationRenderer::alertBannerLineFonts[0]); +} + +void test_lone_bracket_line_is_kept_as_text() +{ + showBanner("[", notificationTypeEnum::text_banner); + + TEST_ASSERT_EQUAL_STRING("[", NotificationRenderer::alertBannerLines[0]); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_DEFAULT, NotificationRenderer::alertBannerLineFonts[0]); +} + +// --- resolveBannerLine: what the draw code actually puts on the panel --- + +void test_resolve_uses_parsed_lines_for_pairing_pin() +{ + showBanner(BLE_PIN_MESSAGE, notificationTypeEnum::pairing_pin); + + NotificationRenderer::BannerFont font = NotificationRenderer::BANNER_FONT_DEFAULT; + const char *text = NotificationRenderer::resolveBannerLine(2, "[M]123 456", font); + TEST_ASSERT_EQUAL_STRING("123 456", text); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_MEDIUM, font); +} + +// The historical bug: the pairing banner drawn from the raw message, with the parsed-line +// cache not consulted (before the pairing_pin type was tag-aware) or not populated (a draw +// racing the parse from the BLE task). The tag must still act as a font change, not text. +void test_resolve_strips_tag_when_parsed_cache_missing() +{ + strncpy(NotificationRenderer::alertBannerMessage, BLE_PIN_MESSAGE, 255); + NotificationRenderer::current_notification_type = notificationTypeEnum::pairing_pin; + NotificationRenderer::alertBannerOptions = 0; + // Deliberately no parseBannerMessageWithFonts call: cache empty. + + NotificationRenderer::BannerFont font = NotificationRenderer::BANNER_FONT_DEFAULT; + const char *text = NotificationRenderer::resolveBannerLine(2, "[M]123 456", font); + TEST_ASSERT_EQUAL_STRING("123 456", text); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_MEDIUM, font); +} + +// Picker content can be user data (e.g. node names); it must never be tag-interpreted. +void test_resolve_leaves_picker_lines_untouched() +{ + NotificationRenderer::current_notification_type = notificationTypeEnum::node_picker; + NotificationRenderer::alertBannerOptions = 0; + + NotificationRenderer::BannerFont font = NotificationRenderer::BANNER_FONT_LARGE; + const char *text = NotificationRenderer::resolveBannerLine(0, "[M]allory", font); + TEST_ASSERT_EQUAL_STRING("[M]allory", text); + TEST_ASSERT_EQUAL(NotificationRenderer::BANNER_FONT_DEFAULT, font); +} + +void setup() +{ + initializeTestEnvironment(); + UNITY_BEGIN(); + + RUN_TEST(test_pairing_message_parses_and_strips_medium_tag); + RUN_TEST(test_small_and_large_tags_parse); + RUN_TEST(test_unknown_tag_is_kept_as_text); + RUN_TEST(test_tag_not_at_line_start_is_kept_as_text); + RUN_TEST(test_tag_only_line_yields_empty_text_with_font); + RUN_TEST(test_lone_bracket_line_is_kept_as_text); + + RUN_TEST(test_resolve_uses_parsed_lines_for_pairing_pin); + RUN_TEST(test_resolve_strips_tag_when_parsed_cache_missing); + RUN_TEST(test_resolve_leaves_picker_lines_untouched); + + exit(UNITY_END()); +} + +void loop() {} + +#else // !HAS_SCREEN + +void setUp(void) {} +void tearDown(void) {} + +void setup() +{ + initializeTestEnvironment(); + UNITY_BEGIN(); + exit(UNITY_END()); +} + +void loop() {} + +#endif // HAS_SCREEN