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

refactor(ui): audit-pass fixes + dead-code/redundancy cleanup

Browse Source 
Fixes from a full ui-new audit (OLED + e-ink both build green):
- remove dead DisplayDriver::drawScrollArrows (all lists use drawScrollIndicator)
- SettingsScreen selection bar -> lineStep()-1, matching the drawList screens
- HomeScreen: don't force the 1s blink refresh on the CLOCK page (status icons
  are hidden there anyway)
- FullscreenMsgView: clamp KEY_DOWN scroll to a cached _max_scroll instead of
  over-incrementing and leaning on the next render to clamp
- DataStore: clamp use_lemon_font (>1 -> 0) on load, like the other enum fields
- move the ~1.5KB wrap scratch (trans/lines) off the render stack into shared
  file-scope statics in FullscreenMsgView (single-threaded UI; the fullscreen
  view and history list never lay out in the same frame)

Cleanup:
- drop dead members RingtoneEditorScreen::DUR_VALS and HomeScreen::sensors_scroll
- delete redundant manual scroll-clamps in handleInput across the drawList
  screens (drawList already reclamps each render); remove the now-unused _visible
  from QuickMsgScreen/NearbyScreen/BotScreen and BotScreen::scrollToSel()

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Jakub
2026-06-16 08:54:11 +02:00
parent da161d49d2
commit f591cddf4f
10 changed files with 60 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
examples/companion_radio/DataStore.cpp
View File

@@ -338,6 +338,9 @@ void DataStore::loadPrefsInt(const char *filename, NodePrefs& _prefs, double& no
// can leave stray bytes here, so clamp out-of-range values back to defaults.
// Values for notif_melody_ad: 0=built-in, 1=melody1, 2=melody2, 3=none.
if (_prefs.notif_melody_ad > 3) _prefs.notif_melody_ad = 0;
// A stale value >1 from an older multi-font build would read as "Lemon" (all
// sites test != 0) until the user toggles Font; clamp it to default here.
if (_prefs.use_lemon_font > 1) _prefs.use_lemon_font = 0;
if (_prefs.units_imperial > 1) _prefs.units_imperial = 0;
if (_prefs.trail_show_pace > 1) _prefs.trail_show_pace = 0;
if (_prefs.advert_sound_scope > 1) _prefs.advert_sound_scope = ADVERT_SOUND_SCOPE_ALL;

17
examples/companion_radio/ui-new/BotScreen.h
View File

@@ -11,8 +11,7 @@ class BotScreen : public UIScreen {
static const int ITEM_COUNT = 9;
int _sel;
int _scroll; // index of first visible item
int _visible; // items fitting on screen; updated each render, used by handleInput
int _scroll; // index of first visible item (managed by drawList in render)
bool _dirty;
// keyboard state (reused for trigger and reply fields)
@@ -32,12 +31,6 @@ class BotScreen : public UIScreen {
}
}
// Keep the selected row inside the visible window (_visible set by render()).
void scrollToSel() {
if (_sel < _scroll) _scroll = _sel;
else if (_sel >= _scroll + _visible) _scroll = _sel - _visible + 1;
}
// Returns index into _channel_indices[] for current bot_channel_idx, or -1 if not found/disabled.
int currentChannelListIdx() const {
if (!_prefs->bot_channel_enabled) return -1;
@@ -52,7 +45,6 @@ public:
void enter() {
_sel = 0;
_scroll = 0;
_visible = 1;
_kb_field = -1;
_dirty = false;
refreshChannels();
@@ -82,7 +74,7 @@ public:
static const char* labels[] = { "Enable", "Channel", "Trigger DM", "Reply DM",
"Trigger Ch", "Reply Ch", "Commands",
"Quiet from", "Quiet to" };
_visible = drawList(display, ITEM_COUNT, _sel, _scroll, [&](int i, int y, bool sel, int reserve) {
drawList(display, ITEM_COUNT, _sel, _scroll, [&](int i, int y, bool sel, int reserve) {
display.drawSelectionRow(0, y - 1, display.width() - reserve, display.lineStep() - 1, sel);
display.setCursor(2, y);
display.print(labels[i]);
@@ -153,8 +145,9 @@ public:
_task->gotoToolsScreen();
return true;
}
if (up && _sel > 0) { _sel--; scrollToSel(); return true; }
if (down && _sel < ITEM_COUNT - 1) { _sel++; scrollToSel(); return true; }
// drawList() reclamps _scroll from _sel every render.
if (up && _sel > 0) { _sel--; return true; }
if (down && _sel < ITEM_COUNT - 1) { _sel++; return true; }
if (_sel == 0 && (enter || left || right)) {
_prefs->bot_enabled ^= 1;

26
examples/companion_radio/ui-new/FullscreenMsgView.h
View File

@@ -6,11 +6,22 @@
static const int FS_CHARS_MAX = 80; // max bytes per wrapped line
// Shared, non-reentrant wrap scratch. The UI renders on a single thread, and the
// 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.
static char s_wrap_trans[512];
static char s_wrap_lines[12][FS_CHARS_MAX];
struct FullscreenMsgView {
int scroll;
bool active;
int _max_scroll; // last value computed in render(); bounds KEY_DOWN
FullscreenMsgView() : scroll(0), active(false) {}
FullscreenMsgView() : scroll(0), active(false), _max_scroll(0) {}
enum Result { NONE, PREV, NEXT, CLOSE, REPLY };
@@ -100,22 +111,21 @@ struct FullscreenMsgView {
}
display.setColor(DisplayDriver::LIGHT);
char trans_text[512];
display.translateUTF8ToBlocks(trans_text, body, sizeof(trans_text));
char lines[12][FS_CHARS_MAX];
int lcount = wrapLines(display, trans_text, max_px, lines, 12);
display.translateUTF8ToBlocks(s_wrap_trans, body, sizeof(s_wrap_trans));
int lcount = wrapLines(display, s_wrap_trans, max_px, s_wrap_lines, 12);
// Reserve the right-edge column for the scroll indicator and re-wrap so text
// can't run under it. Reserve appears only once the list overflows, and a
// narrower second pass only ever yields more lines — so it stays overflowing.
int reserve = scrollIndicatorReserve(display, lcount, visible);
if (reserve > 0)
lcount = wrapLines(display, trans_text, max_px - reserve, lines, 12);
lcount = wrapLines(display, s_wrap_trans, max_px - reserve, s_wrap_lines, 12);
int max_scroll = lcount > visible ? lcount - visible : 0;
_max_scroll = max_scroll;
if (scroll > max_scroll) scroll = max_scroll;
for (int i = 0; i < visible && (scroll + i) < lcount; i++) {
display.setCursor(0, startY + i * lineH);
display.print(lines[scroll + i]);
display.print(s_wrap_lines[scroll + i]);
}
drawScrollIndicator(display, startY, visible * lineH, lcount, visible, scroll);
const int nav_y = display.height() - lineH;
@@ -132,7 +142,7 @@ struct FullscreenMsgView {
Result handleInput(char c) {
if (c == KEY_UP) { if (scroll > 0) scroll--; return NONE; }
if (c == KEY_DOWN) { scroll++; return NONE; }
if (c == KEY_DOWN) { if (scroll < _max_scroll) scroll++; return NONE; }
if (c == KEY_LEFT) return NEXT;
if (c == KEY_RIGHT) return PREV;
if (c == KEY_CONTEXT_MENU) return REPLY;

19
examples/companion_radio/ui-new/NearbyScreen.h
View File

@@ -17,8 +17,6 @@
class NearbyScreen : public UIScreen {
UITask* _task;
int _visible = 4; // updated each render; used by handleInput for scroll clamping
// ── filter (type axis) ──────────────────────────────────────────────────────
enum Filter : uint8_t { F_ALL, F_FAV, F_COMP, F_RPT, F_ROOM, F_SNSR, F_COUNT };
static const char* FILTER_LABELS[F_COUNT];
@@ -550,9 +548,7 @@ public:
}
int item_h = display.lineStep();
int start_y = display.listStart();
int dist_col = display.width() - display.getCharWidth() * 7;
_visible = display.listVisible(item_h);
display.setColor(DisplayDriver::LIGHT);
char title[28];
@@ -587,7 +583,7 @@ public:
display.drawTextCentered(display.width() / 2, display.height() / 2 + display.lineStep() / 2, hint);
}
} else {
_visible = drawList(display, _count, _sel, _scroll, [&](int idx, int y, bool sel, int reserve) {
drawList(display, _count, _sel, _scroll, [&](int idx, int y, bool sel, int reserve) {
const Entry& e = _entries[idx];
display.drawSelectionRow(0, y - 1, display.width() - reserve, item_h - 1, sel);
@@ -671,16 +667,9 @@ public:
return true;
}
if (c == KEY_CONTEXT_MENU) { openActionMenu(); return true; }
if (c == KEY_UP && _sel > 0) {
_sel--;
if (_sel < _scroll) _scroll = _sel;
return true;
}
if (c == KEY_DOWN && _sel < _count - 1) {
_sel++;
if (_sel >= _scroll + _visible) _scroll = _sel - _visible + 1;
return true;
}
// drawList() reclamps _scroll from _sel every render.
if (c == KEY_UP && _sel > 0) { _sel--; return true; }
if (c == KEY_DOWN && _sel < _count - 1) { _sel++; return true; }
if (c == KEY_ENTER) {
if (_count == 0) { if (_source == SRC_STORED) enterScan(); return true; }
_detail = true;

75
examples/companion_radio/ui-new/QuickMsgScreen.h
View File

@@ -129,7 +129,6 @@ class QuickMsgScreen : public UIScreen {
int _dm_hist_sel, _dm_hist_scroll;
FullscreenMsgView _dm_fs;
int _visible = 4; // updated in render(); used by handleInput() for scroll clamping
int _hist_visible = 2; // updated in render(); for history list scroll clamping
void expandMsg(const char* tmpl, char* out, int out_len) const {
@@ -819,7 +818,6 @@ public:
int item_h = display.lineStep();
int start_y = display.listStart();
int cw = display.getCharWidth();
_visible = display.listVisible(item_h);
if (_phase == MODE_SELECT) {
display.drawCenteredHeader("MESSAGE");
@@ -854,7 +852,7 @@ public:
return 5000;
}
_visible = drawList(display, _num_contacts, _contact_sel, _contact_scroll, [&](int list_idx, int y, bool sel, int reserve) {
drawList(display, _num_contacts, _contact_sel, _contact_scroll, [&](int list_idx, int y, bool sel, int reserve) {
int mesh_idx = _sorted[list_idx];
display.drawSelectionRow(0, y - 1, display.width() - reserve, item_h - 1, sel);
@@ -889,7 +887,7 @@ public:
return 5000;
}
_visible = drawList(display, _num_channels, _channel_sel, _channel_scroll, [&](int list_idx, int y, bool sel, int reserve) {
drawList(display, _num_channels, _channel_sel, _channel_scroll, [&](int list_idx, int y, bool sel, int reserve) {
display.drawSelectionRow(0, y - 1, display.width() - reserve, item_h - 1, sel);
ChannelDetails ch;
if (the_mesh.getChannel(_channel_indices[list_idx], ch)) {
@@ -966,10 +964,8 @@ public:
if (portrait_expand) {
int rp = dmHistEntryForContact(_sel_contact.id.pub_key, _dm_hist_scroll + ii);
if (rp >= 0) {
char trans[160];
display.translateUTF8ToBlocks(trans, skipReplyPrefix(_dm_hist[rp].text), sizeof(trans));
char wl[8][FS_CHARS_MAX];
int nl = FullscreenMsgView::wrapLines(display, trans, display.width() - 6 - reserve, wl, 8);
display.translateUTF8ToBlocks(s_wrap_trans, skipReplyPrefix(_dm_hist[rp].text), sizeof(s_wrap_trans));
int nl = FullscreenMsgView::wrapLines(display, s_wrap_trans, display.width() - 6 - reserve, s_wrap_lines, 8);
bh = (1 + (nl > 0 ? nl : 1)) * lh + 1;
}
}
@@ -1012,11 +1008,9 @@ public:
if (age[0]) { display.setCursor(display.width() - age_w - reserve, y + 1); display.print(age); }
if (!sel) display.setColor(DisplayDriver::LIGHT);
if (portrait_expand) {
char trans[160];
display.translateUTF8ToBlocks(trans, skipReplyPrefix(e.text), sizeof(trans));
char wl[8][FS_CHARS_MAX];
int nl = FullscreenMsgView::wrapLines(display, trans, display.width() - 6 - reserve, wl, 8);
for (int li = 0; li < nl; li++) { display.setCursor(3, y + (li + 1) * lh + 1); display.print(wl[li]); }
display.translateUTF8ToBlocks(s_wrap_trans, skipReplyPrefix(e.text), sizeof(s_wrap_trans));
int nl = FullscreenMsgView::wrapLines(display, s_wrap_trans, display.width() - 6 - reserve, s_wrap_lines, 8);
for (int li = 0; li < nl; li++) { display.setCursor(3, y + (li + 1) * lh + 1); display.print(s_wrap_lines[li]); }
} else {
display.drawTextEllipsized(3, y + lh + 1, display.width() - cw - 2, skipReplyPrefix(e.text));
}
@@ -1109,10 +1103,8 @@ public:
const char* rtext = _hist[rp].text;
const char* rsep = strstr(rtext, ": ");
const char* rbody = rsep ? rsep + 2 : rtext;
char trans[160];
display.translateUTF8ToBlocks(trans, skipReplyPrefix(rbody), sizeof(trans));
char wl[8][FS_CHARS_MAX];
int nl = FullscreenMsgView::wrapLines(display, trans, display.width() - 6 - reserve, wl, 8);
display.translateUTF8ToBlocks(s_wrap_trans, skipReplyPrefix(rbody), sizeof(s_wrap_trans));
int nl = FullscreenMsgView::wrapLines(display, s_wrap_trans, display.width() - 6 - reserve, s_wrap_lines, 8);
bh = (1 + (nl > 0 ? nl : 1)) * lh + 1;
}
}
@@ -1170,11 +1162,9 @@ public:
if (age[0]) { display.setCursor(display.width() - age_w - reserve, y + 1); display.print(age); }
if (!sel) display.setColor(DisplayDriver::LIGHT);
if (portrait_expand) {
char trans[160];
display.translateUTF8ToBlocks(trans, skipReplyPrefix(msg_part), sizeof(trans));
char wl[8][FS_CHARS_MAX];
int nl = FullscreenMsgView::wrapLines(display, trans, display.width() - 6 - reserve, wl, 8);
for (int li = 0; li < nl; li++) { display.setCursor(3, y + (li + 1) * lh + 1); display.print(wl[li]); }
display.translateUTF8ToBlocks(s_wrap_trans, skipReplyPrefix(msg_part), sizeof(s_wrap_trans));
int nl = FullscreenMsgView::wrapLines(display, s_wrap_trans, display.width() - 6 - reserve, s_wrap_lines, 8);
for (int li = 0; li < nl; li++) { display.setCursor(3, y + (li + 1) * lh + 1); display.print(s_wrap_lines[li]); }
} else {
display.drawTextEllipsized(3, y + lh + 1, display.width() - cw - 2, skipReplyPrefix(msg_part));
}
@@ -1233,7 +1223,7 @@ public:
display.drawCenteredHeader(title);
int total_msg_items = 1 + _active_msg_count;
_visible = drawList(display, total_msg_items, _msg_sel, _msg_scroll, [&](int idx, int y, bool sel, int reserve) {
drawList(display, total_msg_items, _msg_sel, _msg_scroll, [&](int idx, int y, bool sel, int reserve) {
display.drawSelectionRow(0, y - 1, display.width() - reserve, item_h - 1, sel);
if (idx == 0) {
@@ -1398,16 +1388,9 @@ public:
return true;
}
if (c == KEY_CANCEL) { _room_mode = false; _phase = MODE_SELECT; return true; }
if (c == KEY_UP && _contact_sel > 0) {
_contact_sel--;
if (_contact_sel < _contact_scroll) _contact_scroll = _contact_sel;
return true;
}
if (c == KEY_DOWN && _contact_sel < _num_contacts - 1) {
_contact_sel++;
if (_contact_sel >= _contact_scroll + _visible) _contact_scroll = _contact_sel - _visible + 1;
return true;
}
// drawList() reclamps _contact_scroll from _contact_sel every render.
if (c == KEY_UP && _contact_sel > 0) { _contact_sel--; return true; }
if (c == KEY_DOWN && _contact_sel < _num_contacts - 1) { _contact_sel++; return true; }
if (c == KEY_ENTER && _num_contacts > 0) {
if (the_mesh.getContactByIdx(_sorted[_contact_sel], _sel_contact)) {
_task->clearDMUnread(_sel_contact.id.pub_key);
@@ -1495,16 +1478,9 @@ public:
return true;
}
if (c == KEY_CANCEL) { _phase = MODE_SELECT; return true; }
if (c == KEY_UP && _channel_sel > 0) {
_channel_sel--;
if (_channel_sel < _channel_scroll) _channel_scroll = _channel_sel;
return true;
}
if (c == KEY_DOWN && _channel_sel < _num_channels - 1) {
_channel_sel++;
if (_channel_sel >= _channel_scroll + _visible) _channel_scroll = _channel_sel - _visible + 1;
return true;
}
// drawList() reclamps _channel_scroll from _channel_sel every render.
if (c == KEY_UP && _channel_sel > 0) { _channel_sel--; return true; }
if (c == KEY_DOWN && _channel_sel < _num_channels - 1) { _channel_sel++; return true; }
if (c == KEY_ENTER && _num_channels > 0) {
_sel_channel_idx = _channel_indices[_channel_sel];
int hc = histCountForChannel(_sel_channel_idx);
@@ -1727,16 +1703,9 @@ public:
_phase = _sending_to_channel ? CHANNEL_HIST : DM_HIST;
return true;
}
if (c == KEY_UP && _msg_sel > 0) {
_msg_sel--;
if (_msg_sel < _msg_scroll) _msg_scroll = _msg_sel;
return true;
}
if (c == KEY_DOWN && _msg_sel < total_msg_items - 1) {
_msg_sel++;
if (_msg_sel >= _msg_scroll + _visible) _msg_scroll = _msg_sel - _visible + 1;
return true;
}
// drawList() reclamps _msg_scroll from _msg_sel every render.
if (c == KEY_UP && _msg_sel > 0) { _msg_sel--; return true; }
if (c == KEY_DOWN && _msg_sel < total_msg_items - 1) { _msg_sel++; return true; }
if (c == KEY_ENTER) {
if (_msg_sel == 0) {
_kb->begin(_reply_mode ? _reply_prefix : "");

2
examples/companion_radio/ui-new/RingtoneEditorScreen.h
View File

@@ -14,7 +14,6 @@ class RingtoneEditorScreen : public UIScreen {
int _visible_notes = 7; // updated in render(); used by clampScroll()
static const uint16_t BPM_OPTS[5];
static const uint8_t DUR_VALS[4];
static const char* DUR_LABELS[4];
static const char PITCH_NAMES[8]; // lowercase rtttl names
@@ -293,6 +292,5 @@ public:
};
const uint16_t RingtoneEditorScreen::BPM_OPTS[5] = { 60, 90, 120, 150, 180 };
const uint8_t RingtoneEditorScreen::DUR_VALS[4] = { 4, 8, 16, 32 };
const char* RingtoneEditorScreen::DUR_LABELS[4] = { "1/4", "1/8", "1/16", "1/32" };
const char RingtoneEditorScreen::PITCH_NAMES[8] = { 'p', 'c', 'd', 'e', 'f', 'g', 'a', 'b' };

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

@@ -413,7 +413,7 @@ class SettingsScreen : public UIScreen {
bool collapsed = (_collapsed >> si) & 1;
bool sel = (item == _selected);
display.setColor(DisplayDriver::LIGHT);
display.drawSelectionRow(0, y - 1, display.width() - _reserve, display.lineStep(), sel);
display.drawSelectionRow(0, y - 1, display.width() - _reserve, display.lineStep() - 1, sel);
display.setCursor(2, y);
display.print(collapsed ? "+" : "-");
display.print(" ");
@@ -422,7 +422,7 @@ class SettingsScreen : public UIScreen {
}
bool sel = (item == _selected);
display.drawSelectionRow(0, y - 1, display.width() - _reserve, display.lineStep(), sel);
display.drawSelectionRow(0, y - 1, display.width() - _reserve, display.lineStep() - 1, sel);
display.setCursor(2, y);

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

@@ -482,7 +482,6 @@ class HomeScreen : public UIScreen {
CayenneLPP sensors_lpp;
int sensors_nb = 0;
bool sensors_scroll = false;
int sensors_scroll_offset = 0;
int next_sensors_refresh = 0;
@@ -498,7 +497,6 @@ class HomeScreen : public UIScreen {
reader.skipData(type);
sensors_nb ++;
}
sensors_scroll = sensors_nb > UI_RECENT_LIST_SIZE;
#if AUTO_OFF_MILLIS > 0
next_sensors_refresh = millis() + 5000; // refresh sensor values every 5 sec
#else
@@ -992,8 +990,10 @@ public:
}
}
bool auto_adv = _node_prefs && _node_prefs->advert_auto_interval_sec > 0;
// Any blinking status-bar indicator needs a 1 s refresh to animate evenly.
bool need_blink = auto_adv || _task->trail().isActive();
// Any blinking status-bar indicator needs a 1 s refresh to animate evenly —
// but the status bar (and its icons) is hidden on the CLOCK page, so don't
// pay the 1 s cadence there for icons that aren't drawn.
bool need_blink = (_page != HomePage::CLOCK) && (auto_adv || _task->trail().isActive());
if (Features::IS_EINK) {
// slow display: poll every 30 s; inbound msgs force immediate refresh via notify()
return Features::HOME_REFRESH_MS;

3
examples/companion_radio/ui-new/WaypointsView.h
View File

@@ -319,7 +319,8 @@ public:
int n = wpListCount();
int total = n + 1; // + the "Add by coords" row
if (c == KEY_CANCEL) { _mode = OFF; return true; }
if (c == KEY_UP && _sel > 0) { _sel--; if (_sel < _scroll) _scroll = _sel; return true; }
// drawList() reclamps _scroll from _sel every render.
if (c == KEY_UP && _sel > 0) { _sel--; return true; }
if (c == KEY_DOWN && _sel < total - 1) { _sel++; return true; }
if (c == KEY_ENTER) {
if (_sel == n) openAddForm(); // last row → open the add form

10
src/helpers/ui/DisplayDriver.h
View File

@@ -162,16 +162,6 @@ public:
}
}
// Up/down scroll indicators in the right-edge column. top_y is the first
// row's y, bottom_y the last visible row's y. Replaces the 4-line
// setCursor/print("^")/setCursor/print("v") block in every scrollable list.
void drawScrollArrows(int top_y, int bottom_y, bool more_up, bool more_down) {
int x = width() - getCharWidth();
setColor(LIGHT);
if (more_up) { setCursor(x, top_y); print("^"); }
if (more_down) { setCursor(x, bottom_y); print("v"); }
}
// Inverted title bar: light background, dark ellipsized label, then the
// standard separator line. The label is UTF-8 translated by
// drawTextEllipsized. Leaves ink colour LIGHT for following content.