Router::handleReceived stores its allocCopy of the encrypted packet in the
class member p_encrypted. callModules() invokes module replies that re-enter
the router via MeshService::sendToMesh -> Router::sendLocal, which on a
broadcast reply recursively calls handleReceived. The inner call overwrites
the outer's p_encrypted without releasing it; on the way out it nulls the
member, the outer release(p_encrypted) now releases nullptr, and the original
allocation is permanently leaked. ~381 B per recursion.
Promote p_encrypted to a function-local so each invocation owns its own copy
for its full lifetime. The MQTT-publish null check at the call site (added by
PR #9136 as a workaround for this bug) stays in place because allocCopy can
still legitimately return nullptr on packetPool exhaustion.
Copilot's review of PR #8999 (the original introduction) flagged this exact
pattern at merge time:
"Storing p_encrypted as a class member can cause issues with recursive or
concurrent calls to handleReceived() since each call would overwrite the
previous packet pointer."
The historical reason for the member (S&F needing to retain the encrypted
copy across calls) was satisfied differently by PR #9799 (ServerAPI converted
to std::unique_ptr + cleanup on connection close), so the member is no longer
load-bearing.
Reproduces issues #9632 / #10101 / #8729 (heap leak when MeshMonitor
connected; TCP drops on Station G2 / LILYGO ServerAPI dump abort).
Hardware A/B on Station G2 under sustained TCP-API retry storm (open :4403,
request config, disconnect mid-stream, repeat at ~0.6/s) - 9 min run:
| Build | heapFree drift | rebootCount delta |
| this patch | -1.5 KB (noise)| 0 |
| stock 2.7.13 | -73 KB (8.1KB/min) | +1 (OOM crash) |
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
* True Colors on TFT (Heltec Mesh Node T114, Heltec Vision Master T190, CardPuter Adv, T-Deck, T-Lora Pager)
* Theme support - New and some Classic Themes!
* Colored Compass
---------
Co-authored-by: Jason P <applewiz@mac.com>
Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Fixes issues with #includes inherited from `configuration.h` when building for pioarduino.
Aligns t5s3_epaper with other variants like t_deck_pro.
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Marker boxes, the own-node bullseye, and the labeled-marker cross were
all hardcoded in pixels (11px box, r=8 circle, 12px cross). On the
T5S3 with a 12pt fontSmall (~17px line height) the hop-count digit
overflowed its box entirely. Sizes now derive from fontSmall.lineHeight()
so the applet renders correctly on both small (6pt) and large (12pt+)
display variants.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix INA226 detection for non-TI compatible chip (Silergy)
* Removed extra I2C transaction + 20ms delay on every scan of address 0x40 (including real SHT2x sensors).
Changes suggested by Copilot
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Apply formatting (trunk fmt)
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* PMU interrupt pin defined in t-watch s3
* Implement button control on T-Watch S3
Added interrupt handling for the Power/Corona button on T-Watch S3, I use it to control screen state.
* Reducing labels
* Reducing labels
* Updated the comment
* ISR is now IRAM-safe
Updated interrupt management not to cause random crashes.
* Trunk
* Simplify and use INPUT_BROKER_CANCEL
---------
Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>
* PositionModule::sendLostAndFoundText: use stack buffer, eliminate heap alloc
The lost-and-found message was built with an unnecessary heap allocation:
char *message = new char[60];
sprintf(message, "..."...);
...
delete[] message;
Two problems:
1. **Buffer too small.** The format string expands with two %f (IEEE 754
doubles), which `sprintf` prints with full precision — easily 15+
digits each plus separators — so the actual rendered string can run
40-50 characters before even considering the emoji (4 UTF-8 bytes)
and the embedded BEL. A pathological lat/lon can overflow 60 bytes
and corrupt heap metadata. Unbounded `sprintf` with no size check.
2. **Heap churn in a GPS callback.** This function is called from the
position-update path which is already heap-sensitive. An infrequent
60-byte transient alloc isn't catastrophic, but stack is trivially
available here and removes the failure mode entirely.
Fix: replace with a 128-byte stack buffer and `snprintf` bounded by
`sizeof(message)`. Drop the matching `delete[]` since there's nothing
to delete.
Behavior is identical on the happy path; the overflow case now
truncates safely instead of scribbling over heap.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* PositionModule.cpp: add trailing newline for clang-format
* Address Copilot review: cleaner snprintf size handling
Review feedback from @Copilot on PR #10251: the ternary-plus-static-cast
form mixed signed/unsigned types (int written vs. pb_size_t payload.size
vs. size_t sizeof(message)) and was harder to read than necessary.
Cleaner form:
const size_t msg_len = std::min(static_cast<size_t>(written), sizeof(message) - 1);
p->decoded.payload.size = msg_len;
Same behaviour (clamp to buffer-minus-NUL) with one explicit cast and
a size_t variable that names the meaning. Handles the encoding-error
path (written < 0) separately so no bad values leak into payload.size.
* Trunk
---------
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Use hash table for O(1) lookup of recently seen packets
* Eliminate a packet lookup during deduplication
* Infinite loop checks for find and remove
* Consolidate conditional compilation
* Exclude hash table from minimal build
* Additional comment on hash table capacity
* Unit tests for packet history changes
* Update incorrect comment about size clamp
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Const
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Mirror the EXT_PWR_DETECT pattern: replace runtime static variables
(ext_chrg_detect_mode, ext_chrg_detect_value) with compile-time macros.
Auto-infer EXT_CHRG_DETECT_VALUE from EXT_CHRG_DETECT_MODE when the mode
is INPUT_PULLUP (→ LOW) or INPUT_PULLDOWN (→ HIGH); default to HIGH.
This fixes inverted polarity on variants that define EXT_CHRG_DETECT_MODE
INPUT_PULLUP without an explicit EXT_CHRG_DETECT_VALUE (e.g. russell):
previously the runtime default of HIGH caused isCharging() to return the
opposite of the correct value. With auto-inference the correct LOW active
level is now derived at compile time.
Remove the now-redundant EXT_CHRG_DETECT_VALUE HIGH from ELECROW-ThinkNode-M4
variant.h since HIGH is the inferred default.
Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <noreply@example.com>
Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>
* Delete unused clearNVS() (last used in commit 761804b1).
* virtual methods: add 'override' to ensure we get the signature right.
This is a safety net for pioarduino/NimBLE work where there's multiple
similar variants of the same method (eg. onConnect) and it's easy to get
the wrong one and accidentally miss a callback.
* PhoneAPI: add missing tak_tag case + skip reserved gap in module-config iteration
The STATE_SEND_MODULECONFIG state machine iterates config_state through
ModuleConfigType enum values (1..MAX+1 = 16) and switches on
meshtastic_ModuleConfig_*_tag values. Two of the resulting tag values
land in the default / LOG_ERROR path:
1. `tak_tag` (16) — the meshtastic_ModuleConfig_TAKConfig struct exists
in the protobuf and has a `.tak` member in payload_variant, but no
PhoneAPI case ever sends it to the phone. As a result, Android
clients can't read the persisted TAK (Team Awareness Kit) module
config at all. Added case that sends moduleConfig.tak, matching the
pattern used for all other module-config tags (paxcounter,
traffic_management, etc.). NodeDB already persists the struct via
the moduleConfig protobuf save; this just wires the read path to
the phone.
2. Tag 14 — reserved gap in the oneof numbering. No payload_variant
member exists at tag 14. Without this patch, every phone reconnect
walks through config_state=14 and hits
`LOG_ERROR("Unknown module config type %d", config_state)`. On an
active deployment that's ~1,400 LOG_ERROR lines per day per node —
burying real errors. Added explicit `case 14: break;` so the gap
is silently skipped.
Also: lowered the `default:` log level from LOG_ERROR to LOG_DEBUG. A
truly-new unknown type number would indicate firmware lagging the
protobuf — annoying but not an error event worth LOG_ERROR, especially
since this path runs on every phone handshake. If a new ModuleConfig
tag appears, devs will notice via the phone UI missing it, not via log.
Observed on a Station G2 fleet: 1403 "Unknown module config type 16"
and 1427 "Unknown module config type 14" LOG_ERROR lines in 24 hours
from routine phone reconnects.
* Get rid of the placeholder
---------
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
The "Invalid protobufs (bad psk?)" and "Invalid portnum (bad psk?)"
messages fire every time a neighbor transmits on a channel whose
8-bit hash matches one of ours but the PSK differs. In RF environments
with multiple mesh groups nearby this is routine — a single device
can see dozens of these per minute from SAR/MeshCA/private networks
sharing a hash collision.
LOG_ERROR for a benign "not our PSK" event:
- spams the log when you have any neighboring mesh group
- makes a genuine PSK misconfiguration on YOUR own channel
indistinguishable from the constant cross-channel noise
- hides actual errors in the stream
LOG_DEBUG matches how similar decryption-failure paths are handled
elsewhere (eg. the PKC "decrypt attempted but failed" uses LOG_WARN).
Dropping the trailing "!" as well — these are expected events, not
exceptional ones.
The constructor sets `RadioLibInterface::instance = this` immediately,
before `init()` runs. `initLoRa()` in RadioInterface.cpp creates each
radio variant with `new SX1262Interface(...)` or similar, then calls
`init()`, and if init fails the `unique_ptr<RadioInterface>` is reset
to nullptr — destroying the object — while the static `instance`
pointer continues to point at the freed memory.
Main loop then checks `RadioLibInterface::instance != nullptr` and
calls `pollMissedIrqs()` or `resetAGC()` on the dangling pointer →
Guru Meditation (IllegalInstruction / LoadProhibited).
Reported in #9880 on an ESP32-S3 dev board without radio hardware
attached, where init always fails and the leftover pointer crashes
the device on the next `loop()` iteration.
Fix: add a virtual destructor to `RadioLibInterface` that clears the
static pointer iff it still references this object. A later successful
init() may have replaced `instance` with a different interface — the
`instance == this` guard preserves that case.
Fixes#9880
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
