Mirrors/MeshCore-Solo
1
0
Fork 0
mirror of https://github.com/MarekZegare4/MeshCore-Solo.git synced 2026-09-28 21:56:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(ui): apply code review fixes — bugs, risk, optimisations, cleanup

Browse Source 
Fixes 7 bugs (timing UB on unsigned long, abs() no-op, XBM CRC PROGMEM
scan, CayenneLPP heap thrash ×2, UTF-8 reply prefix, lock-seq reset on
wake), 3 risk items (notif melody stop guard, bot trigger pre-lowercase,
lock-seq cleared on display wake), plus optimisations (fmtMsgAge/
getTextWidth caching, RTTTL builder consolidated to NodePrefs, HP_*
constants and homePageLabel table moved to NodePrefs, DataStore flat
lambda schema, translateUTF8Static factored out) and minor cleanup.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Jakub
2026-05-23 20:03:45 +02:00
co-authored by Claude Sonnet 4.6
parent 05335420e3
commit b279b95b2d
16 changed files with 198 additions and 245 deletions
Download Patch File Download Diff File Expand all files Collapse all files
examples/companion_radio/ui-new/BotScreen.h
+2 -1
View File
@@ -117,6 +117,7 @@ public:
if (_kb_field == 2) {
strncpy(_prefs->bot_trigger, _kb.buf, sizeof(_prefs->bot_trigger) - 1);
_prefs->bot_trigger[sizeof(_prefs->bot_trigger) - 1] = '\0';
for (char* p = _prefs->bot_trigger; *p; p++) *p = (char)tolower((uint8_t)*p);
} else if (_kb_field == 3) {
strncpy(_prefs->bot_reply_dm, _kb.buf, sizeof(_prefs->bot_reply_dm) - 1);
_prefs->bot_reply_dm[sizeof(_prefs->bot_reply_dm) - 1] = '\0';
@@ -184,7 +185,7 @@ public:
}
_kb.begin(initial, max);
if (_sel == 2) {
_kb._ph_count = 0; // trigger is literal text — placeholders never match incoming msgs
_kb.clearPlaceholders(); // trigger is literal text — placeholders never match incoming msgs
} else {
kbAddSensorPlaceholders(_kb, &sensors);
}
examples/companion_radio/ui-new/KeyboardWidget.h
+2
View File
@@ -46,6 +46,8 @@ struct KeyboardWidget {
addPlaceholder("{time}");
}
void clearPlaceholders() { _ph_count = 0; }
void addPlaceholder(const char* ph) {
if (_ph_count < KB_PH_MAX) {
strncpy(_ph_buf[_ph_count], ph, KB_PH_LEN - 1);
examples/companion_radio/ui-new/QuickMsgScreen.h
+9 -6
View File
@@ -125,8 +125,7 @@ class QuickMsgScreen : public UIScreen {
}
static void fmtMsgAge(char* buf, int n, uint32_t timestamp) {
uint32_t now = rtc_clock.getCurrentTime();
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);
@@ -645,6 +644,7 @@ public:
display.fillRect(0, lh + 1, display.width(), display.sepH());
int dm_count = dmHistCountForContact(_sel_contact.id.pub_key);
uint32_t now_ts = rtc_clock.getCurrentTime();
for (int i = 0; i < _hist_visible && (_dm_hist_scroll + i) < dm_count; i++) {
int item = _dm_hist_scroll + i;
@@ -657,7 +657,7 @@ public:
const DmHistEntry& e = _dm_hist[ring_pos];
const char* sender = e.outgoing ? "Me" : filtered_name;
char age[6]; fmtMsgAge(age, sizeof(age), e.timestamp);
char age[6]; fmtMsgAge(age, sizeof(age), e.timestamp, now_ts);
int age_w = age[0] ? display.getTextWidth(age) + 3 : 0;
if (sel) {
@@ -748,6 +748,7 @@ public:
display.fillRect(0, lh + 1, display.width(), display.sepH());
int ch_hist_count = histCountForChannel(_sel_channel_idx);
uint32_t now_ts = rtc_clock.getCurrentTime();
for (int i = 0; i < _hist_visible && (_hist_scroll + i) < ch_hist_count; i++) {
int item = _hist_scroll + i;
@@ -771,7 +772,7 @@ public:
msg_part[sizeof(msg_part) - 1] = '\0';
}
char age[6]; fmtMsgAge(age, sizeof(age), _hist[ring_pos].timestamp);
char age[6]; fmtMsgAge(age, sizeof(age), _hist[ring_pos].timestamp, now_ts);
int age_w = age[0] ? display.getTextWidth(age) + 3 : 0;
if (sel) {
@@ -1048,7 +1049,8 @@ public:
} else if (res == FullscreenMsgView::REPLY) {
int ring_pos = dmHistEntryForContact(_sel_contact.id.pub_key, _dm_hist_sel);
if (ring_pos >= 0 && !_dm_hist[ring_pos].outgoing) {
snprintf(_reply_prefix, sizeof(_reply_prefix), "@[%.31s] ", _sel_contact.name);
char _tname[32]; DisplayDriver::translateUTF8Static(_tname, _sel_contact.name, sizeof(_tname));
snprintf(_reply_prefix, sizeof(_reply_prefix), "@[%.31s] ", _tname);
_ctx_menu.begin("Options", 1);
_ctx_menu.addItem("Reply");
}
@@ -1096,7 +1098,8 @@ public:
if (c == KEY_CONTEXT_MENU && _dm_hist_sel >= 0) {
int ring_pos = dmHistEntryForContact(_sel_contact.id.pub_key, _dm_hist_sel);
if (ring_pos >= 0 && !_dm_hist[ring_pos].outgoing) {
snprintf(_reply_prefix, sizeof(_reply_prefix), "@[%.31s] ", _sel_contact.name);
char _tname[32]; DisplayDriver::translateUTF8Static(_tname, _sel_contact.name, sizeof(_tname));
snprintf(_reply_prefix, sizeof(_reply_prefix), "@[%.31s] ", _tname);
_ctx_menu.begin("Options", 1);
_ctx_menu.addItem("Reply");
}
examples/companion_radio/ui-new/RingtoneEditorScreen.h
+1 -13
View File
@@ -47,19 +47,7 @@ class RingtoneEditorScreen : public UIScreen {
}
void buildRTTTL() {
uint16_t bpm = BPM_OPTS[_bpm_idx < 5 ? _bpm_idx : 2];
int pos = snprintf(_play_buf, sizeof(_play_buf), "Ring:d=8,o=5,b=%u:", bpm);
for (int i = 0; i < _len && pos < (int)sizeof(_play_buf) - 8; i++) {
if (i > 0 && pos < (int)sizeof(_play_buf) - 1) _play_buf[pos++] = ',';
uint8_t pitch = notePitch(_notes[i]);
uint8_t octave = noteOctave(_notes[i]);
uint8_t dur_val = DUR_VALS[noteDurIdx(_notes[i])];
if (pitch == 0)
pos += snprintf(_play_buf + pos, sizeof(_play_buf) - pos, "%dp", dur_val);
else
pos += snprintf(_play_buf + pos, sizeof(_play_buf) - pos, "%d%c%d", dur_val, PITCH_NAMES[pitch], octave);
}
if (pos < (int)sizeof(_play_buf)) _play_buf[pos] = '\0';
NodePrefs::buildRTTTLString(_notes, _len, _bpm_idx, _play_buf, sizeof(_play_buf));
}
void previewNote(uint8_t note_byte) {
examples/companion_radio/ui-new/SettingsScreen.h
+6 -33
View File
@@ -168,47 +168,20 @@ class SettingsScreen : public UIScreen {
}
uint16_t homePageBit(int item) const {
if (item == HOME_CLOCK) return HP_CLOCK;
if (item == HOME_RECENT) return HP_RECENT;
if (item == HOME_RADIO) return HP_RADIO;
if (item == HOME_BT) return HP_BLUETOOTH;
if (item == HOME_ADVERT) return HP_ADVERT;
if (item == HOME_TOOLS) return HP_TOOLS;
if (item == HOME_SHUTDOWN) return HP_SHUTDOWN;
// HOME_SETTINGS and HOME_QUICK_MSG have no mask bit — always visible
#if ENV_INCLUDE_GPS == 1
if (item == HOME_GPS) return HP_GPS;
#endif
#if UI_SENSORS_PAGE == 1
if (item == HOME_SENSORS) return HP_SENSORS;
#endif
return 0;
int bit = homePageBitIndex(item);
return (bit >= 0 && bit < 9) ? (uint16_t)(1 << bit) : 0;
}
const char* homePageLabel(int item) const {
if (item == HOME_CLOCK) return "Clock";
if (item == HOME_RECENT) return "Recent";
if (item == HOME_RADIO) return "Radio";
if (item == HOME_BT) return "Bluetooth";
if (item == HOME_ADVERT) return "Advert";
if (item == HOME_TOOLS) return "Tools";
if (item == HOME_SHUTDOWN) return "Shutdown";
if (item == HOME_SETTINGS) return "Settings";
if (item == HOME_QUICK_MSG) return "Messages";
#if ENV_INCLUDE_GPS == 1
if (item == HOME_GPS) return "GPS";
#endif
#if UI_SENSORS_PAGE == 1
if (item == HOME_SENSORS) return "Sensors";
#endif
return "";
int bit = homePageBitIndex(item);
return NodePrefs::homePageLabel((uint8_t)(bit >= 0 ? bit : 11));
}
bool homePageVisible(int item, const NodePrefs* p) const {
if (item == HOME_SETTINGS || item == HOME_QUICK_MSG) return true;
uint16_t bit = homePageBit(item);
if (!bit) return false;
uint16_t mask = (p && p->home_pages_mask) ? p->home_pages_mask : HP_ALL;
uint16_t mask = (p && p->home_pages_mask) ? p->home_pages_mask : NodePrefs::HP_ALL;
return (mask & bit) != 0;
}
@@ -600,7 +573,7 @@ public:
return true;
}
if (enter && homePageToggleable(_selected)) {
if (!p->home_pages_mask) p->home_pages_mask = HP_ALL;
if (!p->home_pages_mask) p->home_pages_mask = NodePrefs::HP_ALL;
p->home_pages_mask ^= homePageBit(_selected);
_dirty = true;
return true;
examples/companion_radio/ui-new/UITask.cpp
+14 -42
View File
@@ -111,18 +111,6 @@ public:
static const int QUICK_MSGS_MAX = 10;
// Bit positions for NodePrefs::home_pages_mask.
// Bit=1 means page is shown. 0 in the field means ALL visible (default/unset).
static const uint16_t HP_CLOCK = 1 << 0;
static const uint16_t HP_RECENT = 1 << 1;
static const uint16_t HP_RADIO = 1 << 2;
static const uint16_t HP_BLUETOOTH = 1 << 3;
static const uint16_t HP_ADVERT = 1 << 4;
static const uint16_t HP_GPS = 1 << 5;
static const uint16_t HP_SENSORS = 1 << 6;
static const uint16_t HP_TOOLS = 1 << 7;
static const uint16_t HP_SHUTDOWN = 1 << 8;
static const uint16_t HP_ALL = 0x01FF;
#include "KeyboardWidget.h"
#include "FullscreenMsgView.h"
@@ -216,7 +204,7 @@ class HomeScreen : public UIScreen {
bool isPageVisible(int page) const {
int bit = pageBit(page);
if (bit < 0) return true;
uint16_t mask = (_node_prefs && _node_prefs->home_pages_mask) ? _node_prefs->home_pages_mask : HP_ALL;
uint16_t mask = (_node_prefs && _node_prefs->home_pages_mask) ? _node_prefs->home_pages_mask : NodePrefs::HP_ALL;
return (mask >> bit) & 1;
}
@@ -698,7 +686,6 @@ public:
// scan LPP buffer for this type
LPPReader r(sensors_lpp.getBuffer(), sensors_lpp.getSize());
uint8_t ch, type;
bool found = false;
char buf[22] = "--";
while (r.readHeader(ch, type)) {
if (type == target) {
@@ -721,12 +708,10 @@ public:
case LPP_CONCENTRATION: r.readConcentration(v); snprintf(buf, sizeof(buf), "%.0fppm", v); break;
default: r.skipData(type); continue;
}
found = true;
break;
}
r.skipData(type);
}
(void)found;
static const struct { uint8_t type; const char* name; } TYPE_NAMES[] = {
{ LPP_VOLTAGE, "voltage" },
@@ -1015,24 +1000,10 @@ void UITask::showAlert(const char* text, int duration_millis) {
}
static void buildMelodyFromPrefs(const NodePrefs* p, int slot, char* buf, int size) {
static const uint16_t BPM_OPTS[] = { 60, 90, 120, 150, 180 };
static const uint8_t DUR_VALS[] = { 4, 8, 16, 32 };
static const char PITCHES[] = { 'p', 'c', 'd', 'e', 'f', 'g', 'a', 'b' };
const uint8_t* notes = (slot == 2) ? p->ringtone2_notes : p->ringtone_notes;
uint8_t len = (slot == 2) ? p->ringtone2_len : p->ringtone_len;
uint8_t bpm_i = (slot == 2) ? p->ringtone2_bpm_idx : p->ringtone_bpm_idx;
if (len == 0) { buf[0] = '\0'; return; }
uint16_t bpm = BPM_OPTS[bpm_i < 5 ? bpm_i : 2];
int pos = snprintf(buf, size, "Ring:d=8,o=5,b=%u:", bpm);
for (int i = 0; i < len && pos < size - 8; i++) {
if (i > 0 && pos < size - 1) buf[pos++] = ',';
uint8_t pitch = notes[i] & 0x07;
uint8_t octave = ((notes[i] >> 3) & 0x03) + 4;
uint8_t dur_val = DUR_VALS[(notes[i] >> 5) & 0x03];
if (pitch == 0) pos += snprintf(buf + pos, size - pos, "%dp", dur_val);
else pos += snprintf(buf + pos, size - pos, "%d%c%d", dur_val, PITCHES[pitch], octave);
}
if (pos < size) buf[pos] = '\0';
const uint8_t* notes = (slot == 2) ? p->ringtone2_notes : p->ringtone_notes;
uint8_t len = (slot == 2) ? p->ringtone2_len : p->ringtone_len;
uint8_t bpm_i = (slot == 2) ? p->ringtone2_bpm_idx : p->ringtone_bpm_idx;
NodePrefs::buildRTTTLString(notes, len, bpm_i, buf, size);
}
void UITask::notify(UIEventType t) {
@@ -1066,6 +1037,7 @@ switch(t){
}
bool custom_played = false;
if (slot > 0 && _node_prefs) {
if (buzzer.isPlaying()) buzzer.stop(); // stop before overwriting _notif_mel_buf
buildMelodyFromPrefs(_node_prefs, slot, _notif_mel_buf, sizeof(_notif_mel_buf));
if (_notif_mel_buf[0]) {
if (force) buzzer.playForced(_notif_mel_buf); else buzzer.play(_notif_mel_buf);
@@ -1101,6 +1073,7 @@ switch(t){
}
bool custom_played = false;
if (slot > 0 && _node_prefs) {
if (buzzer.isPlaying()) buzzer.stop(); // stop before overwriting _notif_mel_buf
buildMelodyFromPrefs(_node_prefs, slot, _notif_mel_buf, sizeof(_notif_mel_buf));
if (_notif_mel_buf[0]) {
if (force) buzzer.playForced(_notif_mel_buf); else buzzer.play(_notif_mel_buf);
@@ -1181,7 +1154,7 @@ void UITask::newMsg(uint8_t path_len, const char* from_name, const char* text, i
void UITask::userLedHandler() {
#ifdef PIN_STATUS_LED
int cur_time = millis();
unsigned long cur_time = millis();
if (cur_time > next_led_change) {
if (led_state == 0) {
led_state = 1;
@@ -1276,8 +1249,7 @@ static void formatDashVal(uint8_t field, char* val, int val_len, uint16_t batt_m
case DASH_CO2: lpp_type = LPP_CONCENTRATION; break;
}
if (lpp_type) {
CayenneLPP local_lpp(200);
if (!lpp) { local_lpp.reset(); sensors.querySensors(0xFF, local_lpp); lpp = &local_lpp; }
if (!lpp) { static CayenneLPP s_lpp(200); s_lpp.reset(); sensors.querySensors(0xFF, s_lpp); lpp = &s_lpp; }
LPPReader r(lpp->getBuffer(), lpp->getSize());
uint8_t ch, type;
while (r.readHeader(ch, type)) {
@@ -1383,7 +1355,7 @@ void UITask::loop() {
}
#endif
#if defined(PIN_USER_BTN_ANA)
if (abs(millis() - _analogue_pin_read_millis) > 10) {
if (millis() - _analogue_pin_read_millis > 10) {
ev = analog_btn.check();
if (ev == BUTTON_EVENT_CLICK) {
c = checkDisplayOn(KEY_NEXT);
@@ -1474,14 +1446,13 @@ void UITask::loop() {
// Two sensor values side by side (dashboard_fields[0] and [1])
if (_node_prefs) {
char v0[20] = "", v1[20] = "";
CayenneLPP shared_lpp(200);
CayenneLPP* lpp_ptr = nullptr;
uint8_t f0 = _node_prefs->dashboard_fields[0], f1 = _node_prefs->dashboard_fields[1];
auto isLPP = [](uint8_t f) {
return f==DASH_TEMP||f==DASH_HUM||f==DASH_PRES||f==DASH_ALT||f==DASH_LUX||f==DASH_CO2;
};
if (isLPP(f0) || isLPP(f1)) {
shared_lpp.reset(); sensors.querySensors(0xFF, shared_lpp); lpp_ptr = &shared_lpp;
_dash_lpp.reset(); sensors.querySensors(0xFF, _dash_lpp); lpp_ptr = &_dash_lpp;
}
formatDashVal(f0, v0, sizeof(v0), _batt_mv, lpp_ptr);
formatDashVal(f1, v1, sizeof(v1), _batt_mv, lpp_ptr);
@@ -1597,6 +1568,8 @@ char UITask::checkDisplayOn(char c) {
_next_refresh = 0;
return 0; // eat the waking key press
}
_lock_seq_count = 0;
_lock_seq_used = false;
c = 0;
}
if (!_locked) {
@@ -1627,8 +1600,7 @@ char UITask::handleTripleClick(char c) {
MESH_DEBUG_PRINTLN("UITask: triple click triggered");
checkDisplayOn(c);
toggleBuzzer();
c = 0;
return c;
return 0;
}
bool UITask::getGPSState() {
examples/companion_radio/ui-new/UITask.h
+5 -4
View File
@@ -51,11 +51,11 @@ class UITask : public AbstractUITask {
DMUnreadEntry _dm_unread_table[DM_UNREAD_TABLE_SIZE];
unsigned long ui_started_at, next_batt_chck;
uint16_t _batt_mv; // EMA-filtered battery voltage
int next_backlight_btn_check = 0;
unsigned long next_backlight_btn_check = 0;
#ifdef PIN_STATUS_LED
int led_state = 0;
int next_led_change = 0;
int last_led_increment = 0;
unsigned long next_led_change = 0;
unsigned long last_led_increment = 0;
#endif
#ifdef PIN_USER_BTN_ANA
@@ -73,6 +73,7 @@ class UITask : public AbstractUITask {
UIScreen* dashboard_config;
UIScreen* auto_advert_screen;
UIScreen* curr;
CayenneLPP _dash_lpp;
void userLedHandler();
@@ -86,7 +87,7 @@ class UITask : public AbstractUITask {
public:
UITask(mesh::MainBoard* board, BaseSerialInterface* serial) : AbstractUITask(board, serial), _display(NULL), _sensors(NULL), _node_prefs(NULL) {
UITask(mesh::MainBoard* board, BaseSerialInterface* serial) : AbstractUITask(board, serial), _display(NULL), _sensors(NULL), _node_prefs(NULL), _dash_lpp(200) {
next_batt_chck = _next_refresh = 0;
ui_started_at = 0;
_batt_mv = 0;