From fe19d1b0a7744f194bcc183adeb90c52f3643ebe Mon Sep 17 00:00:00 2001 From: MarekZegare4 Date: Mon, 29 Jun 2026 17:43:17 +0200 Subject: [PATCH] refactor(companion): dedup age formatting + reply-prefix parsing; drop dead code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidation pass over the message/nearby UI, no functional change beyond one intentional display tweak: - Age tags: NearbyScreen::fmtAge, the nearby list's inline column, and QuickMsgScreen::fmtMsgAge each reimplemented the same s/m/h bucket ladder on top of geo::fmtAgeShort. All now delegate to it (fmtMsgAge removed). Visible effect: ages over 24h render as "Nd" instead of capped hours, matching the Locator target picker which already used fmtAgeShort. - Reply prefix: the "@[nick] " parse was duplicated in skipReplyPrefix() and FullscreenMsgView::render(). Extracted to one msgReplyBody() helper (body, plus optional addressee nick) — one place to handle its edge cases. - LiveTrack: the expiry predicate was duplicated in expire()/isActive(); extracted to a private expired() helper. - Removed a dead M_PI define (and unused ) in NearbyScreen.h, and a comment pointing at a CODE_REVIEW.md that doesn't exist. Co-Authored-By: Claude Opus 4.8 --- examples/companion_radio/LiveTrack.h | 20 ++++----- .../ui-new/FullscreenMsgView.h | 45 ++++++++++++------- .../companion_radio/ui-new/NearbyScreen.h | 28 ++++-------- .../companion_radio/ui-new/QuickMsgScreen.h | 25 +++-------- 4 files changed, 52 insertions(+), 66 deletions(-) diff --git a/examples/companion_radio/LiveTrack.h b/examples/companion_radio/LiveTrack.h index 3fe45e21..78e71e6b 100644 --- a/examples/companion_radio/LiveTrack.h +++ b/examples/companion_radio/LiveTrack.h @@ -51,14 +51,9 @@ public: } // Drop entries not refreshed within EXPIRY_SECS. `now` is RTC epoch seconds. - // Guarded against now < ts (RTC stepped backwards) so a clock fix can't wipe - // the table. void expire(uint32_t now) { - for (int i = 0; i < CAPACITY; i++) { - if (_e[i].used && now > _e[i].ts && (now - _e[i].ts) > EXPIRY_SECS) { - _e[i].used = false; - } - } + for (int i = 0; i < CAPACITY; i++) + if (_e[i].used && expired(_e[i], now)) _e[i].used = false; } void clear() { for (int i = 0; i < CAPACITY; i++) _e[i].used = false; } @@ -66,9 +61,7 @@ public: // Slot-wise access (caller skips inactive slots via isActive()). const Entry& slotAt(int i) const { return _e[i]; } bool isActive(int i, uint32_t now) const { - const Entry& e = _e[i]; - if (!e.used) return false; - return !(now > e.ts && (now - e.ts) > EXPIRY_SECS); + return _e[i].used && !expired(_e[i], now); } // Number of currently non-expired entries. @@ -94,6 +87,13 @@ public: private: Entry _e[CAPACITY] = {}; + // An entry is stale once EXPIRY_SECS have passed since its last update. + // Guarded against now < ts (RTC stepped backwards) so a clock fix can't + // mass-expire the table. + static bool expired(const Entry& e, uint32_t now) { + return now > e.ts && (now - e.ts) > EXPIRY_SECS; + } + // Match an existing entry: verified shares by key, channel shares by name. int find(const uint8_t* key, const char* name, bool verified) const { for (int i = 0; i < CAPACITY; i++) { diff --git a/examples/companion_radio/ui-new/FullscreenMsgView.h b/examples/companion_radio/ui-new/FullscreenMsgView.h index 4390a0f9..30bcdff4 100644 --- a/examples/companion_radio/ui-new/FullscreenMsgView.h +++ b/examples/companion_radio/ui-new/FullscreenMsgView.h @@ -10,12 +10,35 @@ static const int FS_CHARS_MAX = 80; // max bytes per wrapped line // fullscreen message view and the history list are never laid out in the same // frame (opening the fullscreen view early-returns before the list loop runs), // so one static buffer serves both. This keeps ~1.5 KB of line buffers off the -// render-call stack (see CODE_REVIEW.md "Render-path stack peak") at the cost of -// a fixed RAM allocation. Only used inside render()/wrap helpers — never across -// a yield, so the single instance is safe. +// render-call stack at the cost of a fixed RAM allocation. Only used inside +// render()/wrap helpers — never across a yield, so the single instance is safe. static char s_wrap_trans[512]; static char s_wrap_lines[12][FS_CHARS_MAX]; +// Parse a leading "@[nick] " reply prefix. Returns the message body that +// follows it (and any leading whitespace); when nick/nick_n are supplied, +// fills nick with the addressee, or "" when there's no prefix. One parser for +// both the history list (body only — see QuickMsgScreen::skipReplyPrefix) and +// the fullscreen view (which also shows the "To:" nick). +static inline const char* msgReplyBody(const char* text, char* nick = nullptr, int nick_n = 0) { + if (nick && nick_n > 0) nick[0] = '\0'; + const char* body = text; + if (text[0] == '@' && text[1] == '[') { + const char* close = strchr(text + 2, ']'); + if (close && close[1] == ' ' && close[2]) { + if (nick && nick_n > 0) { + int len = (int)(close - text) - 2; + if (len > nick_n - 1) len = nick_n - 1; + memcpy(nick, text + 2, len); + nick[len] = '\0'; + } + body = close + 2; + } + } + while (*body == '\n' || *body == '\r' || *body == ' ') body++; + return body; +} + struct FullscreenMsgView { int scroll; bool active; @@ -80,19 +103,9 @@ struct FullscreenMsgView { const int lineH = display.getLineHeight(); const int max_px = display.width() - 6; - // detect @recipient at start of message - char to_nick[32] = ""; - const char* body = text; - if (text[0] == '@' && text[1] == '[') { - const char* close = strchr(text + 2, ']'); - if (close && close[1] == ' ' && close[2]) { - int len = (int)(close - text) - 2; - if (len >= (int)sizeof(to_nick)) len = sizeof(to_nick) - 1; - memcpy(to_nick, text + 2, len); - to_nick[len] = '\0'; - body = close + 2; - } - } + // "@[nick] " reply prefix → "To:" header + body (shared parser). + char to_nick[32]; + const char* body = msgReplyBody(text, to_nick, sizeof(to_nick)); const int cw = display.getCharWidth(); const int header_h = to_nick[0] ? (lineH * 2 + 4) : (lineH + 2); diff --git a/examples/companion_radio/ui-new/NearbyScreen.h b/examples/companion_radio/ui-new/NearbyScreen.h index 57a503ff..a971b4ee 100644 --- a/examples/companion_radio/ui-new/NearbyScreen.h +++ b/examples/companion_radio/ui-new/NearbyScreen.h @@ -1,12 +1,7 @@ #pragma once -#include #include "../GeoUtils.h" #include "NavView.h" -#ifndef M_PI - #define M_PI 3.14159265358979323846 -#endif - // ── Nearby Nodes ────────────────────────────────────────────────────────────── // One list / detail / action-menu interaction path over two sources: // SRC_STORED — contacts known to the mesh (distance / bearing / last-heard) @@ -111,14 +106,13 @@ class NearbyScreen : public UIScreen { bool useImperial() const { return _task && _task->useImperial(); } + // Full "X ago" form for the detail view, built on the shared short-age tag + // so there's one bucket ladder (see geo::fmtAgeShort). static void fmtAge(char* buf, int n, uint32_t lastmod) { - uint32_t now = rtc_clock.getCurrentTime(); - if (now < lastmod || lastmod == 0) { snprintf(buf, n, "unknown"); return; } - uint32_t age = now - lastmod; - if (age < 60) snprintf(buf, n, "%us ago", age); - else if (age < 3600) snprintf(buf, n, "%um ago", age / 60); - else if (age < 86400) snprintf(buf, n, "%uh ago", age / 3600); - else snprintf(buf, n, ">1d ago"); + char s[8]; + geo::fmtAgeShort(s, sizeof(s), rtc_clock.getCurrentTime(), lastmod); + if (!s[0]) { snprintf(buf, n, "unknown"); return; } + snprintf(buf, n, "%s ago", s); } static const char* typeName(uint8_t t) { @@ -715,14 +709,8 @@ public: if (_source == SRC_SCAN) { snprintf(right, sizeof(right), "%d", (int)e.rssi); } else if (_sort == SORT_TIME) { - uint32_t now = rtc_clock.getCurrentTime(); - if (e.lastmod == 0 || now < e.lastmod) snprintf(right, sizeof(right), "?"); - else { - uint32_t age = now - e.lastmod; - if (age < 60) snprintf(right, sizeof(right), "%us", age); - else if (age < 3600) snprintf(right, sizeof(right), "%um", age / 60); - else snprintf(right, sizeof(right), "%uh", age / 3600); - } + geo::fmtAgeShort(right, sizeof(right), rtc_clock.getCurrentTime(), e.lastmod); + if (!right[0]) snprintf(right, sizeof(right), "?"); // unknown / RTC not synced } else { if (e.dist_km >= 0.0f) geo::fmtDist(right, sizeof(right), e.dist_km, useImperial()); else strncpy(right, "?GPS", sizeof(right)); diff --git a/examples/companion_radio/ui-new/QuickMsgScreen.h b/examples/companion_radio/ui-new/QuickMsgScreen.h index 533bcb5e..183ce51f 100644 --- a/examples/companion_radio/ui-new/QuickMsgScreen.h +++ b/examples/companion_radio/ui-new/QuickMsgScreen.h @@ -209,15 +209,9 @@ class QuickMsgScreen : public UIScreen { return r; } - // Strip "@[nick] " reply prefix from a message body for compact list display. - static const char* skipReplyPrefix(const char* text) { - if (text[0] == '@' && text[1] == '[') { - const char* close = strchr(text + 2, ']'); - if (close && close[1] == ' ' && close[2]) text = close + 2; - } - while (*text == '\n' || *text == '\r' || *text == ' ') text++; - return text; - } + // Strip the "@[nick] " reply prefix for compact list display (body only). + // Shares the one parser with the fullscreen view — see msgReplyBody(). + static const char* skipReplyPrefix(const char* text) { return msgReplyBody(text); } // Split a DM-history entry into the name to show as the author and the body to // show beneath it. Room servers carry many guests, so incoming room posts are @@ -351,15 +345,6 @@ class QuickMsgScreen : public UIScreen { } - static void fmtMsgAge(char* buf, int n, uint32_t timestamp, uint32_t now) { - if (timestamp == 0 || now < timestamp) { buf[0] = '\0'; return; } - uint32_t age = now - timestamp; - if (age < 60) snprintf(buf, n, "%us", age); - else if (age < 3600) snprintf(buf, n, "%um", age / 60); - else if (age < 86400) snprintf(buf, n, "%uh", age / 3600); - else snprintf(buf, n, ">1d"); - } - // count history entries for a specific channel int histCountForChannel(int ch_idx) const { int n = 0; @@ -1207,7 +1192,7 @@ public: const char* body = dmDisplayParts(e, is_room, filtered_name, sender_buf, sizeof(sender_buf)); const char* sender = sender_buf; - char age[6]; fmtMsgAge(age, sizeof(age), e.timestamp, now_ts); + char age[6]; geo::fmtAgeShort(age, sizeof(age), now_ts, e.timestamp); int age_w = age[0] ? display.getTextWidth(age) + 3 : 0; if (sel) { @@ -1368,7 +1353,7 @@ public: msg_part[sizeof(msg_part) - 1] = '\0'; } - char age[6]; fmtMsgAge(age, sizeof(age), _hist[ring_pos].timestamp, now_ts); + char age[6]; geo::fmtAgeShort(age, sizeof(age), now_ts, _hist[ring_pos].timestamp); int age_w = age[0] ? display.getTextWidth(age) + 3 : 0; if (sel) {