From 80580ead54ef0fc9178ce05c569fd59ebbac0334 Mon Sep 17 00:00:00 2001 From: Jakub <106778416+MarekZegare4@users.noreply.github.com> Date: Mon, 26 Oct 2026 20:06:03 +0100 Subject: [PATCH] fix: review pass - options menu room for Show route, 'known route' for a direct-routed !path, 3-byte hashes, a channel's route not from here, bot sender looked up once, no decimal times as routes Co-Authored-By: Claude Opus 5.5 --- docs/solo/tools.md | 2 +- examples/companion_radio/MyMesh.cpp | 2 +- examples/companion_radio/MyMeshBot.h | 15 +++++++---- examples/companion_radio/ui-core/BotConfig.h | 2 +- examples/companion_radio/ui-core/HopRoute.h | 26 +++++++++---------- .../ui-lvgl/ConversationScreen.h | 2 +- .../companion_radio/ui-new/MessagesScreen.h | 4 +-- 7 files changed, 28 insertions(+), 25 deletions(-) diff --git a/docs/solo/tools.md b/docs/solo/tools.md index fa2bd809..254d8586 100644 --- a/docs/solo/tools.md +++ b/docs/solo/tools.md @@ -49,7 +49,7 @@ channel and one room, each switched on separately: | `!ping` | `pong` | | `!batt`, `!temp`, `!time`, `!loc` | battery, temperature, time, position | | `!hops` | how many hops the command took | - | `!path` | which way it came: the hop hashes, oldest first (`3f:a1:c8`; 2-byte mode `3fa1:c802`), or `direct` | + | `!path` | which way it came: the hop hashes, oldest first (`3f:a1:c8`; 2-byte mode `3fa1:c802`), `direct`, or `known route` when it came over a saved path (whose hops aren't carried) | | `!snr` | how its last hop was heard here: SNR and RSSI | | `!dist` | how far away and which way the sender is (both need a position) | | `!status` | battery, position and time | diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index 634c7f75..a1b44275 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -825,7 +825,7 @@ void MyMesh::onChannelMessageRecv(const mesh::GroupChannel &channel, mesh::Packe // hop count for !hops (see onMessageRecv); not the wire path_len above. uint8_t ch_hops = pkt->getPathHashCount(); - { + if (_prefs.bot_channel_enabled && channel_idx == _prefs.bot_channel_idx) { // the bot's channel: who's asking char sender[32]; const char* msg; botChannelSenderSplit(text, sender, sizeof(sender), &msg); diff --git a/examples/companion_radio/MyMeshBot.h b/examples/companion_radio/MyMeshBot.h index 74bf21d4..5e81ed49 100644 --- a/examples/companion_radio/MyMeshBot.h +++ b/examples/companion_radio/MyMeshBot.h @@ -135,6 +135,7 @@ bool MyMesh::botInQuietHours() const { // contact's, when it has one (on a channel, the contact of that name). static char s_bot_path[MAX_PATH_SIZE * 3 + 8]; static BotInbound s_bot_in = { s_bot_path, 0, 0 }; +static const ContactInfo* s_bot_sender = nullptr; // the asker's contact, if known (Actions by) static bool s_bot_from_pos = false; static int32_t s_bot_from_lat = 0, s_bot_from_lon = 0; @@ -142,8 +143,12 @@ void MyMesh::setBotInbound(const mesh::Packet* pkt, const ContactInfo* sender) { const uint8_t n = pkt ? pkt->getPathHashCount() : 0, w = pkt ? pkt->getPathHashSize() : 1; s_bot_in.snr = pkt ? pkt->getSNR() : 0; s_bot_in.rssi = _radio->getLastRSSI(); + s_bot_sender = sender; s_bot_from_pos = sender && (sender->gps_lat || sender->gps_lon); if (s_bot_from_pos) { s_bot_from_lat = sender->gps_lat; s_bot_from_lon = sender->gps_lon; } + // A direct-routed packet's path is used up on the way (each repeater takes + // its own hash off), so it says nothing about the hops it went through. + if (pkt && pkt->isRouteDirect()) { strcpy(s_bot_path, "known route"); return; } if (n == 0) { strcpy(s_bot_path, "direct"); return; } char* o = s_bot_path; for (uint8_t h = 0; h < n; h++) { @@ -152,9 +157,10 @@ void MyMesh::setBotInbound(const mesh::Packet* pkt, const ContactInfo* sender) { } } -// The contact named `name` (a channel sender: only a name, unverified). +// The contact named `name` (a channel sender: only a name, unverified). From +// MAX_ANON_CONTACTS: the slots before it are anonymous requesters. const ContactInfo* MyMesh::botContactByName(const char* name) { - for (int idx = 0; ; idx++) { + for (int idx = MAX_ANON_CONTACTS; ; idx++) { ContactInfo c; if (!getContactByIdx(idx, c)) return nullptr; if (c.name[0] && strcmp(c.name, name) == 0) return lookupContactByPubKey(c.id.pub_key, PUB_KEY_SIZE); @@ -541,7 +547,7 @@ bool MyMesh::tryBotChannelCommand(uint8_t channel_idx, const char* text, uint8_t uint32_t ts = getRTCClock()->getCurrentTime(); char out[BOT_SCRATCH]; - if (botScanCommands(msg, hops, ts, out, sizeof(out), sender_name, _prefs.bot_actions_ch != 0 && botActionsFrom(botContactByName(sender_name))) == 0) return false; // no commands + if (botScanCommands(msg, hops, ts, out, sizeof(out), sender_name, _prefs.bot_actions_ch != 0 && botActionsFrom(s_bot_sender)) == 0) return false; // no commands if (botInQuietHours()) { resetPendingBotActions(); return true; } // quiet hours if (millis() - _bot_last_ch_reply_ms <= BOT_REPLY_COOLDOWN_MS) { resetPendingBotActions(); return true; } // throttled @@ -576,8 +582,7 @@ bool MyMesh::tryBotRoomCommand(const ContactInfo& from, const uint8_t* sender_pr uint32_t ts = getRTCClock()->getCurrentTime(); char out[BOT_SCRATCH]; - if (botScanCommands(text, hops, ts, out, sizeof(out), sender_name, _prefs.bot_actions_room != 0 && - botActionsFrom(sender_prefix ? lookupContactByPubKey(sender_prefix, 4) : nullptr)) == 0) return false; // no commands + if (botScanCommands(text, hops, ts, out, sizeof(out), sender_name, _prefs.bot_actions_room != 0 && botActionsFrom(s_bot_sender)) == 0) return false; // no commands if (botInQuietHours()) { resetPendingBotActions(); return true; } // quiet hours if (millis() - _bot_last_room_reply_ms <= BOT_REPLY_COOLDOWN_MS) { resetPendingBotActions(); return true; } // throttled diff --git a/examples/companion_radio/ui-core/BotConfig.h b/examples/companion_radio/ui-core/BotConfig.h index b4c600f1..74cc8fce 100644 --- a/examples/companion_radio/ui-core/BotConfig.h +++ b/examples/companion_radio/ui-core/BotConfig.h @@ -57,7 +57,7 @@ static const Row DIRECT_ROWS[] = { static const Row OTHER_ROWS[] = { { QUIET_FROM, "Quiet from", "No replies in these hours" }, { QUIET_TO, "Quiet to", "Same hour = always on" }, - { ACTIONS_SCOPE, "Actions by", "Everyone, or favourites only" }, + { ACTIONS_SCOPE, "Actions by", "Fav: favourites only (a channel name can be faked)" }, }; static int rowCount(int tab) { diff --git a/examples/companion_radio/ui-core/HopRoute.h b/examples/companion_radio/ui-core/HopRoute.h index b8a796c5..5b719a6e 100644 --- a/examples/companion_radio/ui-core/HopRoute.h +++ b/examples/companion_radio/ui-core/HopRoute.h @@ -2,13 +2,13 @@ // A route written in a message -- a bot's {path} / !path reply ("3f:a1:c8", // "3fa1:c802") -- placed on the map: this device, then each hop's repeater, // then the node the message came from when it has a position. The route a -// reply carries is the way our own message went to the bot, so it starts -// here. +// DM reply carries is the way our own message went to the bot, so it starts +// here; on a channel it's whoever asked's, so the caller leaves `own` out. // -// Finding one: two or more hex groups of one width (2 or 4 characters, i.e. -// 1- or 2-byte hashes) joined by ':', standing alone. A clock time (12:30, -// 12:30:45) reads the same, so all-decimal groups that make a time aren't -// one. A single hop ("3f") is too like an ordinary word to tell. +// Finding one: two or more hex groups of one width (2, 4 or 6 characters, +// i.e. 1- to 3-byte hashes) joined by ':', standing alone. A clock time or a +// score (12:30, 45:30:10) reads the same, so two or three all-decimal 2-digit +// groups aren't one. A single hop ("3f") is too like an ordinary word to tell. // // Placing a hop: a repeater with a position whose key starts with the hash // (PacketFlow::placeHash); of several, the nearest the point before it. A hop @@ -27,17 +27,16 @@ public: int8_t hop; // its hop (0-based), -1 here, -2 the sender }; - // The first route in `text`: its hashes (w bytes each) into `hashes`, the - // count into `n`. + // The first route in `text`: its hashes (w bytes each) into `hashes` + // (MAX_HOPS * 3 bytes), the count into `n`. static bool find(const char* text, uint8_t* hashes, uint8_t& n, uint8_t& w) { for (const char* p = text; p && *p; p++) { if (p != text && (isalnum((unsigned char)p[-1]) || p[-1] == ':')) continue; int gw = 0; while (isxdigit((unsigned char)p[gw])) gw++; - if (gw != 2 && gw != 4) continue; + if (gw != 2 && gw != 4 && gw != 6) continue; uint8_t k = 0; bool decimal = true, ok = true; - int vals[3] = {0, 0, 0}; const char* q = p; for (;;) { for (int i = 0; i < gw; i++) if (!isxdigit((unsigned char)q[i])) { ok = false; break; } @@ -46,14 +45,13 @@ public: for (int b = 0; b < gw / 2; b++) hashes[k * (gw / 2) + b] = (uint8_t)((hexv(q[2 * b]) << 4) | hexv(q[2 * b + 1])); } for (int i = 0; i < gw; i++) if (!isdigit((unsigned char)q[i])) decimal = false; - if (k < 3 && gw == 2) vals[k] = (q[0] - '0') * 10 + (q[1] - '0'); k++; q += gw; if (*q != ':') break; q++; } if (!ok || k < 2) continue; - if (decimal && gw == 2 && k <= 3 && vals[0] <= 23 && vals[1] <= 59 && (k < 3 || vals[2] <= 59)) continue; // a time + if (decimal && gw == 2 && k <= 3) continue; // a time, a score n = k > MAX_HOPS ? MAX_HOPS : k; w = (uint8_t)(gw / 2); return true; @@ -61,7 +59,7 @@ public: return false; } static bool has(const char* text) { - uint8_t h[MAX_HOPS * 2], n, w; + uint8_t h[MAX_HOPS * 3], n, w; return find(text, h, n, w); } @@ -124,7 +122,7 @@ private: } } - uint8_t _hash[MAX_HOPS * 2]; + uint8_t _hash[MAX_HOPS * 3]; uint8_t _n = 0, _w = 1, _placed = 0; Pt _pt[MAX_PTS]; int _pts = 0; diff --git a/examples/companion_radio/ui-lvgl/ConversationScreen.h b/examples/companion_radio/ui-lvgl/ConversationScreen.h index ecd882a9..a31d1df8 100644 --- a/examples/companion_radio/ui-lvgl/ConversationScreen.h +++ b/examples/companion_radio/ui-lvgl/ConversationScreen.h @@ -425,7 +425,7 @@ void UITask::messageAction(uint8_t act) { // The way our message went to whoever wrote this: from here, to them. const char* text = m.channel ? s_th_ch[m.pos].text : s_th_dm[m.pos].text; int32_t lat, lon; - const bool own = _core->course.currentLocation(lat, lon) || _core->fixedPosition(lat, lon); + const bool own = !m.channel && (_core->course.currentLocation(lat, lon) || _core->fixedPosition(lat, lon)); // a channel's: someone else's way if (!_core->hop_route.show(text, own, lat, lon, m.channel ? nullptr : _thread_key, m.from)) return; if (_core->hop_route.points() == 0) { showToast("None of its hops has a position"); _core->hop_route.close(); return; } _map_nav = true; // the nodes map (openMap(false)), with the route's bar diff --git a/examples/companion_radio/ui-new/MessagesScreen.h b/examples/companion_radio/ui-new/MessagesScreen.h index 90ca1849..6d474553 100644 --- a/examples/companion_radio/ui-new/MessagesScreen.h +++ b/examples/companion_radio/ui-new/MessagesScreen.h @@ -92,7 +92,7 @@ class MessagesScreen : public UIScreen { // entry has hop data recorded. _fs_act maps each visible row back to an // action so the index math survives the conditional layout. enum FsAct : uint8_t { FS_REPLY, FS_NAV, FS_SAVE, FS_TARGET, FS_PATH, FS_ROUTE }; - uint8_t _fs_act[5]; + uint8_t _fs_act[6]; // Reply, Navigate / Save / Target, Path, Show route int _fs_act_n = 0; // Which history entry the currently-open Options menu (and, after FS_PATH is // chosen, the path/relay detail popup) refers to -- set by buildFsMenu(). @@ -401,7 +401,7 @@ class MessagesScreen : public UIScreen { key = _history.dmAtPos(_fs_menu_pos).prefix; } int32_t lat, lon; - const bool own = _task->currentLocation(lat, lon) || _task->core().fixedPosition(lat, lon); + const bool own = !_fs_menu_is_channel && (_task->currentLocation(lat, lon) || _task->core().fixedPosition(lat, lon)); // a channel's: someone else's way HopRoute& R = _task->core().hop_route; if (!R.show(text, own, lat, lon, key, name)) return; if (R.points() == 0) { R.close(); _task->showAlert("No hop has a position", 1200); }