From 9a71ee14b69e901466a7032c0f422aee54a314d4 Mon Sep 17 00:00:00 2001 From: Jakub <106778416+MarekZegare4@users.noreply.github.com> Date: Sun, 5 Jul 2026 16:08:44 +0200 Subject: [PATCH] fix(solo): six fixes from the 2026-07-05 UI audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - locked device: a ringing alarm/timer was invisible — wakeForAlarm() now holds the lock wake window for the whole ring, the lock screen draws the alert overlay, and a key-dismissed ring falls back to a 5 s glance - status bar: A / live-share / trail / repeater icons no longer vanish when Bluetooth is off (moved outside the isSerialEnabled() gate) - alert overlay: long text wraps to up to 3 lines inside the box instead of overflowing the border (new UITask::renderAlertOverlay, shared with lock) - MyMesh: force NUL on contact.name copied from an app frame (unterminated name could overrun the AdvertPath strcpy) - clock tools: ring/timer deadline compares now wrap-safe (signed diff), matching the trail/loc-share timers - messages: channel context menu freezes its target channel at open; fav toggle no longer retargets the menu when the fav-only filter drops the row All three solo envs build green (WioTrackerL1 OLED/Eink, GAT562-30S). Co-Authored-By: Claude Fable 5 --- examples/companion_radio/MyMesh.cpp | 4 + .../companion_radio/ui-new/QuickMsgScreen.h | 23 +++- examples/companion_radio/ui-new/UITask.cpp | 129 +++++++++++------- examples/companion_radio/ui-new/UITask.h | 11 +- 4 files changed, 113 insertions(+), 54 deletions(-) diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index 82940377..ffe558a3 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -211,6 +211,10 @@ void MyMesh::updateContactFromFrame(ContactInfo &contact, uint32_t& last_mod, co memcpy(contact.out_path, &frame[i], MAX_PATH_SIZE); i += MAX_PATH_SIZE; memcpy(contact.name, &frame[i], 32); + // The frame comes from the app and nothing guarantees a NUL inside the 32 + // bytes; an unterminated name would later overrun strcpy/strlen consumers + // (e.g. the AdvertPath name cache). + contact.name[sizeof(contact.name) - 1] = '\0'; i += 32; memcpy(&contact.last_advert_timestamp, &frame[i], 4); i += 4; diff --git a/examples/companion_radio/ui-new/QuickMsgScreen.h b/examples/companion_radio/ui-new/QuickMsgScreen.h index 08911c86..0aeb180c 100644 --- a/examples/companion_radio/ui-new/QuickMsgScreen.h +++ b/examples/companion_radio/ui-new/QuickMsgScreen.h @@ -53,6 +53,11 @@ class QuickMsgScreen : public UIScreen { // Context menu (opened by KEY_CONTEXT_MENU in CHANNEL_PICK / CONTACT_PICK / histories) PopupMenu _ctx_menu; bool _ctx_dirty; + // Channel the open channel-context menu acts on, frozen at open time. The + // Fav toggle can remove the highlighted channel from a fav-only list, so + // re-reading _channel_indices[_channel_sel] mid-interaction could silently + // retarget the menu at a different channel. + uint8_t _ctx_ch_idx = 0; char _ctx_notif_item[22]; char _ctx_melody_item[20]; char _ctx_pin_item[28]; // "Pin to dial" or "Unpin (slot N)" @@ -1389,7 +1394,7 @@ public: if (left || right) { static const char* NOTIF_LABELS[] = { "default", "OFF", "ON" }; static const char* ML[] = { "global", "M1", "M2" }; - uint8_t ch_idx = _channel_indices[_channel_sel]; + uint8_t ch_idx = _ctx_ch_idx; // frozen at menu open — see declaration int sel = _ctx_menu.selectedIndex(); if (sel == 1) { uint8_t v = chNotifState(ch_idx); @@ -1410,8 +1415,10 @@ public: bool is_fav = (p2->ch_fav_bitmask & (1ULL << ch_idx)); snprintf(_ctx_ch_fav_item, sizeof(_ctx_ch_fav_item), is_fav ? "Fav: yes" : "Fav: no"); _ctx_dirty = true; - buildChannelList(); - if (_channel_sel >= _num_channels) _channel_sel = _num_channels > 0 ? _num_channels - 1 : 0; + // List rebuild is deferred to menu close: with the fav-only + // filter on, un-favouriting this channel removes it from the + // list, and rebuilding under the open menu would shift + // _channel_sel onto a different channel mid-interaction. } } return true; @@ -1419,7 +1426,7 @@ public: } auto res = _ctx_menu.handleInput(c); if (res == PopupMenu::SELECTED && _num_channels > 0) { - uint8_t ch_idx = _channel_indices[_channel_sel]; + uint8_t ch_idx = _ctx_ch_idx; // frozen at menu open — see declaration int sel = _ctx_menu.selectedIndex(); if (sel == 0) { int cleared = (int)_history.chUnread(ch_idx); @@ -1428,7 +1435,12 @@ public: } // sel 1/2/3 already handled by LEFT/RIGHT; ENTER just closes. } - if (res != PopupMenu::NONE) _task->savePrefsIfDirty(_ctx_dirty); + if (res != PopupMenu::NONE) { + _task->savePrefsIfDirty(_ctx_dirty); + // Apply any Fav change to the visible list now that the menu is done. + buildChannelList(); + if (_channel_sel >= _num_channels) _channel_sel = _num_channels > 0 ? _num_channels - 1 : 0; + } return true; } if (c == KEY_CANCEL) { _phase = MODE_SELECT; return true; } @@ -1453,6 +1465,7 @@ public: } if (c == KEY_CONTEXT_MENU && _num_channels > 0) { uint8_t ch_idx = _channel_indices[_channel_sel]; + _ctx_ch_idx = ch_idx; // freeze the menu's target channel static const char* NOTIF_LABELS[] = { "default", "OFF", "ON" }; snprintf(_ctx_notif_item, sizeof(_ctx_notif_item), "Notif: %s", NOTIF_LABELS[chNotifState(ch_idx)]); diff --git a/examples/companion_radio/ui-new/UITask.cpp b/examples/companion_radio/ui-new/UITask.cpp index f1fd6a06..f9e5b84a 100644 --- a/examples/companion_radio/ui-new/UITask.cpp +++ b/examples/companion_radio/ui-new/UITask.cpp @@ -493,39 +493,46 @@ class HomeScreen : public UIScreen { else drawSlotIcon(display, btX, ind, ind_h, ICON_BLUETOOTH); // plain glyph: available, not linked leftmostX = btX - ind_gap; + } else { + leftmostX = battLeftX - ind_gap; // no BT icon; keep the same first-slot spacing + } - // "A" indicator — left of BT; blinks on OLED, always shown on e-ink - if (_node_prefs && _node_prefs->advert_auto_interval_sec > 0) { - int aX = leftmostX - ind; - if (blinkOn()) drawBoxedIcon(display, aX, ind, ind_h, ICON_ADVERT); - leftmostX = aX - 1; - } + // Background-mode indicators — deliberately OUTSIDE the isSerialEnabled() + // gate above: auto-advert, live share, the trail and the repeater all keep + // running with Bluetooth switched off, so their at-a-glance cues must not + // vanish with the BT icon. - // Live location sharing active. Same blink convention — another - // "leave it on and forget" broadcast, like auto-advert above. Reuses - // the diamond the map uses for a live-tracked contact, so the glyph - // already means "sharing position" elsewhere in the UI. - if (_node_prefs && _node_prefs->loc_share_enabled) { - int lsX = leftmostX - ind; - if (blinkOn()) drawBoxedIcon(display, lsX, ind, ind_h, ICON_MAP_CONTACT); - leftmostX = lsX - 1; - } + // "A" indicator — blinks on OLED, always shown on e-ink + if (_node_prefs && _node_prefs->advert_auto_interval_sec > 0) { + int aX = leftmostX - ind; + if (blinkOn()) drawBoxedIcon(display, aX, ind, ind_h, ICON_ADVERT); + leftmostX = aX - 1; + } - // GPS trail logging active. Same blink convention. - if (_task->trail().isActive()) { - int gX = leftmostX - ind; - if (blinkOn()) drawBoxedIcon(display, gX, ind, ind_h, ICON_TRAIL); - leftmostX = gX - 1; - } + // Live location sharing active. Same blink convention — another + // "leave it on and forget" broadcast, like auto-advert above. Reuses + // the diamond the map uses for a live-tracked contact, so the glyph + // already means "sharing position" elsewhere in the UI. + if (_node_prefs && _node_prefs->loc_share_enabled) { + int lsX = leftmostX - ind; + if (blinkOn()) drawBoxedIcon(display, lsX, ind, ind_h, ICON_MAP_CONTACT); + leftmostX = lsX - 1; + } - // Repeater relaying. Same blink convention — a "leave it on and forget" - // mode, so an at-a-glance cue matters (especially on a Custom profile, - // where the device is off your own network while this is shown). - if (_node_prefs && _node_prefs->client_repeat) { - int rX = leftmostX - ind; - if (blinkOn()) drawBoxedIcon(display, rX, ind, ind_h, ICON_REPEATER); - leftmostX = rX - 1; - } + // GPS trail logging active. Same blink convention. + if (_task->trail().isActive()) { + int gX = leftmostX - ind; + if (blinkOn()) drawBoxedIcon(display, gX, ind, ind_h, ICON_TRAIL); + leftmostX = gX - 1; + } + + // Repeater relaying. Same blink convention — a "leave it on and forget" + // mode, so an at-a-glance cue matters (especially on a Custom profile, + // where the device is off your own network while this is shown). + if (_node_prefs && _node_prefs->client_repeat) { + int rX = leftmostX - ind; + if (blinkOn()) drawBoxedIcon(display, rX, ind, ind_h, ICON_REPEATER); + leftmostX = rX - 1; } // GPS fix status — boxed (lit) when the receiver has a valid fix, plain @@ -1528,11 +1535,6 @@ void UITask::gotoRepeaterScreen() { setCurrScreen(repeater_screen); } void UITask::gotoClockTools() { setCurrScreen(clock_tools); } void UITask::gotoLiveShareScreen() { setCurrScreen(live_share_screen); } -void UITask::wakeForAlarm() { - if (_display != NULL) _display->turnOn(); - _next_refresh = 0; // draw the alert overlay immediately -} - // ── Clock tools engine (alarm / countdown / ring) ─────────────────────────── // Lives here, not in ClockToolsScreen, so it fires regardless of the current // screen. The melody overrides mute (playMelody → buzzer.playForced); the ring @@ -1541,6 +1543,16 @@ static const char* CLOCK_ALARM_MELODY = "alarm:d=8,o=6,b=125:c,c,c,c,p, static const uint32_t CLOCK_RING_MS = 60000; static const uint32_t CLOCK_ALARM_CATCHUP_SECS = 6 * 3600; // fire late up to 6 h, else reschedule +void UITask::wakeForAlarm() { + if (_display != NULL) _display->turnOn(); + // Locked: the lock-screen blanking check (loop()) turns the display straight + // back off once _lock_wake_until is in the past — which it always is by the + // time an alarm fires. Hold the wake window open for the whole ring so the + // lock screen (and its alert overlay) stays visible while ringing. + if (_locked) _lock_wake_until = millis() + CLOCK_RING_MS; + _next_refresh = 0; // draw the alert overlay immediately +} + // Next absolute wall instant matching alarm_hour:alarm_min in local time, // strictly after now_wall (an alarm set to the current minute waits a day). uint32_t UITask::computeAlarmNextFire(uint32_t now_wall) const { @@ -1590,12 +1602,14 @@ void UITask::evaluateAlarm() { void UITask::tickClockTools() { uint32_t now_ms = millis(); // Ring maintenance: repeat the melody until dismissed or the window elapses. + // Signed-difference compares (like the trail/loc-share timers) so deadlines + // landing past the millis() rollover don't read as already elapsed. if (_ringing) { - if (now_ms >= _ring_until_ms) { stopMelody(); _ringing = false; clearAlert(); } + if ((int32_t)(now_ms - _ring_until_ms) >= 0) { stopMelody(); _ringing = false; clearAlert(); } else if (!isMelodyPlaying()) playMelody(CLOCK_ALARM_MELODY); } // Countdown timer (millis — sync-immune). - if (_timer_running && now_ms >= _timer_deadline_ms) { + if (_timer_running && (int32_t)(now_ms - _timer_deadline_ms) >= 0) { _timer_running = false; fireClockAlert("Timer done"); } @@ -1865,6 +1879,30 @@ void UITask::userLedHandler() { #endif } +// Centred alert box. Long text used to be drawn as one drawTextCentered line +// that overflowed the border on both sides (e.g. "GPS on, tracking started" +// is already wider than a 128 px OLED); wrap it to up to three lines inside +// the box instead. Uses the shared wrap scratch (s_wrap_*) — single-threaded +// render path, same contract as the message views. +void UITask::renderAlertOverlay() { + _display->setTextSize(1); + const int lh = _display->getLineHeight(); + const int pad = 3; + const int box_w = _display->width() - 8; + const int box_x = 4; + _display->translateUTF8ToBlocks(s_wrap_trans, _alert, sizeof(s_wrap_trans)); + int nl = FullscreenMsgView::wrapLines(*_display, s_wrap_trans, box_w - pad * 2, s_wrap_lines, 3); + if (nl < 1) nl = 1; + int box_h = nl * lh + pad * 2; + int box_y = (_display->height() - box_h) / 2; + _display->setColor(DisplayDriver::DARK); + _display->fillRect(box_x, box_y, box_w, box_h); + _display->setColor(DisplayDriver::LIGHT); + _display->drawRect(box_x, box_y, box_w, box_h); + for (int i = 0; i < nl; i++) + _display->drawTextCentered(_display->width() / 2, box_y + pad + i * lh, s_wrap_lines[i]); +} + void UITask::setCurrScreen(UIScreen* c) { // Fail safe on a null target: a screen pointer left uninitialised (member // declared + navigator wired, but the `new XScreen()` line forgotten in @@ -2117,6 +2155,9 @@ void UITask::loop() { dismissRing(); _kq_head = _kq_tail = 0; _next_refresh = 0; + // Locked: wakeForAlarm() held the wake window open for the whole ring; + // once dismissed, fall back to the usual brief lock-screen glance. + if (_locked) _lock_wake_until = millis() + 5000; } if (_kq_head != _kq_tail) { @@ -2226,24 +2267,16 @@ void UITask::loop() { _display->setColor(DisplayDriver::DARK); _display->setCursor(hx, hy); _display->print(hint); + // Alert overlay on top — without this a ringing alarm on a locked device + // played its melody against a screen that never said what was ringing. + if (millis() < _alert_expiry) renderAlertOverlay(); _display->endFrame(); _next_refresh = millis() + Features::LOCKSCREEN_REFRESH_MS; } else if (!_locked && millis() >= _next_refresh && curr) { _display->startFrame(); int delay_millis = curr->render(*_display); if (millis() < _alert_expiry) { // alert overlay on top of any screen - _display->setTextSize(1); - int lh = _display->getLineHeight(); - int pad = 3; - int box_h = lh + pad * 2; - int box_w = _display->width() - 8; - int box_x = 4; - int box_y = (_display->height() - box_h) / 2; - _display->setColor(DisplayDriver::DARK); - _display->fillRect(box_x, box_y, box_w, box_h); - _display->setColor(DisplayDriver::LIGHT); - _display->drawRect(box_x, box_y, box_w, box_h); - _display->drawTextCentered(_display->width() / 2, box_y + pad, _alert); + renderAlertOverlay(); // Keep refreshing the underlying screen at its own cadence (capped at the // alert's expiry) so layouts that settle over a frame — e.g. the message- // history scrollbar reserve — don't stay stuck behind the alert. Unchanged diff --git a/examples/companion_radio/ui-new/UITask.h b/examples/companion_radio/ui-new/UITask.h index 8bf140da..5b62f7b7 100644 --- a/examples/companion_radio/ui-new/UITask.h +++ b/examples/companion_radio/ui-new/UITask.h @@ -187,6 +187,12 @@ class UITask : public AbstractUITask { void setCurrScreen(UIScreen* c); + // Centred alert overlay (the showAlert() box). Wraps long text to up to + // three lines inside the box instead of letting it overflow the border. + // Shared by the normal render path and the lock screen (so a ringing + // alarm's label is visible while locked). + void renderAlertOverlay(); + public: UITask(mesh::MainBoard* board, BaseSerialInterface* serial) : AbstractUITask(board, serial), _display(NULL), _sensors(NULL), _node_prefs(NULL), _dash_lpp(200) { @@ -290,7 +296,10 @@ public: uint32_t timerRemainingMs() const { if (!_timer_running) return 0; uint32_t now = millis(); - return (now >= _timer_deadline_ms) ? 0 : (_timer_deadline_ms - now); + // Signed-difference compare so a deadline that lands past the millis() + // rollover (~49.7 days) doesn't read as already elapsed. + if ((int32_t)(now - _timer_deadline_ms) >= 0) return 0; + return _timer_deadline_ms - now; } bool isRinging() const { return _ringing; } void dismissRing() { stopMelody(); _ringing = false; clearAlert(); }