fix: bundle review findings from PRs #9-#13 into one release - #14
Merged
Conversation
…uty-cycle accounting Ui::tick() ran the pending warning-repeat retransmit before the Charging early-return, so a repeat armed just before a charger was plugged in could call Radio::send() after main.cpp had already put the radio to sleep for charging mode. Move the repeat check below the early return and clear pendingRepeatArmed on the charging rising edge so a stale repeat can't fire on resume either. Also close two duty-cycle budget gaps in Radio: the token bucket started full on every boot, letting a brownout-reboot mid-ride exceed the EU868 1% hourly limit by simply power-cycling (now starts empty); and a failed transmit() still deducted airtime from the budget even though nothing was actually sent over the air (now deducts only on success). Finally, correct the `charge` console command's calibration hint, which told users to flip INA219_CURRENT_CHARGE_SIGN to -1 when it is already -1 -- the fix for a negative reading is +1.
selected_port() split the combobox label on " (" to recover the actual
serial port, but a "likely the Vaura device" label for a port with an empty
description (list_ports.comports() can return one, notably for the ESP32-S3
native USB-CDC/JTAG port on some platforms) has no " (" to split on --
selected_port() then returned the whole label, arrow suffix included, and
every action (flash/read/apply) targeted a nonexistent port. Pull the
label<->device mapping out of refresh_ports() into a plain build_port_labels()
helper so selected_port() can look the real port up directly instead of
re-parsing the display label, with the old split as a fallback for a stale
selection.
Also replace the dead "YOUR-GITHUB-USER" placeholder guard in
fetch_latest_release() -- GITHUB_REPO is already a real repo, so the guard
could never fire -- with an explicit HTTP 404 handler that surfaces a clear
"no releases yet, use Local file" message instead of a raw HTTPError. Update
the matching README fork-setup note.
Add flasher/test_flasher.py (pytest, no hardware/GUI) covering the new
port-label round trip, the `status` key=value parser, and the merged-binary
NVS split, wired into CI.
…verflow - Power::refreshIfDue() now sanity-clamps the raw INA219 bus-voltage read to [0, 5]V before caching it. A glitched/out-of-range I2C read previously wrapped silently into a nonsensical uint16_t millivolt value that then propagated into the battery display, the low-battery latch, and (via the heartbeat) every peer's view of this rider's battery. - Protocol::decode() now rejects a Warning packet whose type byte is outside the WarningType enum instead of letting it through to degrade only much later, at display time, to warningLabel()'s "?" fallback -- symmetric with how an unknown MsgType is already rejected. Covered by a new test_reject_unknown_warning_type native test. - The `beep` console command now range-checks its frequency argument as a signed long before casting to unsigned, rejecting negative or absurd values (e.g. "beep -5" or "beep 999999") that previously slipped past a plain `== 0` check once wrapped into an unsigned int. - Roster::findOrCreate() now logs once (not once per dropped heartbeat) when MAX_PEERS is exhausted, so a rider beyond the cap doesn't silently vanish with no trace on a safety-relevant device. No wire-format changes.
…endencies
- CLAUDE.md's repository layout table was missing coop.{h,cpp} (the
cooperative drop-off gossip mesh), charging_decision.{h,cpp}, and
node_id.h -- all load-bearing, natively-tested modules. Added them plus
the corresponding test directories.
- README referenced "protocol version 2" and "v2 is radio-incompatible with
v1" in two places; the wire format has since moved to v3 (cooperative
drop-off confirmation). Reworded both version-agnostically so this doesn't
drift again on the next protocol bump, and fixed the hardware table's
"Wio-SX1262 (LoRa)" label, which contradicted the GFSK design described a
few paragraphs later.
- Documented the RELEASE_PAT repo secret auto-tag.yml needs to push release
tags -- previously only GITHUB_REPO was mentioned in the fork setup notes,
so a fresh fork's release pipeline would silently never tag or release
anything.
- Added a License section to the README (the MIT LICENSE file itself already
existed) plus a short third-party notice for the bundled libraries.
- Pinned Adafruit_INA219 to the PlatformIO registry (adafruit/Adafruit
INA219@^1.2.3) instead of an unpinned git-HEAD URL -- confirmed this is
the exact commit already in use (same publish timestamp) via a clean
rebuild, so behavior is unchanged. Also floor-pinned certifi
(>=2024.7.4, fixing a known-expired intermediate CA in older releases)
and pyinstaller (>=6.0,<7) in release.yml.
- Translated flasher/requirements.txt's one remaining German comment.
No wire-format or runtime logic changes.
CLAUDE.md flags tick()'s timeout handling as the one place in ui.cpp's
four-way State sync that a plain if-chain, not a switch, so the compiler
can't catch a State that should time out but has no entry. Replaced the
eleven near-identical if-blocks with a small TimeoutEntry table ({State,
timeoutMs, optional action}) and a single loop, so adding a timed state is
now one row instead of a copy-pasted block that's easy to get subtly wrong.
BootChannelSelect keeps its distinct action (confirmBootChannel instead of
enterIdle) via the table's optional action function pointer.
DismissPrompt's extra per-tick candidate-refresh logic (not a timeout) stays
exactly where it was, unfolded into the table on purpose. Idle/RangeTest/
Charging remain absent, same as before.
Behavior is unchanged -- this is a pure refactor, verified by inspection
(every {state, timeout, action} triple carried over unmodified) plus a
clean build and the full native test suite.
# Conflicts: # README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Combines the five review-fix branches (#9, #10, #11, #12, #13) into one merge so they land in
mainas a single push and produce exactly one release instead of five — merged here in the same sensible order they were reviewed/built in, each--no-ffso the original commits/history are preserved:fix/firmware-correctness— charging-mode repeat-send bug, duty-cycle accountingfix/flasher-port-and-repo—selected_port()bug, dead repo guard, flasher testsfix/robustness— battery-voltage clamp, warning-type validation,beep/roster hardeningdocs/license-and-drift— CLAUDE.md/README drift, RELEASE_PAT docs, dependency pinningrefactor/ui-tick-timeouts—tick()timeout table refactorOnly one merge conflict occurred (README.md's fork-setup section, touched by both #10 and #12) — resolved by keeping both paragraphs (the corrected "no releases found" wording from #10, plus the
RELEASE_PATdocumentation from #12). Everything else merged cleanly, including the two places #9 and #13 both touchui.cpp'stick()(no textual overlap between the two changes).No wire-format changes anywhere in this set; no
Protocol::kVersionbump needed.Test plan (run against the fully merged tree)
pio run— firmware builds cleanpio test -e native— 62/62 passpython flasher/flasher.py --check— self-test passespytest flasher/test_flasher.py— 14/14 pass