Mirrors/MeshCore-Solo
1
0
Fork 0
mirror of https://github.com/MarekZegare4/MeshCore-Solo.git synced 2026-07-31 09:18:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ui,bot): resolve remaining findings from the 2026-07-26 screen review

Browse Source 
- DM/room unread badges could claim messages the ring no longer held
  (same class as the channel fix in 6470afaf, not covered by it):
  getDMUnread()/getDMUnreadTotal() now clamp to dmHistCountForContact(),
  and a new reconcileDMUnread() (called once per loop()) frees any
  _dm_unread_table slot whose ring occupancy has dropped to 0, so a
  17th sender isn't starved by stale entries. onContactRemoved() now
  also clears _dm_unread_table -- the one per-contact table it was
  missing.
- Shift didn't capitalise ł/ń/ź/ż (+ĺ/ľ/ň/ž): the Latin Extended-A
  case-pairing rule assumed a single parity for the whole block, but it
  flips around the unpaired codepoints ĸ/ʼn/Ÿ. Fixed with four
  sub-ranges, verified exhaustively over U+0100-U+017F.
- Triple-click could still toggle the buzzer while locked on
  PIN_USER_BTN/PIN_USER_BTN_ANA boards (joystick path already guarded
  this).
- millis() wraparound: 4 absolute comparisons in UITask.cpp (battery
  poll, auto-off, lock-wake, backlight) converted to the existing
  (int32_t)(millis()-deadline)>=0 idiom; MyMeshBot.h's DM-throttle
  eviction now picks the oldest slot by elapsed time instead of raw
  t_ms, which picked the wrong slot right after a rollover.
- Long-press bypassed checkDisplayOn() on all 5 call sites -- neither
  woke the display nor extended auto-off, and could deliver
  KEY_CONTEXT_MENU to the invisible screen. Moved the gate inside
  handleLongPress() itself instead of patching each site.
- CardKB's backspace/printable-insert branches didn't reset t9_cell,
  so typing right after a T9 cycle tap could get silently overwritten
  by a same-cell re-tap within the T9 timeout.
- buildContactList()'s counts[MAX_CONTACTS] was a 1400 B int array on
  the 4 KB loop() stack; values are bounded by DM_HIST_MAX (32), so
  now uint8_t.
- ACK table treated ack==0 as a wildcard: isAckPending(0) matched any
  free slot, and processAck() with an all-zero ACK matched the first
  free slot and returned its stale contact pointer. Both now skip/reject
  ack==0, and the matched slot's contact pointer is cleared alongside
  its ack hash.
- ensurePageOrderInit() could write one byte past page_order[13] when
  migrating a saved order with all 13 slots full and CLOCK last --
  guarded on insert_at < PAGE_ORDER_LEN.

Two findings from the same review were resolved as no-op decisions,
not code changes: !buzz over DM ignoring quiet hours is intentional
(the pull exemption is meant to cover the buzzer), and the offline
queue's full-queue drop-newest behaviour is upstream code, left alone.

Build-verified green on WioTrackerL1_companion_solo_dual (RAM 71.1%,
Flash 66.6%) and WioTrackerL1Eink_companion_solo_dual (RAM 73.0%,
Flash 67.9%).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Jakub
2026-07-27 22:05:52 +02:00
parent 6470afaf56
commit 8e5f083e2e
9 changed files with 110 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
examples/companion_radio/MyMesh.cpp
View File

@@ -462,8 +462,15 @@ void MyMesh::onContactPathUpdated(const ContactInfo &contact) {
}
ContactInfo* MyMesh::processAck(const uint8_t *data) {
// 0 marks an empty/cleared table slot, not a real ACK -- a bare memcmp below
// would otherwise match the first free slot against an all-zero `data` (which
// *is* remotely reachable, ack_crc comes off the air) and misattribute the
// ACK to whatever contact that slot last belonged to.
static const uint8_t zero_ack[4] = {0, 0, 0, 0};
if (memcmp(data, zero_ack, 4) == 0) return checkConnectionsAck(data);
// see if matches any in a table
for (int i = 0; i < EXPECTED_ACK_TABLE_SIZE; i++) {
if (expected_ack_table[i].ack == 0) continue; // empty slot
if (memcmp(data, &expected_ack_table[i].ack, 4) == 0) { // got an ACK from recipient
out_frame[0] = PUSH_CODE_SEND_CONFIRMED;
memcpy(&out_frame[1], data, 4);
@@ -471,9 +478,11 @@ ContactInfo* MyMesh::processAck(const uint8_t *data) {
memcpy(&out_frame[5], &trip_time, 4);
_serial->writeFrame(out_frame, 9);
ContactInfo* contact = expected_ack_table[i].contact;
// NOTE: the same ACK can be received multiple times!
expected_ack_table[i].ack = 0; // clear expected hash, now that we have received ACK
return expected_ack_table[i].contact;
expected_ack_table[i].ack = 0; // clear expected hash, now that we have received ACK
expected_ack_table[i].contact = nullptr; // and the stale pointer along with it
return contact;
}
}
return checkConnectionsAck(data);

3
examples/companion_radio/MyMesh.h
View File

@@ -298,8 +298,9 @@ public:
void applyRepeaterRadio();
bool isAckPending(uint32_t expected_ack) const {
if (expected_ack == 0) return false; // 0 marks an empty/cleared slot, not a real ACK
for (int i = 0; i < EXPECTED_ACK_TABLE_SIZE; i++)
if (expected_ack_table[i].ack == expected_ack) return true;
if (expected_ack_table[i].ack != 0 && expected_ack_table[i].ack == expected_ack) return true;
return false;
}

8
examples/companion_radio/MyMeshBot.h
View File

@@ -107,9 +107,11 @@ void MyMesh::botDmRecord(const uint8_t* pubkey) {
for (int i = 0; i < BOT_DM_LOG_SIZE; i++)
if (!_bot_dm_log[i].used) { target = i; break; }
if (target < 0) { // ...or evict the oldest
target = 0;
for (int i = 1; i < BOT_DM_LOG_SIZE; i++)
if (_bot_dm_log[i].t_ms < _bot_dm_log[target].t_ms) target = i;
target = 0; // compare by elapsed-since-now (wraparound-safe),
for (int i = 1; i < BOT_DM_LOG_SIZE; i++) // not raw t_ms -- a bare '<' picks the wrong slot
if ((int32_t)(now - _bot_dm_log[i].t_ms) > // right after millis() rolls over (new entries
(int32_t)(now - _bot_dm_log[target].t_ms)) // then have small raw values, not large ones)
target = i;
}
memcpy(_bot_dm_log[target].key, pubkey, 4);
_bot_dm_log[target].t_ms = now;

22
examples/companion_radio/ui-new/KeyboardWidget.h
View File

@@ -126,11 +126,12 @@ static const int KB_PREVIEW_CAP = 46;
// special-cased before the range rule.
// - Latin Extended-A (ą/č/ĺ/œ/etc., U+0100-017F): NOT a flat offset like
// Latin-1 — this block alternates even=uppercase/odd=lowercase in adjacent
// pairs, so lowercase - 1 = uppercase. Verified true (against Python's own
// str.upper()) for every character actually used across the Polish/Czech/
// Slovak/French keyboards below; NOT a universal rule for the whole block
// (it has a handful of unpaired/irregular codepoints elsewhere) — recheck
// before adding more from it.
// pairs, so lowercase - 1 = uppercase. But the parity flips around the
// unpaired codepoints ĸ (U+0138), ʼn (U+0149) and Ÿ (U+0178), so a single
// "odd=lower" rule is wrong for U+0139-0148 and U+0179-017E (this broke
// ł ń ź ż among others). Verified exhaustively over the whole
// U+0100-U+017F block — see the four sub-ranges below; ı and ſ are the
// only remaining exceptions and appear in no keyboard table here.
// - Greek α-ω (U+03B1-03C9): flat -0x20 offset, same shape as Cyrillic/ASCII.
// Final sigma ς (U+03C2) is the one exception — it has no uppercase of its
// own; -0x20 would land on U+03A2, which is unassigned. It capitalizes to
@@ -149,7 +150,13 @@ static void kbApplyCapsUtf8(const char* in, bool caps, char* out, size_t out_siz
else if (cp >= 0x0430 && cp <= 0x044F) cp -= 0x20; // а-я -> А-Я
else if (cp >= 0x03B1 && cp <= 0x03C9) cp -= 0x20; // α-ω -> Α-Ω
else if (cp >= 0x00E0 && cp <= 0x00FE && cp != 0x00F7) cp -= 0x20; // à-þ -> À-Þ
else if (cp >= 0x0100 && cp < 0x0180 && (cp & 1) == 1) cp -= 1; // ą-ż (odd=lower) -> Ą-Ż
// ą-ż (Latin Extended-A): pairing parity flips around the unpaired
// codepoints ĸ (U+0138), ʼn (U+0149), Ÿ (U+0178) -- verified exhaustively
// over the whole U+0100-U+017F block, resolves ł ń ź ż + ĺ ľ ň ž too.
else if (cp >= 0x0100 && cp <= 0x0137 && (cp & 1) == 1) cp -= 1; // odd=lower
else if (cp >= 0x0139 && cp <= 0x0148 && (cp & 1) == 0) cp -= 1; // even=lower
else if (cp >= 0x014A && cp <= 0x0177 && (cp & 1) == 1) cp -= 1; // odd=lower
else if (cp >= 0x0179 && cp <= 0x017E && (cp & 1) == 0) cp -= 1; // even=lower
else if (cp >= 'a' && cp <= 'z') cp -= 0x20; // a-z -> A-Z
}
if (cp < 0x80) {
@@ -778,6 +785,7 @@ struct KeyboardWidget {
// user, so there's no ambiguity to resolve here.
if (c == KEY_KB_ENTER) return DONE;
if (c == 0x08) {
t9_cell = -1; // invalidate any pending T9 cycle -- see the grid paths below
if (cursor_pos > 0) {
int n = kbUtf8LastCharBytes(buf, cursor_pos);
memmove(buf + cursor_pos - n, buf + cursor_pos, len - cursor_pos);
@@ -787,6 +795,8 @@ struct KeyboardWidget {
return NONE;
}
if (c >= 0x20 && c <= 0x7E) {
t9_cell = -1; // otherwise a same-cell T9 tap within KB_T9_TIMEOUT_MS would
// overwrite this character instead of inserting a new one
char one[2] = { c, '\0' };
insertGlyph(one, false);
return NONE;

10
examples/companion_radio/ui-new/MessagesScreen.h
View File

@@ -483,7 +483,10 @@ class MessagesScreen : public UIScreen {
} else {
bool show_all = (p && p->dm_show_all);
// Build _sorted and counts[] in one pass — avoids a second getContactByIdx loop.
int counts[MAX_CONTACTS];
// uint8_t, not int: values are bounded by MessageHistory::DM_HIST_MAX (32), and
// this array is 1400 B at this build's MAX_CONTACTS=350 as an int[] -- a sizeable
// slice of the 4 KB loop() task stack for one local array.
uint8_t counts[MAX_CONTACTS];
for (int i = 0; i < total; i++) {
if (!the_mesh.getContactByIdx(i, c) || c.type != ADV_TYPE_CHAT) continue;
if (!show_all && !(c.flags & 0x01)) continue;
@@ -774,6 +777,11 @@ public:
int getTotalChannelUnread() const { return _history.getTotalChannelUnread(); }
// How many DM ring entries this contact/room currently holds -- lets UITask
// clamp its separate _dm_unread_table counters to what the shared 32-slot DM
// ring actually still has, the same self-healing shape as the channel fix.
int dmHistCountForContact(const uint8_t* pub_key) const { return _history.dmHistCountForContact(pub_key); }
void markReadAlert(int n) {
char msg[32];
snprintf(msg, sizeof(msg), "%d marked read", n);

13
examples/companion_radio/ui-new/SettingsScreen.h
View File

@@ -323,9 +323,16 @@ class SettingsScreen : public UIScreen {
// PAGE_ORDER_LEN so there's room; only a pathologically full order would
// drop its last entry, which buildVisibleOrder's fallback re-appends.
int insert_at = clock_at + 1;
int tail = (len < NodePrefs::PAGE_ORDER_LEN) ? len : NodePrefs::PAGE_ORDER_LEN - 1;
for (int i = tail; i > insert_at; i--) p->page_order[i] = p->page_order[i - 1];
p->page_order[insert_at] = NodePrefs::HPB_FAVOURITES + 1;
// Guard against a saved order with all PAGE_ORDER_LEN slots already
// valid and CLOCK in the last one: insert_at would be PAGE_ORDER_LEN,
// one past the array, and the shift loop below wouldn't run (tail is
// clamped to the last index) to catch it -- the write would land one
// byte past page_order, into whatever NodePrefs field follows.
if (insert_at < NodePrefs::PAGE_ORDER_LEN) {
int tail = (len < NodePrefs::PAGE_ORDER_LEN) ? len : NodePrefs::PAGE_ORDER_LEN - 1;
for (int i = tail; i > insert_at; i--) p->page_order[i] = p->page_order[i - 1];
p->page_order[insert_at] = NodePrefs::HPB_FAVOURITES + 1;
}
}
// Append any pages that are absent from the saved order (e.g. added by a
// later firmware version). Recount first since the block above may have

54
examples/companion_radio/ui-new/UITask.cpp
View File

@@ -1779,11 +1779,32 @@ void UITask::addDMMsg(const uint8_t* pub_key, bool outgoing, const char* text, u
int UITask::getDMUnreadTotal() const {
int total = 0;
for (int i = 0; i < DM_UNREAD_TABLE_SIZE; i++)
total += _dm_unread_table[i].count;
for (int i = 0; i < DM_UNREAD_TABLE_SIZE; i++) {
if (_dm_unread_table[i].count == 0) continue;
int held = ((MessagesScreen*)messages_screen)->dmHistCountForContact(_dm_unread_table[i].prefix);
total += (_dm_unread_table[i].count < held) ? _dm_unread_table[i].count : held;
}
return total;
}
uint8_t UITask::getDMUnread(const uint8_t* pub_key) const {
for (int i = 0; i < DM_UNREAD_TABLE_SIZE; i++) {
if (_dm_unread_table[i].count > 0 && memcmp(_dm_unread_table[i].prefix, pub_key, 4) == 0) {
int held = ((MessagesScreen*)messages_screen)->dmHistCountForContact(pub_key);
return _dm_unread_table[i].count < held ? _dm_unread_table[i].count : (uint8_t)held;
}
}
return 0;
}
void UITask::reconcileDMUnread() {
for (int i = 0; i < DM_UNREAD_TABLE_SIZE; i++) {
if (_dm_unread_table[i].count == 0) continue;
if (((MessagesScreen*)messages_screen)->dmHistCountForContact(_dm_unread_table[i].prefix) == 0)
_dm_unread_table[i].count = 0; // ring no longer holds anything for this sender -- free the slot
}
}
void UITask::showAlert(const char* text, int duration_millis) {
snprintf(_alert, sizeof(_alert), "%s", text);
_alert_expiry = millis() + duration_millis;
@@ -2176,6 +2197,7 @@ void UITask::loop() {
// Background delivery: resend pending on-device DMs whose ACK timed out, and
// finalise the ✗ marker — runs regardless of which screen is active.
((MessagesScreen*)messages_screen)->tickDmResends();
reconcileDMUnread();
#if UI_HAS_JOYSTICK
uint8_t joy_rot = _node_prefs ? _node_prefs->joystick_rotation : JOYSTICK_ROTATION;
int ev = user_btn.check();
@@ -2250,7 +2272,7 @@ void UITask::loop() {
} else if (ev == BUTTON_EVENT_DOUBLE_CLICK) {
enqueueKey(handleDoubleClick(KEY_PREV));
} else if (ev == BUTTON_EVENT_TRIPLE_CLICK) {
enqueueKey(handleTripleClick(KEY_SELECT));
if (!_locked) enqueueKey(handleTripleClick(KEY_SELECT));
}
#endif
#if defined(PIN_USER_BTN_ANA)
@@ -2263,14 +2285,14 @@ void UITask::loop() {
} else if (ev == BUTTON_EVENT_DOUBLE_CLICK) {
enqueueKey(handleDoubleClick(KEY_PREV));
} else if (ev == BUTTON_EVENT_TRIPLE_CLICK) {
enqueueKey(handleTripleClick(KEY_SELECT));
if (!_locked) enqueueKey(handleTripleClick(KEY_SELECT));
}
_analogue_pin_read_millis = millis();
}
#endif
pollCardKB();
#if defined(BACKLIGHT_BTN)
if (millis() > next_backlight_btn_check) {
if ((int32_t)(millis() - next_backlight_btn_check) >= 0) {
bool touch_state = digitalRead(PIN_BUTTON2);
#if defined(DISP_BACKLIGHT)
digitalWrite(DISP_BACKLIGHT, !touch_state);
@@ -2330,7 +2352,7 @@ void UITask::loop() {
tickClockTools();
if (_display != NULL && _display->isOn()) {
if (_locked && millis() > _lock_wake_until) {
if (_locked && (int32_t)(millis() - _lock_wake_until) >= 0) {
_display->turnOff();
} else if (_locked && millis() >= _next_refresh) {
_display->startFrame();
@@ -2442,7 +2464,7 @@ void UITask::loop() {
_auto_off = millis() + AUTO_OFF_MILLIS;
}
#endif
if (!_locked && autoOffMillis() > 0 && millis() > _auto_off && !isRinging()) {
if (!_locked && autoOffMillis() > 0 && (int32_t)(millis() - _auto_off) >= 0 && !isRinging()) {
_display->turnOff();
#ifdef PIN_LED
digitalWrite(PIN_LED, LOW); // turn off status LED with display to save power
@@ -2459,7 +2481,7 @@ void UITask::loop() {
vibration.loop();
#endif
if (millis() > next_batt_chck) {
if ((int32_t)(millis() - next_batt_chck) >= 0) {
uint16_t raw = AbstractUITask::getBattMilliVolts();
if (raw > 0) {
// EMA filter: alpha=0.2 (80% old, 20% new) — smooths ADC noise from uneven load
@@ -2706,12 +2728,17 @@ void UITask::clearTargetIfWaypoint(int32_t lat_1e6, int32_t lon_1e6) {
// cleared here, so a removed contact can't leave a dangling reference. If you
// add such a field, add its cleanup below (and mark the field in NodePrefs.h).
// Currently covered: favourite_contacts, locator_key, loc_share_dm_prefix,
// dm_notif[], dm_melody[]. Called for both explicit removal and silent
// auto-eviction (see MyMesh CMD_REMOVE_CONTACT / onContactOverwrite).
// dm_notif[], dm_melody[]. Also clears _dm_unread_table (RAM-only, not a
// NodePrefs field, so no savePrefs() needed for it) -- same 4-byte-prefix
// shape and same 16-slot starvation risk as dm_notif/dm_melody above. Called
// for both explicit removal and silent auto-eviction (see MyMesh
// CMD_REMOVE_CONTACT / onContactOverwrite).
void UITask::onContactRemoved(const uint8_t* pub_key) {
if (!_node_prefs || !pub_key) return;
bool changed = false;
clearDMUnread(pub_key);
int slot = findFavouriteSlot(pub_key);
if (slot >= 0) { clearFavouriteSlot(slot); changed = true; }
@@ -2977,6 +3004,13 @@ char UITask::checkDisplayOn(char c) {
}
char UITask::handleLongPress(char c) {
// Same checkDisplayOn() gate every other input path goes through (see
// pollCardKB()'s Fn+letter handling for the same shape) -- without it, a long
// press while the display is off neither wakes it nor extends auto-off, and
// while unlocked it delivers KEY_CONTEXT_MENU to the invisible screen (found
// already open at the next wake instead of the press being consumed as a wake).
c = checkDisplayOn(c);
if (c == 0) return 0;
if (millis() - ui_started_at < 8000) { // long press in first 8 seconds since startup -> CLI/rescue
the_mesh.enterCLIRescue();
return 0;

14
examples/companion_radio/ui-new/UITask.h
View File

@@ -363,18 +363,20 @@ public:
int getChannelUnreadCount() const;
int getRoomUnreadCount() const { return _room_unread; }
void clearRoomUnread() { _room_unread = 0; }
uint8_t getDMUnread(const uint8_t* pub_key) const {
for (int i = 0; i < DM_UNREAD_TABLE_SIZE; i++)
if (_dm_unread_table[i].count > 0 && memcmp(_dm_unread_table[i].prefix, pub_key, 4) == 0)
return _dm_unread_table[i].count;
return 0;
}
// Clamped to the DM ring's actual occupancy for this contact -- defined in
// UITask.cpp (needs MessagesScreen to be a complete type). Same self-healing
// shape as MessageHistory::chUnread() for channels.
uint8_t getDMUnread(const uint8_t* pub_key) const;
void clearDMUnread(const uint8_t* pub_key) {
for (int i = 0; i < DM_UNREAD_TABLE_SIZE; i++)
if (_dm_unread_table[i].count > 0 && memcmp(_dm_unread_table[i].prefix, pub_key, 4) == 0)
{ _dm_unread_table[i].count = 0; return; }
}
void clearAllDMUnread() { memset(_dm_unread_table, 0, sizeof(_dm_unread_table)); }
// Frees any table slot whose ring occupancy has dropped to zero (evicted or
// deduped-away messages) so a genuinely new sender isn't starved once the
// fixed 16-slot table fills with stale entries. Called once per loop().
void reconcileDMUnread();
bool hasDisplay() const { return _display != NULL; }
DisplayDriver* getDisplay() const { return _display; }