mirror of
https://github.com/MarekZegare4/MeshCore-Solo.git
synced 2026-07-26 14:58:12 +00:00
chore: address code-audit hygiene (stale refs, dead TODO, popup scroll)
- FEATURES audit: refresh references the Nearby/Trail refactors made stale — M4 (renderDiscoverDetail → renderScanDetail), L1 (SNR %.1f now in scan detail + ping; list cards show RSSI), Trail grid (round-step renderGrid + MIN_GRID_PX). - MyMesh: drop the obsolete `// TODO: add expected ACK to table` — the code right below already populates expected_ack_table. - PopupMenu: render() is now the single source of truth for _scroll, clamping the selection into view from the live-height cap, so handleInput() only moves _sel and never depends on a stale _cap (M3). Behaviour-preserving. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
12
FEATURES.md
12
FEATURES.md
@@ -766,11 +766,11 @@ The 31-byte name slot in `CMD_SET_DEFAULT_FLOOD_SCOPE` doesn't have to be NUL-te
|
|||||||
|
|
||||||
Left as-is: the framework always renders before forwarding input, so the fragile invariant doesn't fire in practice. Worth a refactor only if the call order ever changes.
|
Left as-is: the framework always renders before forwarding input, so the fragile invariant doesn't fire in practice. Worth a refactor only if the call order ever changes.
|
||||||
|
|
||||||
### ✅ `renderDiscoverDetail` guarded against narrow displays
|
### ✅ Scan detail guarded against narrow displays
|
||||||
|
|
||||||
[`NearbyScreen.h:325-348`](examples/companion_radio/ui-new/NearbyScreen.h#L325-L348)
|
[`NearbyScreen.h:448-451`](examples/companion_radio/ui-new/NearbyScreen.h#L448-L451)
|
||||||
|
|
||||||
Pub-key line is now skipped entirely when `max_chars < 4` instead of feeding a negative length to `strncpy`.
|
Pub-key line is skipped entirely when `max_chars < 4` instead of feeding a negative length to `strncpy`. (Lived in `renderDiscoverDetail` before the one-list refactor; now in `renderScanDetail`.)
|
||||||
|
|
||||||
### 📋 `expandMsg` GPS validity test treats (0, 0) as invalid
|
### 📋 `expandMsg` GPS validity test treats (0, 0) as invalid
|
||||||
|
|
||||||
@@ -786,9 +786,9 @@ Point (0, 0) is a legitimate location (Gulf of Guinea). Corner case but a logic
|
|||||||
|
|
||||||
### ✅ SNR division by 4 — now uses `%.1f`
|
### ✅ SNR division by 4 — now uses `%.1f`
|
||||||
|
|
||||||
[`NearbyScreen.h:355-357, 487`](examples/companion_radio/ui-new/NearbyScreen.h#L355-L487)
|
[`NearbyScreen.h:467-470`](examples/companion_radio/ui-new/NearbyScreen.h#L467-L470), [`330-332`](examples/companion_radio/ui-new/NearbyScreen.h#L330-L332)
|
||||||
|
|
||||||
Detail view (`SNR: %.1f dB`, `Rem: %.1f dB`) and discover list cards (`SNR:%.1f`) both keep the 0.25 dB resolution. Stays consistent with the ping popup, which already used `%.1f`.
|
Scan detail view (`SNR: %.1f dB`, `Rem: %.1f dB`) and the ping popup keep the 0.25 dB resolution. (After the one-list refactor the scan list cards show **RSSI** in the right column, not SNR.)
|
||||||
|
|
||||||
### 📋 Trail `_count` cast to `uint16_t`
|
### 📋 Trail `_count` cast to `uint16_t`
|
||||||
|
|
||||||
@@ -824,7 +824,7 @@ Fix status after this pass:
|
|||||||
- ✅ M4 — `renderDiscoverDetail` skips pub-key line on very narrow displays
|
- ✅ M4 — `renderDiscoverDetail` skips pub-key line on very narrow displays
|
||||||
- ✅ L1 — SNR shown with 0.25 dB precision everywhere
|
- ✅ L1 — SNR shown with 0.25 dB precision everywhere
|
||||||
- ✅ L4 — fallback `"?"` sender no longer memsets through `strncpy`
|
- ✅ L4 — fallback `"?"` sender no longer memsets through `strncpy`
|
||||||
- ✅ Trail map grid silent loss — when a very elongated trail makes `lat_n` or `lon_n` exceed 40, the renderer now bumps the step up instead of dropping the grid entirely. Comment fixed to match the `/ 3.0f` divisor ("~3 intervals", not 4). [`TrailScreen.h:505-565`](examples/companion_radio/ui-new/TrailScreen.h#L505-L565)
|
- ✅ Trail map grid silent loss — superseded by the Trail refactor: `renderGrid` now picks a round labelled step (`1m…100km` / `10ft…100mi`) nearest ~1/3 of the shorter side and enforces a `MIN_GRID_PX` floor, so the grid can never silently vanish on an elongated trail. [`TrailScreen.h:635`](examples/companion_radio/ui-new/TrailScreen.h#L635)
|
||||||
- ❌ M1 — re-checked, not a bug (`default_scope_name[31]`)
|
- ❌ M1 — re-checked, not a bug (`default_scope_name[31]`)
|
||||||
- 📋 H1 + H2 — still open; need coordinated fix in upstream `BaseChatMesh` (`findChannelIdx` should iterate `num_channels`, not `MAX_GROUP_CHANNELS`; `saveChannels` should stop at the first uninitialised slot) or a local override
|
- 📋 H1 + H2 — still open; need coordinated fix in upstream `BaseChatMesh` (`findChannelIdx` should iterate `num_channels`, not `MAX_GROUP_CHANNELS`; `saveChannels` should stop at the first uninitialised slot) or a local override
|
||||||
- 📋 H3 — left as known limitation pending UX call
|
- 📋 H3 — left as known limitation pending UX call
|
||||||
|
|||||||
@@ -1485,7 +1485,6 @@ void MyMesh::handleCmdFrame(size_t len) {
|
|||||||
} else {
|
} else {
|
||||||
result = sendMessage(*recipient, msg_timestamp, attempt, text, expected_ack, est_timeout);
|
result = sendMessage(*recipient, msg_timestamp, attempt, text, expected_ack, est_timeout);
|
||||||
}
|
}
|
||||||
// TODO: add expected ACK to table
|
|
||||||
if (result == MSG_SEND_FAILED) {
|
if (result == MSG_SEND_FAILED) {
|
||||||
writeErrFrame(ERR_CODE_TABLE_FULL);
|
writeErrFrame(ERR_CODE_TABLE_FULL);
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -42,6 +42,16 @@ struct PopupMenu {
|
|||||||
if (max_by_height < 1) max_by_height = 1;
|
if (max_by_height < 1) max_by_height = 1;
|
||||||
_cap = max_by_height;
|
_cap = max_by_height;
|
||||||
int vis = (_count < _cap) ? _count : _cap;
|
int vis = (_count < _cap) ? _count : _cap;
|
||||||
|
// render() is the single source of truth for _scroll: using the cap just
|
||||||
|
// computed from the live display height, keep the selection inside the
|
||||||
|
// visible window. handleInput() therefore never has to touch _scroll (and
|
||||||
|
// can't act on a stale _cap from before the first render).
|
||||||
|
if (_sel < _scroll) _scroll = _sel;
|
||||||
|
else if (_sel >= _scroll + vis) _scroll = _sel - vis + 1;
|
||||||
|
int max_scroll = _count - vis;
|
||||||
|
if (max_scroll < 0) max_scroll = 0;
|
||||||
|
if (_scroll > max_scroll) _scroll = max_scroll;
|
||||||
|
if (_scroll < 0) _scroll = 0;
|
||||||
int bh = 12 + vis * PM_ITEM_H;
|
int bh = 12 + vis * PM_ITEM_H;
|
||||||
|
|
||||||
display.setColor(DisplayDriver::DARK);
|
display.setColor(DisplayDriver::DARK);
|
||||||
@@ -76,18 +86,9 @@ struct PopupMenu {
|
|||||||
|
|
||||||
Result handleInput(char c) {
|
Result handleInput(char c) {
|
||||||
if (_count == 0) { active = false; return CANCELLED; }
|
if (_count == 0) { active = false; return CANCELLED; }
|
||||||
if (c == KEY_UP) {
|
// Selection only moves here; render() keeps it scrolled into view.
|
||||||
_sel = (_sel > 0) ? _sel - 1 : _count - 1;
|
if (c == KEY_UP) { _sel = (_sel > 0) ? _sel - 1 : _count - 1; return NONE; }
|
||||||
if (_sel < _scroll) _scroll = _sel;
|
if (c == KEY_DOWN) { _sel = (_sel < _count - 1) ? _sel + 1 : 0; return NONE; }
|
||||||
else if (_sel == _count - 1 && _count > _cap) _scroll = _count - _cap;
|
|
||||||
return NONE;
|
|
||||||
}
|
|
||||||
if (c == KEY_DOWN) {
|
|
||||||
_sel = (_sel < _count - 1) ? _sel + 1 : 0;
|
|
||||||
if (_sel >= _scroll + _cap) _scroll = _sel - _cap + 1;
|
|
||||||
else if (_sel == 0) _scroll = 0;
|
|
||||||
return NONE;
|
|
||||||
}
|
|
||||||
if (c == KEY_ENTER) { active = false; return SELECTED; }
|
if (c == KEY_ENTER) { active = false; return SELECTED; }
|
||||||
if (c == KEY_CANCEL || c == KEY_CONTEXT_MENU) { active = false; return CANCELLED; }
|
if (c == KEY_CANCEL || c == KEY_CONTEXT_MENU) { active = false; return CANCELLED; }
|
||||||
return NONE;
|
return NONE;
|
||||||
|
|||||||
Reference in New Issue
Block a user