mirror of
https://github.com/meshtastic/firmware.git
synced 2026-09-16 08:30:04 -04:00
ee48094ea8309858cd579100b8aaf14560bcc4b2
7165
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Clive BlackledgeandClaude Opus 5
|
0271be9369 |
fix(SafeFile): remove a stale .tmp before opening it for write (#11428)
* fix(SafeFile): remove a stale .tmp before opening it for write SafeFile writes to <filename>.tmp, verifies it by readback, then renames it over the real file. openFile() never removed a pre-existing .tmp - an unfinished FIXME - and FILE_O_WRITE appends rather than truncates on Adafruit_LittleFS (nRF52) and STM32 LittleFS. So a .tmp left behind by a reset in the window between close() and renameFile() is appended to on the next save. The readback hash covers only the bytes just written, so it mismatches, close() returns false, and the tmp is left behind again - the failure latches and every subsequent save of that file fails. Today saveProto() discards close()'s result, so this is silent and permanent. Guard the remove with exists(): a bare remove() of a missing file logs on Portduino. The same guarded pattern is already used for this exact append trap in xmodem.cpp. Note the FIXME's commented-out body named the wrong path - it removed 'filename', the real file, not 'filenameTmp' - so it would have destroyed the good copy had it ever been enabled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(SafeFile): cover the stale-.tmp path, and trim the fix comment Adds test_safefile, the first coverage of SafeFile's write-tmp / verify-by-readback / rename-over path that every saveProto() caller goes through. Five cases pin the contract the fix restores: whatever the backend does on open, a completed save leaves the real file holding exactly the bytes written and nothing else, on both the fullAtomic and the !fullAtomic construction, with no .tmp left behind. These tests cannot go red on the native host, and that gap cannot be closed here. FILE_O_WRITE is an append-then-seek-to-end open only on Adafruit_LittleFS (nRF52) and on the in-repo STM32 port (STM32_LittleFS_File.cpp: LFS_O_RDWR | LFS_O_CREAT followed by lfs_file_seek to LFS_SEEK_END). On Portduino FILE_O_WRITE is the string "w" (FSCommon.h:13), which reaches fopen() and truncates. Reverting the source fix and re-running leaves all five green, verified rather than assumed. test_write_open_truncates _on_this_host asserts that premise out loud, so if the host ever gains the append behaviour the suite starts discriminating instead of quietly agreeing. Why the original FIXME stayed commented out, since that is the real history here. It read "if (fullAtomic) FSCom.remove(filename)" and named the real file, not the tmp. Running it would delete the last good copy before the replacement had been written and verified, which is precisely the guarantee fullAtomic exists to provide. Disabling it was correct. The fix under test removes filenameTmp instead, which is the file that actually carries the stale bytes, and is safe to drop at any point because nothing has been promised about it yet. Scoping the remove to fullAtomic would be wrong for the same reason. Both paths open the same filenameTmp with the same FILE_O_WRITE; fullAtomic only decides whether the real file is nuked up front to free space. The !fullAtomic path is the space-constrained one, so it is if anything the more likely to be interrupted mid-write and inherit a stale tmp. Test 2 pins that. On the cost of the added exists(). Every saveProto() already ends in SafeFile::close(), which calls testReadback(): it reopens the tmp and reads the whole proto back one byte at a time through f2.read() to XOR a verification hash, then renames. So the per-save cost is already an open, a full write, a close, a full byte-wise reread, and a rename. One exists() is a single path lookup with no erase, no program and no data read, and on the common path there is no remove() at all. Next to the readback loop it is noise. Happy to put a number on it if wanted. Scoping it to fullAtomic would also not do what it looks like it does. SafeFile's constructor defaults fullAtomic to false (SafeFile.h:28), and of the saveProto call sites only saveDeviceStateToDisk passes true. Config, moduleconfig, channels, nodedatabase and backup all take the default, so scoping would leave the stale tmp live on almost every save path, including the space-constrained one most likely to be interrupted mid-write. Also trims the fix's comment to two lines per AGENTS.md, and drops the stale-tmp removal log from LOG_WARN to LOG_DEBUG: an interrupted write is recoverable and self-healing, so it does not warrant a warning on every boot after one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(SafeFile): fail the write when a stale tmp cannot be removed openFile() ignored the result of FSCom.remove(). If the removal failed on an append-on-write backend, the open that follows appended to the stale bytes, and the readback hash is an 8 bit XOR over the whole tmp, so polluted content has a real chance of verifying and being renamed over the good file. It now logs and returns an invalid File. SafeFile::write() already no-ops on !f and close() already returns false, so the caller sees the save fail rather than silently getting a corrupt one. This is the only checked FSCom.remove() in the tree; the other call sites are all best-effort cleanups where failure does not compromise anything. Also gates test_write_open_truncates_on_this_host to ARCH_PORTDUINO. It asserts that this host truncates on FILE_O_WRITE, which is false by design on the Adafruit_LittleFS and STM32 backends the fix exists for, so running the suite there would fail on a premise that is only meant to describe the test host. Both reported by CodeRabbit on #11428. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Ben MeadorsandQuency-D
|
bfd1e1a231 |
Add Heltec RC32, RC52 and RCC6 boards, and LC760CA GNSS support (#11572)
* refactor(graphics): select Arduino_GFX panels with a capability flag TFTDisplay tested `defined(HACKADAY_COMMUNICATOR)` in a dozen places to mean "this panel is driven by Arduino_GFX rather than LovyanGFX". Every new Arduino_GFX board had to be appended to all of them. Move the decision into the variant as USE_ARDUINO_GFX so the display code stops naming individual boards. No behaviour change: the Hackaday Communicator is still the only board that sets it. * feat(boards): add Heltec RC32, RC52 and RCC6 Three boards around the same 128x220 NV3001B panel: RC32 (ESP32-S3), RCC6 (ESP32-C6) and RC52 (nRF52840). They differ only in how the panel bus is wired, so they share one branch in TFTDisplay behind TFT_NV3001B. RC32 and RC52 also carry a rotary encoder on a TCA6408 I2C expander. That lands as its own input source rather than as board conditionals inside i2cButton, which is the M5Stack UnitC6L button driver and stays untouched. On RC52 and RCC6 the panel is an add-on module, so probe it before reporting a screen. The probe reuses the bit-banged SPI helper that already backs the T114 ST7789 check. Arduino_GFX is pinned to the upstream commit that added the NV3001B driver; it has not shipped in a tagged release yet. Co-Authored-By: Quency-D <55523105+Quency-D@users.noreply.github.com> * feat(gps): detect and configure the LC760CA GNSS module The LC760CA is another Unicore part, so it joins the $PDTINFO probe family and reuses the CM121 message-rate setup. It answers with CC1161W. GNSS_MODEL_LC760CA goes immediately before GNSS_MODEL_GENERIC_NMEA: the sentinel has to stay last because isValidGnssModel() uses it as the exclusive upper bound on values the probe cache may hold. Placing the new model after it would leave LC760CA permanently uncacheable. Co-Authored-By: Quency-D <55523105+Quency-D@users.noreply.github.com> * fix(graphics): re-init the NV3001B after the panel rail comes back DISPLAYOFF de-asserts VTFT_CTRL, which cuts power to the panel, so the controller loses MADCTL, COLMOD and gamma. displayOn() only sends sleep-out and cannot restore them, leaving the panel dark or in the wrong format after wake. Re-run begin() once the rail has settled, and repaint in full since the re-init leaves display RAM undefined. Also stop the TCA6408 rotary polling from two threads at once. Registering as an InputPollable meant InputBroker's pollSoon task could call pollOnce() while runOnce() was mid-transfer on the main thread, with nothing serialising Wire or the decoder state. Drop InputPollable and have the interrupt wake the thread instead, the way ButtonThread does, so the bus and the decode stay on one thread. * fix(graphics): skip the NV3001B wake when re-init fails begin() reports whether the bus came up. Ignoring it meant a failed re-init still lit the backlight and drove a full-screen repaint at a panel that was never initialised. * chore(boards): ship the Heltec RC boards at release level release is the normal level for a variant; the matrix generator still builds each of these in this PR because they add a new platformio.ini. --------- Co-authored-by: Quency-D <55523105+Quency-D@users.noreply.github.com> |
||
|
Tadayoshi MIURA
|
ac330e6a6b |
fix(radio): MeshBeacon heap leak and runtime packet payload size check (#11573)
* Fix for MeshBeacon packet leakage * fix: add runtime payload size check against radiobuffer * review fix for PR#11573: clear target radio settings before MeshBeacon packet release * add unit test for radio buffer capacity check, removing related assert for the test * review fix for PR#11573: add explicit verifaction against rejected packets |
||
|
Ben Meadors
|
73f7b35bea |
Report the right hardware model on four boards (#11570)
Four variants declare a custom_meshtastic_hw_model that the build never reaches, so the device announces something else in NodeInfo and the apps cannot match it for OTA. Mini ePaper S3 (125) and Heltec V4 R8 (132) had no arm in the esp32 HW_VENDOR chain at all, so both fell through to #else and reported PRIVATE_HW. Heltec Mesh Node T096 (127) had none in the nrf52 chain and reported NRF52_UNKNOWN. WisMesh Tap V2 defines both RAK3312 and RAK_WISMESH_TAP_V2, and the generic RAK3312 arm sat first, so the board reported RAK3312 (106) instead of WISMESH_TAP_V2 (116). Order the specific arm ahead of the generic one, the same way the nrf52 chain already keeps custom RAK4630 boards ahead of the generic RAK4630. Verified by preprocessing each platform's HW_VENDOR chain with the env's full define set - build flags resolved through extends, the board JSON's build.extra_flags, and the bare #defines in the variant's own variant.h. All four now match their manifest, and rak3312, heltec-v4, heltec-v4-tft and the ThinkNode M9 arm added in #11567 are unchanged. |
||
|
Ben Meadors
|
f6f116a39d |
Fill in device registry metadata for recently added hardware (#11567)
Audit of the custom_meshtastic_* manifest on the variants backing the newest boards, against the protobuf HardwareModel enum, the compiled HW_VENDOR, the board flash size and the artwork actually published by the web flasher. No support flag changes here - actively_supported is left exactly as each variant already had it. ThinkNode M9 had no HW_VENDOR arm, so every M9 has been reporting PRIVATE_HW while its manifest advertised 131; add the mapping and rename the slug to the enum name (THINKNODE_M9) it is meant to mirror. Seeed SenseCAP Mesh-Tracker X1 moves from the PR matrix to release, and its images entry now points at seeed_mesh_tracker_x1.svg, which is what the flasher actually ships - the hyphenated name resolved to nothing. T-Beam BPF, T-Beam 1W and Heltec Wireless Tracker V2 declared the architecture as "esp32s3"; the value is copied verbatim into the manifest, and the flash flow matches on the normalized "esp32-s3". T-Beam BPF and M5Stack Unit C6L both build default_16MB.csv on 16 MB flash but declared no partition scheme, which leaves the flasher on the 4 MB fallback offsets for a legacy clean install. Meshnology W10 and W12 gain the artwork and vendor tag that already exist for them. |
||
|
Tom
|
bc035bb812 |
feat(lora): state a pinned userPrefs preset as the unset region's intent (#11507)
* feat(lora): state a pinned userPrefs preset as the unset region's intent A vendor build can pin USERPREFS_LORACONFIG_MODEM_PRESET while leaving the region unset, so a fresh flash comes up as region UNSET plus a deliberate preset. Stock installs come up as region UNSET plus the LONG_FAST placeholder, and nothing in FromRadio told the two apart - so clients treat every unset-region node as factory-fresh and replace its preset with the region default as soon as the user picks a region. A mesh pinned to SHORT_TURBO loses every new node to LONG_FAST or LONG_TURBO, silently. getRegionPresetMap() now emits an UNSET entry when, and only when, the build pins a preset, stating that preset as both the group's sole entry and its default. Stock builds are unchanged on the wire: no UNSET entry, which clients already read as unconstrained. This is intent, not enforcement. supportsPreset() still accepts any known preset while the region is unset (#11496) and the radio is held silent either way, so the device continues to honour whatever the user or an admin sets. Costs one group slot and one region slot on pinned builds only (6->7 of 8, 34->35 of 38); exhaustion is logged and degrades to the existing unconstrained behaviour. * Trim comments to the project's one-to-two-line limit |
||
|
|
0b906b4d15 |
T-Watch Ultra support (#8171)
* feat: T-Watch Ultra support * fix init touch controller * add framebuffer * update to device-ui * trunk fmt * update amoled driver reference * PMU cosmetics * power off lora * fix NodeDB defaults * trySetRTC when fixedPosition * haptic touch (only BaseUI) * init lora RF switch * update LovyanGFX 1.2.19 * earlyInitVariant() adaptations acc. #9438 * update device-ui / touch handling * Set NFC_CS disabled on boot * Get t-watch-ultra working better on BaseUI * Fix compilation * Fix flash reads on t-watch-ultra * Get baseui drawing to the screen correctly again on t-watch and add touch IRQ handling * Add PMU IRQ handling * Add IMU support * Change define to avoid collision * BaseUI changes to support t-watch-s3 rounded screen (#10786) * BaseUI changes to support t-watch-s3 rounded screen * Extend margin work to CannedMessages * Finish merge * Get audio working on watch-ultra * trunk fmt * added custom_meshtastic boilerplate * T-Echo-Plus: disable BHI260AP while assumingly not implemented * Drop the duplicate origBold declaration from the merge * Inset incoming message bubbles on rounded screens * Fix RTTTL tempo, WiFi screen margins, PMU guard and a duplicate define * fix compile errror (the 2nd time) * fix SDcard * fix/workaround CO5300 pixel flush to SPI * trunk fmt --------- Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz> Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> |
||
|
Clive BlackledgeandClaude Opus 5
|
389559bddb |
fix(NodeDB): re-derive my_node_num when ensurePkiKeys() mints the identity keypair (#11426)
* fix(pki): re-derive NodeNum when setting a region mints the identity key
A node's mesh address is derived from its identity key:
my_node_num == crc32Buffer(config.security.public_key.bytes, 32)
NodeDB::createNewIdentity() is what establishes that, and NodeDB::
generateCryptoKeyPair() is the only thing that called it.
CryptoEngine::ensurePkiKeys() generates or re-derives the keypair and writes
security.public_key, security.private_key and user.public_key - but never
re-derives my_node_num. Boot-time keygen is suppressed while the LoRa region is
UNSET (generateCryptoKeyPair()'s regionBlocksKeygen guard), so on a fresh device
my_node_num is still the MAC-derived value from pickNewNodeNum(). The user then
sets the region - the stock onboarding flow - ensurePkiKeys() mints a key, and
the invariant is broken.
The node then signs its broadcasts (Router.cpp signs when !pki_encrypted &&
(owner.is_licensed || isBroadcast(p->to))). Every receiver runs
verifyFirstContactNodeInfo, fails crc32Buffer(user.public_key) != p->from, and
drops the NodeInfo. The node's identity beacons are invisible to the mesh.
Nothing reboots to repair it: AdminModule sets requiresReboot = false for LoRa
changes ("All LoRa radio changes apply live via configChanged observer") and
MenuHandler ends at service->reloadConfig(changes).
Four call sites reached ensurePkiKeys():
1. AdminModule set_config LORA, region first set (phone app - the common path)
2. MenuHandler applyLoraRegion (on-device region picker)
3. InkHUD MenuApplet applyLoRaRegion (schedules a reboot, so it
self-healed at next boot)
4. portduino wasm wasm_set_region
The reference implementation was already in the tree: the *licensed* branch of
call site 1, thirteen lines below the broken unlicensed one, calls
nodeDB->generateCryptoKeyPair() (which reaches createNewIdentity()) and widens
the persisted mask with SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE.
Rather than repeat that at four call sites, the key-mint is routed through one
chokepoint that owns both halves of the identity: NodeDB::ensurePkiIdentity()
calls crypto->ensurePkiKeys() and then createNewIdentity(). It lives in NodeDB
because createNewIdentity() operates on the devicestate/node-DB globals, which
CryptoEngine deliberately does not touch - ensurePkiKeys() takes the security
config and user by reference precisely so it stays free of that dependency, and
it is unit-tested against a standalone CryptoEngine.
ensurePkiIdentity() returns true only when my_node_num actually moved
(createNewIdentity() early-returns when the key is unchanged, so a repeat region
change does not disturb the self entry or force a needless flash write). Callers
use that to widen their save mask; my_node_num lives in devicestate and the self
row moves in the node DB, so both segments must be persisted or the fix would
revert at the next boot. SEGMENT_CONFIG, which carries the key itself, is
already unconditional on all four paths.
The InkHUD reboot is left as-is. It is now redundant for this invariant, but it
covers the rest of that menu's behaviour and a redundant reboot is not a bug.
Adds test_handleSetConfig_persistsUnlicensedFirstRegionIdentity, the unlicensed
twin of the existing licensed test, asserting both the segment mask and
my_node_num == crc32(public_key).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* style(NodeDB): trim identity-recovery comments and guard the WASM nodeDB deref
Two review asks, no behaviour change on any built target.
Copilot flagged the unguarded nodeDB deref in the WASM region setter; it is the
only ensurePkiIdentity() call site that did not check the pointer first.
The rest is comment length. AGENTS.md:83 caps code comments at two lines, and the
identity-recovery comments across the four call sites plus the NodeDB.h doc block
ran to four and six lines. The rationale they carried is in the commit messages
and the PR body, which is where AGENTS.md says it belongs.
The PR's own fix in AdminModule.cpp is deliberately untouched.
* fix(NodeDB): keep the identity move authoritative when the self record cannot be created
createNewIdentity() removes the old node entry and assigns myNodeInfo.my_node_num
before it tries to create the row for the new number. If getOrCreateMeshNode()
came back null it returned false, so the first-region callers left
SEGMENT_DEVICESTATE and SEGMENT_NODEDATABASE out of the save mask.
The number had already moved in RAM at that point, and the freshly minted key
goes to flash under SEGMENT_CONFIG regardless. The next boot therefore reloads
the old number alongside the new key, which is exactly the
crc32(public_key) != my_node_num break this path exists to prevent, reached
through the error branch instead of the happy one.
Rolling the number back is not an option either, since the key has already been
replaced by the time this runs. So the move is now reported as the fact it is and
the missing self record is logged separately; getOrCreateMeshNode() will recreate
that row on the next contact. Reachable when the self record is absent and the
table is full of protected nodes.
Reported by CodeRabbit on #11426.
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
||
|
|
90a6dec3f3 |
fix(NodeDB): require a full 32-byte key when demoting to the warm tier (#11431)
* fix(NodeDB): require a full 32-byte key when demoting to the warm tier meshtastic_User.public_key is a wire `bytes` field with max_size 32, so any size in 0..32 decodes off the air, and nothing validates it on ingress: NodeInfoModule hands the decoded User straight to NodeDB::updateUser, whose PKI gates are all `== 32` and so fall through for a partial key, and TypeConversions::CopyUserToNodeInfoLite then stores it with the short size. demoteOldestHotNodesToWarm() admitted that partial key into the warm tier on a `size > 0` gate. WarmNodeEntry has no length field - it distinguishes "has a key" from "no key" purely by all-zero - so N real bytes plus 32-N zeros become indistinguishable from a genuine key. copyPublicKeyAuthoritative() then hands that fabricated key back with size = 32 and reports it AUTHORITATIVE, and re-admission writes size = 32 into the hot store. From then on updateUser's key pin permanently rejects the node's real NodeInfo, and DMs to it are encrypted to a key nobody holds. Require a full 32-byte key, so a partial one is absorbed as "no key" (nullptr) rather than as a truncated one. WarmNodeStore::place() already treats a null key as keyless and clears the slot's stale key when repurposing it. This aligns the site with its two siblings, which both already gate on `size == 32` (the purge path in cleanupMeshDB and the runtime eviction in getOrCreateMeshNode). The ingress gap - updateUser accepting a 1..31-byte key at all - is a separate, larger change and is left for its own review. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(NodeDB): shorten warm-demotion comment to two lines Repo guideline (AGENTS.md): keep code comments to one or two lines. Retains the non-obvious invariant - warm entries have no key length field - and drops the restated detail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(NodeDB): cover short-key demotion into the warm tier A warm record stores 32 raw key bytes with no length field, so a partial hot-store key is indistinguishable from a real one once demoted. The public_key.size == 32 gate in demoteOldestHotNodesToWarm() is what keeps a truncated key from being laundered into a full-looking warm key, but nothing exercised it. test_migration_dropsShortKeyOnDemotion overflows the hot store with one node carrying a 31-byte key and asserts it lands as a keyless placeholder while a genuine 32-byte key still survives. push() grows a keySize parameter to seed the partial key, and clearWarm() gives the test an empty warm tier, which it needs because the warm store outlives setUp() and a prior run's warm.dat. Verified to discriminate: with the size gate reverted to size > 0 the new test fails on "a 31-byte key must not be demoted as if it were a full key", and passes again once restored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(NodeDB): assert the keyless placeholder carries last_heard The test only proved a warm metadata row survived the demotion, not that the placeholder does the job the nullptr is there for, which is preserving last_heard when the key is dropped. Asserting the value needed the seeds fixing first. Warm entries pack role, protected category and the xeddsa flag into the low 7 bits of last_heard (WARM_TIME_MASK is 0xFFFFFF80), so warm time has 128 second granularity and the old seeds of 1, 2, 3 all quantised to 0. They are now multiples of 128, which keeps the demotion ordering identical and makes the values survive the round trip. Real last_heard is epoch seconds, so this is closer to production than the old counter was. Reads the entry through WarmNodeStore::take() rather than getOrCreateMeshNode(), which does not restore last_heard from the warm tier and would have been asserting a path that does not exist. Reported by CodeRabbit on #11431. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Ben Meadors <benmmeadors@gmail.com> |
||
|
Thomas Göttgens
|
80f8611e65 |
feat(variants): add Seeed Wio Tracker L1 Pro 1W (#11542)
* fix(sx126x): allow boards to opt out of the PA optimization table Boards driving an external PA can define SX126X_NO_POWER_OPTIMIZATION_TABLE to use the fixed PA config instead of RadioLib's table, which is tuned for a bare SX126x. Default behaviour is unchanged. init() applies the fixed config after begin(), which programs power through the table. * feat(variants): add Seeed Wio Tracker L1 Pro 1W nRF52840 + SX1262 with a 1 W external PA, L76K GNSS, SH1106 OLED. Uses hw_model 144 (meshtastic/protobufs#1038), opts into SX126X_NO_POWER_OPTIMIZATION_TABLE and declares SX126X_MAX_POWER explicitly. The PA gain table is indexed by SX1262 output power in dBm. Requires protobufs#1038 and a protobuf regen before it builds. * chore(deps): bump RadioLib to 510e00cf Carries the current LR11x0 and LR2021 fixes. * fix(variants): correct L1 Pro 1W QSPI pins and clean up comments PIN_QSPI_* are logical pin indices. The QSPI flash sits at D19-D24 in variant.cpp, but the defines carried D21-D26 from seeed_solar_node, where that block does start at D21. D25 and D26 are trackball pins. Also replaces mis-encoded characters in the pin comments and drops the migration note, which referenced a private repo path and a stale PINS_COUNT. * fix(variants): move L1 Pro 1W out of the per-PR build matrix board_level = pr is the high-attention tier that builds on every PR. This board belongs with the mainline set, which uses board_level = release. |
||
|
Thomas Göttgens
|
9c027a24ea |
Toggle GPS and buzzer together on the ThinkNode M8 function button double click (#11551)
* Toggle GPS and buzzer together on the ThinkNode M8 function button double click * Shorten the comments added with the ThinkNode M8 double click toggle * Only sync the buzzer when the GPS mode actually toggles, and unmute before the tone plays |
||
|
Thomas Göttgens
|
bb6a81f1e9 |
Pass framebuffer rotation through DisplayDriverConfig (#11534)
* tftSetup: pass framebuffer rotation via DisplayDriverConfig Replaces the MESHTASTIC_FB_ROTATION environment variable with DisplayDriverConfig::rotation(), which device-ui reads in FBDriver::create(const DisplayDriverConfig &). * tftSetup: carry framebuffer rotation in the panel config Use the DisplayDriverConfig builder with panel_config_t::offset_rotation instead of a dedicated rotation setter. Width and height fall back to the device-ui defaults when the yaml does not set them. * tftSetup: pass the framebuffer panel config unfiltered Take Display.Width, Display.Height and Display.OffsetRotate straight from the portduino config, like the CUSTOM_TFT branch does. |
||
|
Andrew YongandThomas Göttgens
|
93d15a5368 |
Add AS3935 lightning sensor support (#10931)
* Add AS3935 lightning sensor support Implements meshtastic/firmware#10774: an AS3935Sensor (TelemetrySensor subclass) that reports lightning_strike_count_1h and lightning_distance_km on the normal environment telemetry interval, like a rain gauge - strikes are counted over a fixed rolling ~1h window and read non-destructively, so replying to a peer's telemetry request in between broadcasts can't silently drop counted strikes. The AS3935's IRQ pin (opt-in per board via AS3935_IRQ) is polled with a plain digitalRead() in runOnce(), deliberately not attachInterrupt(): the IRQ line is a level that stays asserted until its interrupt register is read, so polling can't miss an event regardless of timing, matching the SparkFun library's own reference examples. An interrupt would also buy nothing here even setting that aside - classification requires an I2C read (readInterruptReg(), which itself calls delay(2) per the datasheet's settle-time requirement), and blocking I2C/delay() calls aren't safe from ISR context on any of this codebase's target platforms, so the ISR could only ever set a flag for later draining - no less work than just polling the pin directly on the next tick. A genuine lightning classification also requests an immediate out-of-cycle send via a new EnvironmentTelemetryModule:: requestImmediateSend() hook. There's no fixed debounce on the request itself - EnvironmentTelemetryModule's existing airtime/duty-cycle gate already paces every send, so it sends as often as airtime allows rather than an arbitrary fixed rate. The request does expire after 5 minutes unfulfilled, so it can't fire an arbitrarily stale broadcast if airtime was blocked for a long stretch. The AS3935's I2C addresses (0x01-0x03) fall inside the range this codebase's I2C scanner otherwise skips as reserved, so detection is a small dedicated probe gated behind AS3935_IRQ and respecting the caller's address filter, rather than a change to the general scan loop. Presence is confirmed via a register write/readback round-trip rather than a fixed expected value, since the AS3935 has no WHOAMI register and a power-on-reset-only check can't survive a warm reboot that doesn't power-cycle the sensor (initDevice() permanently rewrites that register on first configuration). Generated files under src/mesh/generated/ are intentionally excluded from this commit - they're regenerated from the protobufs submodule by update_protobufs.yml, and hand edits get overwritten and conflict once the companion protobufs PR merges and the submodule pointer updates. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> * fix(as3935): calibration and telemetry logging initDevice() never called the library's calibrateOsc(). The AS3935's internal oscillators are calibrated against the antenna's resonance, which the AFE/watchdog/spike-rejection thresholds depend on; without it, only a directly-driven IRQ pin (bypassing detection entirely) reacted during testing. The sensor could already have a historical detection event latching the IRQ pin high before our initialization. Added an explicit drain read after the IRQ pin is configured, so the sensor doesn't start out stuck asserting IRQ. EnvironmentTelemetryModule::sendTelemetry() logs every other environment metric category on send but was missing lightning; added a matching log line. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg> * Support AS3935 without an IRQ line, make the antenna trim configurable Detection no longer requires AS3935_IRQ. The probe is gated like the other environmental sensors, so an I2C-only breakout is found on any board. Where AS3935_IRQ is defined the pin still gates the I2C read, otherwise runOnce() polls the interrupt register, which latches until read. Antenna tuning capacitance moves to AdminMessage.sensor_config.as3935_config, persisted to /prefs/as3935.dat and defaulting to 96pF. The chip does not retain it across power loss. Disturbers are masked in the chip, since runOnce() now polls every second. The lightning telemetry log is guarded so nodes without the sensor no longer log it on every send. Requires meshtastic/protobufs#981. * Revert protobufs pointer to the develop baseline The submodule bump conflicts on merge and the generated headers come from an out of band CI job, so the pointer moves with that job rather than in this branch. * Report lightning strikes over a true rolling hour strikeCountWindow was zeroed on a fixed interval, so lightning_strike_count_1h reported strikes since the last reset rather than over the preceding hour. RollingCounter is a fixed memory sliding window: one counter per bucket, nothing stored per event, so a storm cannot grow it. The ring holds one bucket more than the window needs so none is recycled while part of it is still inside, and the oldest bucket contributes only the fraction still in range. Both are needed to hold the span at exactly the window length rather than letting it drift by a bucket either way. Expiry is exact to one bucket rather than to the event, which is below the 5 minute floor on mesh telemetry sends. The distance expires with the last strike in the window instead of on the interval reset. Covered by test/test_rolling_counter. * Widen the RollingCounter edge weighting to 64 bit counts * inWindow is a 32 bit product, so a bucket holding more than 2^32 / BucketMs events wraps. At a 5 minute width that is about 14k: a bucket of 50000 reported 11367 instead of 40000 once it reached the window edge. Below the threshold nothing changes, so lightning was unaffected, but the helper is meant to be reused by counters with far higher rates. test_large_burst_at_window_edge covers it. The existing burst test sampled only inside the window, where the bucket is whole and never weighted. * Trim RollingCounter comments to the house limit --------- Signed-off-by: Andrew Yong <me@ndoo.sg> Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> |
||
|
Ben MeadorsandClaude Fable 5
|
74119c088b |
fix(mesh): don't reference the position module on MESHTASTIC_EXCLUDE_GPS builds
The event-channel position-request reply added in #11545 calls positionModule-> replyOnPositionChannel() guarded only by USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL. Targets that set MESHTASTIC_EXCLUDE_GPS (repeaters such as rak_wismesh_repeater_mini_hp) never construct PositionModule in Modules.cpp, so an event build for one of those fails to link: undefined reference to `PositionModule::replyOnPositionChannel(...)' undefined reference to `positionModule' Guard the call, the include and the isEventChannelPositionRequestForUs() helper with !MESHTASTIC_EXCLUDE_GPS, matching how AdminModule guards its positionModule use. A node with no position module has nothing to answer a position request with, so skipping the reply is the correct behavior there. Not reachable on develop, where the userpref defaults off and the whole block compiles out - it only breaks builds that enable it, which is why #11545 was green. Verified by building rak_wismesh_repeater_mini_hp with the pref enabled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Ben MeadorsandClaude Fable 5
|
a5fc95f774 |
fix(mesh): coerce coordinate traffic to the position channel on event builds (#11545)
Under USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL every coordinate packet a client aimed at the event channel was rejected with the "Location sharing is disabled on this channel" notification - including the phone's own location feed. Both apps hand a GPS-less node its fix as a POSITION_APP packet addressed to the node itself on channel 0; that packet never leaves the device (Router::sendLocal delivers it locally) but resolved to the event channel and was dropped before PositionModule saw it. Result: the toast on every location tick, and nodes without a GPS never learned a position to share on their private channel. Position traffic now converges on the position channel - findPositionChannel(), the first channel with non-zero on-wire precision, which is never the event channel: - From-us-to-us coordinate packets are exempt from the event block. - Local coordinate sends aimed at the event channel (phone share-location, request-position, waypoints, any module/UI originator) are moved onto the position channel in Router::sendLocal and PhoneAPI instead of rejected. The client notification is only sent when no channel carries positions at all. - A position request DM'd to us on the event channel is answered on the position channel at that channel's precision (request_id preserved, same reply throttle); the requester's coordinates are still not stored, forwarded, relayed or published. want_response from the bitfield is merged before the event-channel decode short-circuit so such requests are seen. - PositionModule::sendOurPosition, positionUnchangedSinceLastSend and MeshService::trySendPosition use the shared helper instead of three copies of the same walk. Non-event builds are unaffected: the coercion compiles out and the helper matches the previous walk. Tests: coverage-event-policy (test_event_channel_phone_api, test_event_channel_router, test_position_precision, test_mqtt, test_nexthop_routing) and the same suites with the policy off. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Ixitxachitl
|
8fe246e250 |
fix(mesh): relay foreign packets whose channel hash collides with a local channel (#11544)
* fix(mesh): relay foreign packets whose channel hash collides with a local channel
The channel hash is one byte, so a foreign channel's name/PSK can fold to the
same hash as a local channel (~1/256 per local channel held). Since
|
||
|
Ben Meadors
|
48699a7a48 |
fix(http): keep reaping open TLS connections under low heap so the heap can recover (#11539)
* fix(http): keep reaping open TLS connections under low heap so the heap can recover Once free heap dropped below MIN_HEAP_FOR_SSL (40 KB) with HTTPS connections open, the node's heap never came back and every later HTTPS or TCP-API connection failed until a reset - node alive, on WiFi, unusable. handleWebResponse() skipped secureServer->loop() entirely under low heap so no new TLS handshake would be attempted on a heap that can't hold its context. But HTTPServer::loop() is the only place already-accepted connections are serviced and reaped: its first pass calls ->loop() on each open one (where the 20 s idle timeout and the SSL close-notify state machine run) and deletes the closed ones. Skipping the whole loop froze the up-to-MAX_HTTPS_CONNECTIONS TLS sessions already open. Never looped, they never timed out, their mbedTLS contexts and pbufs were never freed, so free heap never climbed back over 40 KB, so the loop was skipped forever. The guard's own precondition was what kept it from clearing. Split the two halves. Under low heap keep driving and reaping the connections we already hold, and only skip the accept. HTTPServer keeps its connection table protected, so a thin MeshHTTPSServer subclass exposes serviceExistingConnections(), the first half of HTTPServer::loop() verbatim. Log line reworded to say what now happens: not accepting, not skipping. Verified on a Heltec V3 (Endor AP) against a control build with #11537 (so the node survives the squeeze instead of aborting first): - Recipe: held sockets on 80/4403 + pending TLS, 100 s of HTTPS pokes, repeat. Control: Low heap pins at 6-17 KB, HTTPS dead, and 3 min after all pressure is released heap is still ~12 KB with Low heap firing every 30 s - permanent until reset. Fix: never dips under 40 KB, both pressure rounds 3/3, 65 KB after. - Branch driven deliberately (verify-only heap hog pinning free heap at ~28 KB with a real idle TLS session held open): under the guard the fix logs open=1 -> reaped=1 at the 20 s idle timeout, and heap goes 26 -> 65 KB before the hog is even released. On the control logic that session stays frozen for the whole window. Fixes #11538. * fix(http): trim the low-heap comments to the two-line guideline The mechanism is in the commit message and PR; the source keeps the one-line why. No code change. (CodeRabbit) |
||
|
github-actions[bot]andcaveman99
|
692adc8131 |
Update protobufs (#11543)
Co-authored-by: caveman99 <25002+caveman99@users.noreply.github.com> |
||
|
Ben Meadors
|
fe15786dc1 |
fix(api): stop rebooting ESP32 nodes when a client connects to a fragmented heap (#11537)
* fix(api): stop rebooting ESP32 nodes when a client connects to a fragmented heap
Connecting a client to an ESP32 node over WiFi/TCP rebooted the node. Two
allocations on the accept + config path use operator new, and on ESP32 that is
fatal when it fails: the framework builds with CONFIG_COMPILER_CXX_EXCEPTIONS=n
(esp32-common.ini), and ESP-IDF's cxx component then --wraps __cxa_throw and
every unwinder entry point straight to abort(). libstdc++'s operator new throws
std::bad_alloc on a NULL from malloc, so any new that cannot get its block is a
reboot with no chance to recover. Both hit on a Meshnology W12 running develop
|
||
|
github-actions[bot]andvidplace7
|
ee401242aa |
Update protobufs (#11536)
Co-authored-by: vidplace7 <1779290+vidplace7@users.noreply.github.com> |
||
|
Ben MeadorsandClaude Opus 5
|
83fd62b756 |
test(native): add 14 suites for routing, persistence, parsing and identity gaps (#11515)
* test(native): add 14 suites for routing, persistence, parsing and identity gaps Coverage audit of the native test tree; adds the highest-value untested logic as 11 new suites and extends 3 existing ones (200 test functions). New: test_stream_framing, test_nodedb_boot_recovery, test_nodedb_legacy_migration, test_nodedb_v25_roundtrip, test_nodedb_identity_hygiene, test_channel_keys, test_reliable_ack_matrix, test_hop_start_policy, test_routing_response_hops, test_phone_api_config_dump, test_observer. Extended: test_rtc, test_mqtt, test_xmodem. Two source changes the audit produced: - StreamAPI::handleRecStream copied stream->read()'s `cInt < 0` EOF check into the buffer-fed path, where there is no EOF sentinel; with signed char any byte >= 0x80 (START1 is 0x94) aborted the parse. Read the byte as uint8_t directly. Latent on develop (no callers), pinned by test_stream_framing. - Extract the post-decode pre-hop predicate from Router::handleReceived into shouldSkipHandleForPostDecodeHop() (NodeDB.h) so test_hop_start_policy drives the exact expression the router calls. No behavior change. test/state-manifest.tsv declares the suites that construct a NodeDB. Full 68-suite Docker coverage run matches the pre-change baseline. * test(native): address review - harden observer dispatch, trim comments Review follow-ups on the coverage-audit suites: - Observable::notifyObservers() erased list nodes while holding an iterator into them, so an observer that unobserves itself from onNotify corrupted the dispatch. Today the only self-detacher (PhoneAPI::onNotify -> checkConnectionTimeout -> close -> unobserve) survives solely because it returns -1 and aborts the chain before the increment; that unwritten contract is now gone. Removal during a dispatch nulls the entry and the outermost notify sweeps afterwards, which keeps self-detach, next-detach and destruction-during-notify all safe without an allocation. Hoisting the next iterator instead would have inverted the hazard and broken the existing next-detach case. Two regression tests added. - Correct the documented caller of shouldSkipHandleForPostDecodeHop: the call is in Router::dispatchReceived, not handleReceived. - Cast hop fields to unsigned at the %u call site in test_hop_start_policy. - Trim the new suites' file headers to the one-or-two-line rule in AGENTS.md. - Rename eight test functions whose names were exactly `test_` + 35 chars: that is the shape of a Lob API key, so trufflehog flagged them as secrets and failed the Trunk CI check. Full 68-suite Docker coverage run matches the pre-change baseline. * test(native): revert the observer dispatch change, keep the contract test Backs out the notifyObservers() deferred-removal hardening from the previous commit. It was reviewer-driven scope creep: nothing in the coverage audit needed it, no test required it, and it changes dispatch semantics in a header with ~76 observe() call sites on native verification alone. The hazard it addressed is not reachable today. The only observer that unobserves itself from onNotify is PhoneAPI (onNotify -> checkConnectionTimeout -> close -> unobserve), and it returns -1, which aborts the chain before the iterator is advanced past the erased node. test_self_detach_with_abort_during_notify stays: it passes against the unmodified dispatch and pins that the -1 is load-bearing, so a later cleanup that "simplifies" it away goes red. The unsafe variant (self-detach returning 0) is documented in a comment rather than tested, since asserting it would be asserting UB. * fix(serial): recover the frame behind a stray framing marker A byte that failed the START2 check was discarded rather than re-tested as a possible START1, so 0x94 0x94 0xc3 ... lost the real frame: one corrupted byte on a noisy UART silently dropped the frame behind it. Re-test the byte in place instead. Applied to both copies of the receive state machine. readStream() is the one that matters in the field - it is the serial path every phone client uses - while handleRecStream() still has no callers on develop. Strictly widens what the parser accepts; no frame that parsed before parses differently. test_stream_framing covers it on both receive paths, plus a run of stray markers and a START1-then-unrelated-byte resync. This was originally documented as a known gap in the framing suite. Fixing it instead was NomDeTom's call on review: a passing test asserting the bad behavior is what makes it hard to change later, and it is the same defect shape as the signedness fix three functions away. Also: use Throttle::deadlinePassed() in test_reliable_ack_matrix rather than a bare millis() compare, matching the house deadline rule. * test(native): cover the stray-marker resync on the buffer path too The stray-marker fix went into both copies of the receive state machine, but only test_stray_start1_before_frame_still_delivers drove both. The repeated- marker and unrelated-byte cases drove readStream() alone, so a regression in handleRecStream() would have gone unnoticed by two of the three. Verified load-bearing: reverting only the handleRecStream() half of the fix turns test_repeated_stray_start1_before_frame_still_delivers red on the new assertion. test_start1_then_unrelated_byte_resyncs stays green under that mutation by design - its failing byte is 0x00, where both branches reset to 0 - and covers the other half of the ternary. Also drops the stale header on test_stray_start1_before_frame_still_delivers, which still described the gap as pinned-as-is after the fix landed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(native): make the hop-start truth table assert the rows it prints test_truth_table_summary was six TEST_MESSAGE lines and no assertion, so it reported as a case that could not fail - the anti-pattern #11517 names in its unfinished assertion-presence lint, and the one exception to NomDeTom's "no RUN_TEST without an assertion" pass over this PR. The printed row and the checked expectation now come from one struct, so the summary cannot narrate a table the predicates no longer implement. It also covers the consequence columns the per-row tests do not assert together: classifyHopStart, shouldDropPacketForPreHop and shouldSkipHandleForPostDecodeHop for the same packet, with the expectations gated on MESHTASTIC_PREHOP_DROP. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Ben Meadors
|
e1ea653a45 |
fix(graphics): crash and leak fixes across display drivers (#11455)
* fix(graphics): crash and leak fixes across display drivers - TFTDisplay (portduino): _touch_instance was an uninitialized member, and the touch-config block only assigns it for xpt2046/stmpe610/ ft5x06 while the guard accepts any configured module. A gt911 entry in config.yaml (supported by the color-UI path) reached _touch_instance->config() through an indeterminate pointer. Initialize to nullptr and guard the config block. - Screen: the destructor freed normalFrames but leaked the owned dispdev (driver + framebuffer) and ui objects. Screen is genuinely destroyed on the portduino reboot path (screen = nullptr in Power.cpp). - EInkDisplay2: GxEPD2_BW's constructor takes the low-level driver by value and stores a copy, so the 'new EINK_DISPLAY_MODEL' at nine sites was orphaned the moment connect() returned. Pass temporaries, as GxEPD2Multi already does. - EInkParallelDisplay: the async full-refresh task cleared asyncFullRunning before nulling asyncTaskHandle, so the destructor could observe running==false with a stale handle and vTaskDelete a freed TCB. Null the handle first (same ordering fix the eink/Drivers/EInkParallel.cpp sibling already carries). - Panel_sdl: initFrameBuffer only null-checked the first of its three allocations and returned true regardless, leaving the line array full of null+offset garbage on failure; later redraws would write through those. Check all three, release partial allocations, and return false. * fix(graphics): propagate Panel_sdl framebuffer allocation failure from init() Per review: initFrameBuffer() can now fail cleanly, so init() must not register the monitor and report success when it does. |
||
|
|
c308d0aca4 |
feat: Support Elecrow ThinkNode M9 (#10908)
* thinknode-m9 variant * move lora to SPI1 device * enable SDcard * use HSPI * BaseUI tft -> HSPI * buzzer, webdav lib * fix build issues * M9 default to MUI, no BT, short ringtone * add keyboard long-press config * update variant * add ThingNode-M9 GPS string * GPS 115200 baud * Basic BaseUI support * Fixup power detection * Compass and KB fixes for M9 * add timed Lock::lock() * add SD card * point device-ui to thinknode m9 draft branch * trunk fmt * fix FusionCompass * Fix t-deck-tft linker arg list overflow in CI * SDcard/lora fix: SPI1 must not be declared twice in arduino 3.x -> reuse SPI1 defined in FSCommon.cpp * update battery parameters * reinit SD card when updating; fix PSRAM size * update lib versions * fix wakeup on key press (KB_INT) * fix default nag_timeout for TFT/MUI devices with buzzer * increase PSRAM and SD freq * trunk fmt * update lovyanGFX 1.2.26 * update device-ui commit reference * fix screen definition * remove DONE; maybe a keyword or other used identifier * fixed CI error nag_timeout * fix prepareSleep initialization * trunk fmt * reduce SD SPI frequency * update device-ui * fix SDcard issue * stage * fix device-ui commit reference * fix device-ui commit * update device-ui commit (fixed keyboard lag) * fix QMI8658 * trunk fmt * update .ini meta information, align SD freq * fix device-ui reference to target (ready to merge) * device-ui for all other targets * make the rabbit happy * trunk fmt * fixed lock screen * fix compile error * SPI lock timeout * apply device-ui fix * revert bad RadioLib commit hash in platformio.ini Co-authored-by: mverch67 <71137295+mverch67@users.noreply.github.com> * fix wrong commit hash change * fix fix commit fix * I love changing random numbers in random files --------- Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz> Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
||
|
github-actions[bot]andcaveman99
|
5dffd22584 |
Update protobufs and classes (#11531)
Co-authored-by: caveman99 <25002+caveman99@users.noreply.github.com> |
||
|
|
3a7c499722 |
fix(mesh): dedup opaque relays to prevent an undecryptable-frame broadcast storm (#11522)
* NextHopRouter: dedup opaque relays to prevent a broadcast storm
Undecryptable ("opaque") frames are relayed by relayOpaquePacket(), which by
design never enters PacketHistory - so unauthenticated frames can't poison
next-hop learning or ACK matching (packet-authenticity policy,
|
||
|
oscgonferandcoderabbitai[bot]
|
c773049b1d |
I2C reclock guard - avoid gazillion calls to reclock on SENXX sensors (#11412)
* Add SEN6X * Adds new SENXX class for SEN5X and SEN6X * Adds CO2 sensor calibration class to be shared among othre CO2 sensors * Make existing CO2 sensor draw from CO2Sensor class * Minor coment for CO2 sensor class * Move away from getRTC in SENXX class to keep track of time changes. * Change all sensors to millis for tracking time, instead of using getRTC * Add comments regarding VOC state * Avoid storing non-valid RTC Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Avoid CO2 sensor warm-up time to be below PM measured started Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Fix limits in CO2 sensor calibration Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Add pragma once on headers * Avoid non-working ASC commands Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Fix data poll * Move pm measure started before warmup check * Make cleaning non-blocking * Restore previous state if cleaning fails. Fix data ready condition. * Fix CO2 sensor checks for calibration * Add ReClockI2C guard to simplify calls to Reclock. Make SENXX calls to reclock outside of readBuffer, to avoid bizillion calls * Add new reClockGuard to all sensor classes that require it * Make clock guard store values on each construction and restore them directly * Reduce log messages * Update ADS1X15 to new guard --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> |
||
|
Tom
|
bca7c0b480 |
Tom fiddles with the test suite - again (#11517)
* test: make every suite run its own binary, and fail the run when it does not PlatformIO links every native test program to the one $BUILD_DIR/$PROGNAME path and attributes Unity output by text alone, never checking that the source file a case came from belongs to the suite it thinks it ran. Both harnesses had been split into a build pass (--without-testing) and a run pass (--without-building), and for a non-embedded platform the run pass never relinks - so all 57 suites executed whichever suite was linked last, each reporting PASSED under its own name. Introduced for CI in |
||
|
James Rich
|
51eadb77d4 |
fix(NodeDB): reset a persisted event firmware_edition on vanilla builds (#11504)
* fix(NodeDB): reset a persisted event firmware_edition on vanilla builds myNodeInfo lives in devicestate, which survives a firmware reinstall, and the boot-time edition stamp was compiled out entirely on builds without USERPREFS_FIRMWARE_EDITION. A device flashed from an event build back to vanilla therefore kept reporting the event edition forever, and clients kept its branding until a factory reset. Stamp VANILLA in the else branch so the running build is always the source of truth. * Stamp the edition before the boot save decision, and assert the on-disk value Review follow-up: the stamp sat after the devicestate CRC compare, so an edition-only change stayed RAM-only and the persisted event edition survived on disk. Move it next to the other running-build-wins fixups (device_id, min_app_version), which run inside the CRC window, and extend the test to read device.proto back so the persisted value is asserted too. |
||
|
Ben Meadors
|
f57ee0bd71 |
fix(mesh): restore the implicit ACK for our own overheard PKI DMs (#11502)
* fix(mesh): restore the implicit ACK for our own overheard PKI DMs A DM we originate is PKI-encrypted to the recipient, so when we overhear it being rebroadcast we cannot decrypt it. perhapsHandleReceived() classifies it DECODE_OPAQUE and returns before shouldFilterReceived() runs, which is where the implicit ACK for our own transmission is generated. The client therefore never receives the ROUTING_APP ack it renders as "Delivered to mesh" for a DM, and the message sits in "sending" until it either succeeds outright or times out as max retransmissions. The ACK only needs the packet header (from/id), not the decoded payload, so split it out of shouldFilterReceived() into perhapsGenerateImplicitAckForOwnOverheard() and also call it from the opaque short-circuit for packets that are from us. Behavior on the decodable path is unchanged. Broadcasts on a PSK channel decode normally and always reached the generator, which is why channel messages were unaffected and only DMs showed the symptom. * test: rename implicit-ack tests to avoid a trufflehog false positive The camelCase identifiers tripped trunk's trufflehog/Lob secret detector. * test: shorten one test name past trunk's Lob secret-detector pattern trufflehog's Lob rule matches test_ followed by exactly 35 word characters, which both new test names happened to hit. Unrelated to the fix. |
||
|
Ben Meadors
|
905482ccce |
fix(serial): don't sleep forever with pending PhoneAPI output on UART consoles (#11500)
* fix(serial): don't sleep forever with pending PhoneAPI output on UART consoles Since #11164 bounded the stream drain, a config dump can end a dispatch with output still queued. On UART-console ESP32 boards runOnce() then returns INT32_MAX with no RX pending, and neither rxInt() nor onNowHasData() fires for the remaining output, so the download wedges mid nodeinfo stream until the client happens to send a byte. Add StreamAPI::hasPendingOutput() (transport-retained frame or queued PhoneAPI data) and have SerialConsole::runOnce() short-poll (<=25ms) while it holds instead of sleeping INT32_MAX. The #11164 write budget is unchanged; idle sleep behavior with a drained queue is unchanged. The retained-frame probe also covers the ESP32-S2 USB-CDC branch, which takes the same INT32_MAX path. * test(serial): restore scratch NodeDB via tearDown, trim comments to house style A failed TEST_ASSERT longjmps out of a Unity test without running destructors, so RAII cannot restore the swapped nodeDB pointer; install the scratch NodeDB explicitly and restore/delete it in tearDown(), which runs after every test outcome. Also shorten the new comments to the two-line house limit. |
||
|
Ben Meadors
|
fb6a212b44 |
fix(graphics): make on-screen keyboard lifecycle safe and RAII-managed (#11460)
- VirtualKeyboard::handleLongPress VK_ESC invoked the onTextEntered member std::function directly, but that callback path reaches OnScreenKeyboardModule::stop(), which destroys the keyboard - and with it the std::function whose invocation is still on the stack. handlePress and submitText already deliberately copy-and-clear before invoking for exactly this reason (CannedMessageModule documents the same hazard); do the same here. - OnScreenKeyboardModule's keyboard becomes unique_ptr, replacing the delete-in-destructor / delete-then-new-in-start / delete-in-stop bookkeeping that runs on every keyboard open/close. The NotificationRenderer legacy hook receives a non-owning raw pointer, as before. |
||
|
Ben Meadors
|
0ff10318ad |
refactor(net): unique_ptr for connection-lifecycle objects (#11459)
- WiFiServerAPI/ethServerAPI apiPort and ethApiServer's listener are create/destroy cycles that repeat across WiFi teardown and W5500 chip resets; the manual delete+null bookkeeping becomes reset(). (ethTlsApiServer's listener is left for a follow-up: that file is already touched by the partial-init fix PR and converting it here would conflict.) - ContentHandler::handleFormUpload held its body parser raw with delete on four separate exit paths of a per-request handler; any future early return was a silent leak. unique_ptr removes all four. - The portduino ch341Hal global becomes unique_ptr. The LoRa-error recovery loop's delete/null/new sequence was correct only by hand-preserved ordering; it becomes reset()/make_unique. RadioLibHAL keeps a non-owning raw pointer, as before. No behavior change. |
||
|
Ben Meadors
|
5e54262fe1 |
refactor(io): unique_ptr ownership for motion sensors and I2C keyboard (#11458)
* refactor(io): unique_ptr ownership for motion sensors and I2C keyboard - AccelerometerThread / MagnetometerThread: the owned MotionSensor becomes unique_ptr, removing the manual delete/null bookkeeping in clean(). Deletion behavior is unchanged (MotionSensor's destructor is virtual). - KbI2cBase: the TCA keyboard was a reference member bound to an anonymous heap allocation - ownership was invisible and nothing could ever free it. It becomes unique_ptr with an out-of-line destructor (the base type is only forward-declared in the header). - GeoCoord::pointAtDistance returned shared_ptr with no shared ownership anywhere (and no callers); return by value instead. No behavior change. * fix(io): make TCA8418KeyboardBase destructor public for unique_ptr ownership * refactor(gps): delete dead pointAtDistance instead of converting it Per review: zero callers in this repo or device-ui, and the math was wrong at both ends (rangeMetersToRadians multiplies meters by 1852, treating meters as nautical miles). Remove it, its now-unused helper, and the <memory> include the old shared_ptr signature pulled in. |
||
|
Thomas Göttgens
|
a661fd8cd4 |
fixes #11466 (#11487)
* fixes #11466 * Keep locally-addressed routing feedback out of the phone echo filter allocForSending stamps ACK/NAK packets with from == our nodenum and sendLocal defaults to RX_SRC_RADIO, so the loopback gate never applies. Filtering on isFromUs alone dropped implicit rebroadcast ACKs, duty-cycle and NO_INTERFACE NAKs, and PhoneAPI rate-limit errors on their way to the client. Add coverage through the real RoutingModule, which the mocked one used by the rest of the suite cannot exercise, and correct the test seam comment. * Clean up the temporary RoutingModule in tearDown() A failed Unity assertion longjmps out of the test, so the in-test delete never ran and the module stayed registered in MeshModule::modules for every later test. Track it at file scope, as realNeighborInfoModule already is. |
||
|
Tom
|
a00675e00c | unset can have what it likes (#11496) | ||
|
Manuel
|
778041ec06 | fix esp32 time sync (RTC / NTP) (#11494) | ||
|
Benjamin FaershteinandBen Meadors
|
a400143090 |
fix: improve acknowledged unicast retry reliability (#11320)
* Improve acknowledged unicast retry reliability * Fix merged next-hop routing tests --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com> |
||
|
Jonathan BennettandClaude
|
faa2c8fc52 |
fix(portduino): don't segfault writing the trace file (#11493)
The TraceFile path took the first variadic argument as a char* and
streamed it, which only held while the tree's sole LOG_TRACE sites were
LOG_TRACE("%s", json). Trace-level lines without a string argument (e.g.
"Filesystem files:" from fsInit) read a garbage pointer and crashed
meshtasticd at boot whenever Logging.TraceFile was configured.
Format the message instead. The buffer covers the worst-case packet JSON
(233-byte payload escaped 6x plus metadata, ~1.7 KB); the trace file is
written untruncated today, so it must not be sized below that.
Fixes #11490
Claude-Session: https://claude.ai/code/session_01LBiZc9sfPrH1MZ2L3Fxgt1
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Ben Meadors
|
b565a07a83 |
Remove proprietary Bosch BSEC blob; open in-tree IAQ estimator for BME680 (#11381)
* Remove proprietary Bosch BSEC blob; open in-tree IAQ estimator for BME680 BSEC2 cost ~37-39 KB flash and ~4-5 KB static RAM on ~190 of ~240 build targets, linked whether or not a BME680 was attached, and was a no-source proprietary archive inside GPLv3 release binaries. The firmware consumed exactly one BSEC-exclusive output: the IAQ value. - New BME680IaqEstimator: clean-room log-domain baseline tracker (humidity-compensated gas resistance vs a rise-fast/decay-slow ceiling, 0-500 scale matching the existing UI bands), pure math, unit-tested on native (test_bme680_iaq, 15 tests incl. a deep-sleep reboot simulation). Warm-up/burn-in progress persists to /prefs/bme680.dat via SafeFile so one-sample-per-wake SENSOR nodes converge across reboots; stale /prefs/bsec.dat is removed once. - BME680Sensor: single-path rewrite on Adafruit_BME680 with async once-per-minute sampling (~20x lower heater duty than BSEC LP mode), a hard 2-minute publish-freshness bound (a dead sensor stops reporting instead of freezing its last reading on the wire), and suppression of bogus gas_resistance=0 points from heater-unstable cycles. - platformio.ini: environmental_extra_common/_extra/_no_bsec collapsed into one section; Bosch BSEC2 + BME68x deps deleted; per-variant BSEC link-path hacks and the TEMPORARY promicro lib_ignore removed. nrf52_promicro_diy_tcxo regains BME680 support at 36 KB clear of the warm-store cap; rak4631 lands at 75 KB clear. - EnvironmentTelemetry: iaq rendering gates on has_iaq (a genuine IAQ of 0 now displays); stale BSEC comments rewritten. - rak4631 size budgets tightened (113000->108000 RAM, 786000->746000 flash) to lock in the reclaimed headroom. - bin/bme680_iaq_replay.cpp: host-side replay harness for tuning the estimator against captured BSEC traces (mean abs error + band agreement), no reflashing needed. Measured (develop -> this branch): rak4631 -38.8 KB flash / -4.9 KB RAM; heltec-v3 -36.4 KB / -4.0 KB; tlora-v2-1-1_6 +1.3 KB (its IAQ approximation had been dead code since #9663 due to an inverted isfinite check and now actually runs). Note: gas_resistance stays kOhm on the wire for fleet compatibility; the proto comment claiming MOhm gets a separate meshtastic/protobufs docs PR. * Address CodeRabbit review feedback - Use Throttle::isWithinTimespanMs for all elapsed-time predicates in BME680Sensor per coding guidelines (deadline math for the async reading completion stays raw, as it targets an absolute timestamp) - Make the state file name members static constexpr - Replay tool: cast uint16_t before %u (default argument promotion), report malformed input lines instead of silently skipping, and fail non-zero on stream read errors * Address CodeRabbit nitpicks - Replace the local clampf helper with std::clamp (meshUtils.h's clamp drags in Arduino.h, which would break the estimator's standalone host build that the replay harness depends on) - Trim the replay tool's file header to a two-line summary; the full build, capture, and tuning workflow moves to docs/bme680_iaq_replay.md |
||
|
Ben MeadorsandClaude Opus 5
|
230da77642 |
fix(time): convert the millis() rollover sites #11291's CI guard cannot see (#11483)
* MeshPacketQueue: fix millis() rollover in the late-packet drop test replaceLowerPriorityPacket() read `backPacket->tx_after < now`, with `now` taken from millis() on the line above. tx_after is an absolute deadline, so that comparison inverts while the deadline sits on the far side of the 32-bit wrap: a queued late packet reads as not-yet-due for the rest of the wrap window, or every late packet reads as droppable at once. The same statement ordered two deadlines against each other with `backPacket->tx_after > p->tx_after`, which has the same problem. #11291 swept every site where millis() sits next to the comparison operator, and its CI guard matches that shape. Stashing the clock in a local first is the same bug written so the guard cannot see it. Both tests now subtract before comparing: the due test through Throttle::deadlinePassedAt(), and the ordering through the elapsed-since-now form already used in AdminModule's oldest-slot scan. The snapshot comes from Time::getMillis() so the deadlines and the test read one clock, per the convention deadlinePassedAt() documents. The `dt` the log line reports is now derived from the same elapsed value rather than recomputed. Behaviour is otherwise unchanged, save the boundary: deadlinePassedAt() is inclusive, so a deadline landing exactly on `now` reads as due rather than one millisecond early. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * RadioLibInterface: don't widen a uint32_t deadline delta into a 64-bit long TRANSMIT_DELAY_COMPLETED tested whether the front packet was still waiting with long delay_remaining = txp->tx_after ? txp->tx_after - millis() : 0; if (delay_remaining > 0) ... The subtraction is uint32_t. Where long is 32-bit - every embedded target - an already-due deadline lands negative and the packet transmits, which is why this has never been visible on device. Where long is 64-bit (portduino, and the native test build) the same value zero-extends to ~4.29e9, reads as positive, and the packet is rescheduled 49.7 days out. It stays parked until some later notifyLater() with overwrite happens to reset the timer. That is not an edge case. notifyLater() schedules through setIntervalFromNow(), so the thread wakes at or after the deadline; being a millisecond past due is the ordinary path through this branch. Ask Throttle instead. deadlinePassedAt() is the unsigned half-range test, so there is no signed conversion to get wrong at any width, and the remaining delay handed to notifyLater() is computed from the same snapshot. On 32-bit the behaviour is identical, including at the boundary: a deadline equal to now transmitted before and still does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ExpressLRSFiveWay: convert the two remaining raw window checks to Throttle runOnce() dismissed the alert frame with `now > alertingSinceMs + 2000` and chose its poll rate with `now < keyDownStart + 20000`, both against a millis() snapshot in a local. Same rollover inversion as any other naive compare, and invisible to the millis-deadline-check guard because millis() is not adjacent to the operator. update() in the same file was already on Throttle. hasElapsed()/isWithinTimespanMs() with the stored event give the full ~49.7 day range and need no snapshot. Sentinels are unchanged in meaning: `alerting` is the armed flag for alertingSinceMs and is tested first, and keyDownStart == 0 reads as "recent" for the first 20s of uptime exactly as `now < 0 + 20000` did - a poll rate either way. The arm sites move to Time::getMillis() so the writes land on the clock Throttle reads, which also puts them within reach of Time::setTestMillis(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * GPSUpdateScheduling: record whether a search is running, don't infer it elapsedSearchMs() answered "am I searching?" by ordering two raw millis() stamps: searchStartedMs > searchEndedMs. Whichever stamp lands on the far side of the 32-bit wrap reads as the larger one, so the answer inverts once per wrap cycle, in both directions: - a search that started before the wrap and ended after it keeps reading as "searching". elapsedSearchMs() then grows without bound and searchedTooLong() aborts a search that is not running. - a search that started after the wrap, following one that ended before it, reads as "idle". elapsedSearchMs() returns 0, so an unproductive search is never aborted and the receiver stays powered until it locks. Both self-heal at the next informSearching(), which bounds the damage to one GPS cycle - but the ordering test cannot be made wrap-correct, because the two stamps carry no information about which wrap they belong to. It does not need to be. Whether a search is in progress is a fact the three inform*() calls already have in hand; the ordering was only ever standing in for it. Add the flag and set it there. elapsedSearchMs() keeps its unsigned subtraction, which was always the correct part. The file's clock reads move to Time::getMillis() so the suite can drive them across the wrap. Behaviour-preserving in production - Time::getMillis() is millis() unless a test injects a clock. test_gps_update_scheduling/ gains seven cases: the idle/searching/ended states, elapsed exactness across the wrap, both inversion directions above, and reset(). The two wrap cases fail on the old predicate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * MessageStore: date boot-relative messages in uptime seconds A message received before the wall clock is trustworthy is stamped boot-relative and healed by upgradeBootRelativeTimestamps() once the RTC arrives. Both the stamp and the "same boot?" test were millis() / 1000, which wraps every 49.7 days: a stamp taken before the wrap reads as newer than `bootNow` afterwards, so `m.timestamp <= bootNow` declines to heal it and the message shows "???" until it ages out. MessageRenderer's own copy of the test falls the same way and prints invalidTime. Neither produces a wrong time - the guard is what fails safe - but Time::getUptimeSecs() landed in #11291 for exactly this, and does not wrap for 136 years. Both sites take it, which makes the comparison exact rather than merely fail-safe. While here, the autosave tick had its own hand-rolled deadline helper - `reachedMs(now, target)` as `(int32_t)(now - target) >= 0`. Wrap-correct, but a competing idiom for what Throttle::isWithinTimespanMs() already answers, and the signed cast is the form #11291 replaced everywhere else. Deleted; the stamps read Time::getMillis() so the whole path is on one clock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * WebServer: drop the hand-rolled millis() wrap branch getAdaptiveInterval() special-cased the wrap by hand: if (currentTime >= lastActivityTime) timeSinceActivity = currentTime - lastActivityTime; else timeSinceActivity = (UINT32_MAX - lastActivityTime) + currentTime + 1; Those two expressions are the same number - unsigned subtraction already computes the difference modulo 2^32 - so this is not a bug, just eight lines reimplementing what Throttle does. It also reads like a site that has thought about the wrap and settled it, which makes it a bad example to copy. Two isWithinTimespanMs() calls against the stored activity stamp, matching ethApiServer's shape for the same adaptive-interval decision. The stamps move to Time::getMillis() so the writes and the reads share a clock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * MeshPacketQueue: only order elapsed times once both deadlines have passed The late-packet eviction I rewrote compared how long ago each deadline passed: backElapsed < (uint32_t)(now - p->tx_after) That is only an ordering when both deadlines are in the past. An incoming packet whose tx_after is still in the future subtracts to a near-2^32 elapsed, which reads as the most overdue packet in the queue rather than the least - so a full queue would drop the overdue packet it was about to transmit in favour of one that is not ready yet. The comparison it replaced, `backPacket->tx_after > p->tx_after`, got this right away from the wrap; I lost it in the conversion. Classify before ordering: p->tx_after must be unset, or passed, before its elapsed time means anything. Two expired deadlines still order by which is further overdue, which is what the branch is for. Caught by CodeRabbit on #11483. test/test_meshpacket_queue/ pins the branch: the future-dated arrival that started this, both directions of the both-expired ordering, the undelayed arrival, and all of it again with the deadlines and `now` on opposite sides of the wrap. maxLen is 1 so the suite reaches the branch without dragging in CompareMeshPacketFunc and a NodeDB. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ExpressLRSFiveWay: treat "no key pressed yet" as no activity keyDownStart is 0 until the first press of a boot, and the fast-poll window read that as a press at time zero: 100ms polling for the first 20s of uptime with no activity at all, re-triggering once per millis() wrap. The arithmetic this replaced (`now < keyDownStart + 20000`) did the same, so it is not a regression - but the sentinel is exactly what the conventions say to test before the elapsed comparison, and "has there been recent key activity" has an honest answer here. 250ms is the documented floor for not missing presses, so an idle node simply starts there and moves to 100ms on the first press. Also trims the wrap-cases comment in test_gps_update_scheduling to the two-line house limit. Both from CodeRabbit review on #11483. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Jason P
|
c144fa5484 | Fix the IFDEF guard causing OLED_TINY failures (#11484) | ||
|
oscgonferandcoderabbitai[bot]
|
944fd26580 |
Add SEN6x sensors (#11390)
* Add SEN6X * Adds new SENXX class for SEN5X and SEN6X * Adds CO2 sensor calibration class to be shared among othre CO2 sensors * Make existing CO2 sensor draw from CO2Sensor class * Minor coment for CO2 sensor class * Move away from getRTC in SENXX class to keep track of time changes. * Change all sensors to millis for tracking time, instead of using getRTC * Add comments regarding VOC state * Avoid storing non-valid RTC Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Avoid CO2 sensor warm-up time to be below PM measured started Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Fix limits in CO2 sensor calibration Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Add pragma once on headers * Avoid non-working ASC commands Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Fix data poll * Move pm measure started before warmup check * Make cleaning non-blocking * Restore previous state if cleaning fails. Fix data ready condition. * Fix CO2 sensor checks for calibration --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> |
||
|
Thomas Göttgens
|
3b608b8fc5 |
fix(mesh): keep ROUTING_APP responses when toPhoneQueue is full (#11480)
* fix(mesh): keep ROUTING_APP responses when toPhoneQueue is full #2918 narrowed the overflow policy to evict the oldest entry only for TEXT_MESSAGE_APP and RANGE_TEST_APP, dropping every other portnum. A dropped ROUTING_APP response leaves the phone with no delivery confirmation for a message it sent. Add ROUTING_APP to the eviction list and pin the policy in test/test_tophone_queue. Fixes #11439 * fix(mesh): gate the queue-overflow portnum check on the decoded variant decoded.portnum aliases encrypted.size in the payload union, so an encrypted packet could be read as a privileged portnum by its ciphertext length. Restore config.device.rebroadcast_mode in the test teardown. * test: rename a test to avoid a trufflehog false positive test_text_still_admitted_when_queue_full is "test_" followed by exactly 35 characters, which matches the Lob API key shape and fails trunk check. |
||
|
Tom
|
f5314148c2 |
Serialise AirTime behind a lock, and stop handing out its buckets (#11362)
* Copy airtime reports into a caller buffer instead of exposing the array
airtimeReport() returned a pointer into the rotating bucket arrays, so the
caller held a handle to state that logAirtime() and every accessor mutate
underneath it. Copy into a caller-supplied buffer instead, and report failure
for a null buffer, a count past the log depth, or an unknown report type.
ContentHandler owns its buffer and hoists getPeriodsToLog() out of the three
calls that repeated it.
* Cover the AirTime report API and log-dispatch contract
Half of AirTime's surface had no tests: which store each report type feeds,
what airtimeReport() does when misused, how the first sync seeds itself, and
whether calling several entry points in one interval compounds the rotation.
Eighteen tests, asserted through the public API rather than the public bucket
arrays - those arrays are meant to become private, and a test that reads them
would have to be rewritten rather than pinning a contract.
Two of them state a convention that was never written down: the report arrays
are shift-ordered with slot 0 newest, and slot 0 covers only the time since the
last rotation. channelUtilization and utilizationTX use the opposite convention
- a modular ring indexed by uptime phase - and reading one as if it were the
other is a defect that has already happened once.
* Characterise AirTime window decay, TX gates, and sleep behaviour
Thirty-three tests in three kinds. Invariants must hold forever; boundaries pin
off-by-ones a refactor would move; five characterisations encode today's wrong
numbers, each tagged with the phase that will flip it.
Readings are asserted against an event-log oracle - airtime physically on air
inside (now - window, now], computed from a list of completed packets - rather
than against hand-worked constants, so a test states "this matches the
definition" instead of "this looked right when I wrote it".
The characterisations, all measured rather than assumed:
- the window covers (N-1)p + phase but divides by Np, so a steady 10% load
reads 8.33% right after a bucket boundary -> phase 5
- the same load sweeps across bucket phase instead of holding -> phase 5
- the hour window carries the same defect, 10x smaller -> phase 5
- a packet longer than its bucket is credited whole to the bucket
it completed in, so a saturated LONG_SLOW channel reads >100% -> phase 4b
- getSilentMinutes() reads a modular ring as if the index were an
age, so identical airtime gives different answers by phase -> phase 6
Two tests needed correcting during the write, both my expectations rather than
the code: a six-bucket ring sheds whole buckets, so a 30s gap drops three of
five survivors and not "half"; and the oracle sees 59 completions in a 60s
window, not 60, because the one on the lower edge is outside it.
Not written: the planned RX_LOG/RX_ALL_LOG disjointness test. That is a
property of the two radio drivers, which choose one or the other per packet -
it is not observable from AirTime, which records what it is told. The
AirTime-side half is already covered by the routing tests.
* Drop write-only and undefined AirTime members
None of this was reachable:
air_period_tx / air_period_rx file-scope mirrors of airtimes.periodTX/RX,
accumulated, rotated and memset in lockstep
with them but never read out or serialised.
Orphaned when #2552 re-pointed the writes at
bare globals instead of deleting them.
lastUtilPeriod, lastUtilPeriodTX written on every sync, read nowhere
airtimes.lastPeriodIndex written on every rotation, read nowhere
currentPeriodIndex() computes (secs / 3600) % 8 - a modular-ring
index for the one array that is shift-ordered
rather than a ring. Its only two uses were the
dead field above and a log line. It is the
fossil of the same confusion that makes
getSilentMinutes() wrong.
UtilizationPercentTX() declared, never defined
free logAirtime()/airtimeReport() declared, never defined; the latter still
carried the array-returning signature the
previous commit removed, so it actively misled
Also fixes the rotation log line, which read currentPeriodIndex() from inside
the loop although the index is advanced before it - on a multi-hour wake it
printed the same final value once per rotation. It now reports which of the
crossed hours is being rotated.
airtimeRotatePeriod() is kept: it has no caller in the tree either, but unlike
the above it is a defined public method, so out-of-tree callers are plausible.
Measured, not estimated: sizeof(AirTime) 464 -> 456 B, plus 64 B of globals, so
-72 B of static RAM. Padding accounts for the difference from the 66 B the plan
predicted by counting declared bytes.
The whole point of writing the tests first: the suite is green here with zero
test changes.
* Document what the AirTime figures measure and how they are stored
Comments only, but four of the things they replace were false.
The header's example analytics claimed RX_ALL_LOG was "all received lora
packets" and offered "RX_ALL_LOG - RX_LOG = other lora radios". Both radio
drivers pick exactly one of the two per packet, so they are disjoint: RX_ALL_LOG
is airtime we could not parse, the subtraction can go negative, and the total is
TX + RX + RX_ALL. Replaced with the actual contract - four inputs, eight
outputs, the window each spans, and the fact that the three thresholds are
hard-coded members rather than the settings they look like.
Names the two storage conventions on their declarations, because mixing them up
is what makes getSilentMinutes() wrong: channelUtilization and utilizationTX are
modular rings indexed by uptime phase, where the oldest bucket is (current + 1)
% N; airtimes.period* is shift-ordered with slot 0 newest, where the index IS an
age and slot 0 is a partial hour.
Defines the measurement as wall time rather than awake time, and says why: a
sleeping node still hears traffic, and per-node redefinition would make two
broadcast readings incomparable. Records that the 60s figure is published to the
mesh at >= 1h cadence, so what other nodes see is a snapshot - at LONG_FAST and
1% occupancy it reads exactly 0 in about 44% of reports - and that the contention
window it feeds moves in 20-percentage-point steps, so small errors never reach
the backoff.
Finally, states that rotation happens on access rather than on the scheduler
tick, names the test that enforces it, and leaves a TODO pointing at the plan
phases that fix the characterised accuracy defects.
* Serialise AirTime behind a lock proven by a private token
Two mechanisms solving different halves. A lock-free inner core (Windows) holds
all state and all logic; it has no lock and no way to reach one, so nesting is
impossible by construction. A private Held token takes the lock in its own
constructor and is the only thing that can be passed where a core method demands
one, so the lock cannot be forgotten either.
The rule is now uniform with no exceptions to remember: every public method
takes the lock once and delegates. In particular isTxAllowed*() lock like
everything else - before the split they could not, because they called the
public accessors and the lock is not recursive. That asymmetry was the foot-gun
the previous design documented in prose and hoped nobody would trip.
getPeriodsToLog()/getSecondsPerPeriod() still take no lock; they return
compile-time constants and touch no state.
channelUtilization[] and utilizationTX[] were public, so the lock was bypassable
at compile time. They move into the private core. Four test sites reached in;
all four now use logAirtime() plus the virtual clock, and no new test seam was
needed. Nothing in src/ was affected.
The re-entry assert is guarded on PIO_UNIT_TESTING, so it exists in test builds
only. The design sketched #ifdef DEBUG, but nothing in this tree defines DEBUG
or NDEBUG, so either spelling ships the assert to every board - and
nrf52_promicro_diy_tcxo has ~128 bytes of headroom under its 0xEA000 warm-store
cap, which the assert's strings and abort path overrun. It would have worked on
hardware, since the check runs in Held's owner initialiser and so precedes the
blocking take; the objection is that abort()ing a live mesh node is a poor trade
for a bug never seen in the field. Native tests are where it earns its keep
anyway: Portduino compiles Lock::lock() to an empty body, so a nested take there
succeeds silently and nothing else would notice.
Also comments out ScopedBusyAirTime in test_traffic_management. It is inert
twice over: the module holds no reference to airTime at all since hop exhaustion
was shelved, and the fixture never worked anyway - writing the buckets on a
fresh AirTime is undone by the first accessor call, which takes the firstTime
branch and memsets them. It reported 0%, not the 100% it claimed. Left in place,
commented, with both reasons recorded.
Cost on the tightest board in the tree, nrf52_promicro_diy_tcxo: the six phases
together add 96 bytes of flash, leaving it 32 bytes clear of the warm-store
guard. RAM is 72 bytes lower from the dead-state removal. Suite green at 47/47,
with test_airtime unedited apart from the added nesting test.
* Count rotations with the loop variable, not a separate tally
LOG_DEBUG compiles to nothing under DEBUG_MUTE, so the counter's only read
disappeared with it and the tally became write-only. It does not warn today -
this build has -Wunused-but-set-variable on, and it fires for other locals, but
not for one that is only initialised and never read - so it was latent rather
than broken: a stricter flag or -Werror would have failed muted builds only.
Using the loop variable removes the class of problem, since the loop condition
reads it, and drops the elapsedAirtimePeriods-- mutation as a side benefit.
Same iteration count, same output.
Found by compiling nrf52_promicro_diy_tcxo with -D DEBUG_MUTE, which is worth
recording for its own sake: muting logs takes that image from 802 784 to
673 416 bytes, 98.5% to 82.6% of flash. Logging is 16% of the largest nrf52
image, and its 32 bytes of warm-store headroom are a logging-verbosity question
rather than a code-size one.
* Tighten the comments added by this branch
Comment-only: with comments stripped, all five files are byte-identical to the
previous commit.
Removed the references to the planning notes. Those documents are working
material and will go stale; the code should not depend on them. The five
CHARACTERISATION tags now describe the defect they pin and stop there, and the
accuracy TODO names the four defects and points at the tests instead of a plan
file.
Also removed, as noise rather than information:
- comparisons against pre-#11291 behaviour, which nobody reading this needs
- a comment describing the lock restructure as future work, written before it
landed
- speculation ("plausible", "worth pinning so a future...")
- an aside arguing with an arithmetic slip made while writing the test
Kept the mechanical facts that are slow to re-derive: the two storage orderings
and which array uses which, RX_LOG/RX_ALL_LOG disjointness, the locking rule and
the addSpanned() constraint that protects it, why the re-entry assert is
test-only, and the concrete numbers - (N-1)p + phase, 14 164 ms, the 20 pp
contention-window steps.
Net 16 comment lines out of src/, 33 out of test/.
* Gate the AirTime re-entry check on the host, not on testing
PIO_UNIT_TESTING is injected by PlatformIO purely on BUILD_TYPE, with no
platform check, so it is defined on an on-target `pio test` run too. The
check arms before the lock is taken - a nested take blocks forever, so a
later check would never run - which under preemption false-positives on
legitimate contention and races on its own write.
Derive AIRTIME_REENTRY_CHECK once from PIO_UNIT_TESTING && !HAS_FREE_RTOS
and use it at all three sites. Had the three conditions ever diverged, an
on-target test build would fail to compile on a member the header no
longer declares.
* Log AirTime outside the lock it serialises
DEBUG_PORT.log() blocks on a UART write, and `lock` is a plain binary
semaphore with no priority inheritance, so holding it across a log call
lets the main thread stall the radio thread in getTxDelayMsec().
Move logAirtime()'s LOG_DEBUG into the shell, after the Held scope
closes; the shell already has both arguments, so nothing has to be
passed back out of the core. isTxAllowed{ChannelUtil,AirUtil} read into
a local under the lock and warn after it. The log bodies are braced
because LOG_DEBUG compiles away under DEBUG_MUTE and a bare `if (x) ;`
trips -Wempty-body.
Fold the two doubled index calls into `+=` while touching the lines.
* Give each airtime report its own buffer
handleReport() reused one array across the three airtimeReport() calls
and ignored the bool. A failed report would have left the previous
type's data in place and emitted it under the next type's key. Build
each through a lambda whose buffer is zeroed per call, so a failure
emits zeros.
Unreachable today - the count is always PERIODS_TO_LOG and the type is
always valid - but the old shape only read as correct by accident.
* Drop a stray semicolon from the inert-guard comment
* Address external review: name the race, tighten the claims and the tests
The header sold the lock as mechanism without naming a second thread, which
invites the reasonable objection that this is a cooperative OSThread codebase.
There is a real race and it is nRF52-only: NRF52Bluetooth registers its ToRadio
write callback with defer == false, so a phone's packet runs handleToRadio ->
sendToMesh -> Router::send on the Bluefruit BLE task, reading
utilizationTXPercent() and getSilentMinutes() while loopTask may be inside
logAirtime(). ESP32 hands BLE work to the main task and does not have it.
Three claims in the header were wrong or overstated:
- "nesting is impossible by construction" - Windows is a nested class with an
enclosing class's access rights, and `extern AirTime *airTime` is in the
same header, so airTime->anyPublicMethod() from inside it is well-formed
and would hang. Nothing does it; the assert is the backstop. Say that
instead, because the comment below instructs contributors to add helpers
to Windows on the strength of the guarantee.
- "every public method takes the lock exactly once" - two constant accessors
take none and isTxAllowedAirUtil() takes it zero or one times. State the
exceptions where the invariant is stated, not only at the definitions.
- "both radio drivers pick exactly one per packet" - five drop paths log
neither. At most one. Recorded against plan4 rather than fixed here: it
changes a telemetry value.
getPeriodsToLog()/getSecondsPerPeriod() become static constexpr, which removes
them from the locking claim structurally and lets ContentHandler size its
buffer and its count from one constant.
Tests:
- C14's saturated AirTime is installed by a helper and restored in tearDown.
Unity's TEST_ABORT() is longjmp and does not run destructors of automatic
objects, so the scoped guard it replaces would leave airTime dangling into
an abandoned frame on any assertion failure - and the same commit that
added it removed the tearDown reset that did cover that.
- test_getSilentMinutes_counts_minutes_until_enough_ages_out asserted only
`mins <= 60`, which neither return path can violate. The answer is 59.
- test_backwards_uptime_degrades_safely stepped 600s -> 60s, which leaves
elapsedAirtimePeriods at 0, so it never reached the hourly-report branch
its own comment describes. Step by the wrap instead and assert the exact
figures.
- test_airtime leaked EU_868 out of the duty-cycle case into every later one,
and the reentry test's isTxAllowedAirUtil() coverage depended on it.
Restore the region in tearDown and set it explicitly where it is wanted.
- Rename that test to what it can actually check: no single method takes the
lock twice. The calls are sequential, so it cannot catch two methods
nesting.
* trunk: suppress trufflehog/Lob false positives in test_airtime
* Address CodeRabbit review: the rotate trace, the cap warn, the backoff
Four findings from the CodeRabbit pass. Two were introduced by this branch,
one is a real inconsistency it inherited, one is a naming slip.
The rotate trace was the one that mattered. "Log AirTime outside the lock it
serialises" moved the per-packet lines and the two TX-gate warnings out to the
shell, but missed LOG_DEBUG("Rotate airtimes, crossed hour %u") because it does
not sit in the shell at all: it is inside Windows::syncNow(), the lock-free
core, which by construction only ever runs under Held. Nothing at that line
looks like a lock, which is why it survived.
The exposure is smaller than the review suggests - runOnce() syncs at 1 Hz, so
in steady state this is one line an hour, and the PERIODS_TO_LOG - 1 burst
needs an hour of light sleep with no intervening sync - but a UART write under
a plain binary semaphore with no priority inheritance is exactly what the
comment above logAirtime() says this code does not do. syncNow() now
accumulates crossings in rotationsPendingLog and runOnce() drains it inside the
Held scope, then logs after release. Any caller can cross an hour; only that
thread reports it, so a crossing raised elsewhere is traced at most one tick
late. The `if (rotations > 0)` guard keeps the drained value read under
DEBUG_MUTE, where LOG_DEBUG expands to nothing - the write-only tally that
"Count rotations with the loop variable" removed.
addFromContact()'s favorite fallback stamped silently when the protected cap
refused it. The stamp is new on this branch; the two sibling refusals (ignore,
verify) both emit PROTECTED_CAP_WARN_FMT, so the operator lost the only signal
that the cap was hit on the one path that has a fallback.
lfs_assert() mixed clocks: Throttle read Time::getMillis(), the remainder was
computed from a second, bare millis(). The review's stated failure mode - a
native test overriding the clock - cannot happen, since the hook is behind
PIO_UNIT_TESTING and this file is nRF52-only. The real defect is the second
read: a tick landing on the 20-minute boundary between the check and the
subtraction underflows the remainder into delay(~50 days), on a device that has
just found its flash corrupt. One read, clamped, and preFSBegin() stores from
the same clock.
The eviction test is renamed to
test_eviction_prefersCurrentBootStampOverPost2038Epoch. The finding is right
that it was snake_case, but the suggested testEvictionPrefers... does not match
this file either, which is test_<area>_<camelCase> throughout.
Not taken, both pre-existing and out of scope for a rollover branch:
- t5s3_epaper's touchResumeAtMs/suppressFromMs read an active suppression as
inactive if the wake lands in the 1 ms where millis() is 0. Consequence is
one skipped 150 ms touch-settle window per 49.7-day wrap.
- NRF52Bluetooth::onPairingPasskey() busy-waits 30 s in a BLE callback. Worth
saying plainly that this branch makes it more visible: the old
`millis() < start_time + 30000` overflowed at the wrap and cut the wait
short, so the correct Throttle form is what lets it run the full 30 s.
Reworking it into an OSThread is its own change.
Native suite GREEN, 48/48, 672 cases.
|
||
|
Tom
|
6745995442 |
docs: move the firmware design docs to the documentation site (#11488)
The five documents under docs/ were written in this repo while their features
were developed. Four of them describe shipped, upstream behaviour and belong on
meshtastic.org, where users and client authors will look for them:
traffic_management_module.md -> configuration/module/traffic-management
+ development/reference/traffic-management-internals
node_info_stores.md -> development/reference/node-info-stores
mesh_beacon_module.md -> configuration/module/mesh-beacon
+ development/reference/mesh-beacon-internals
+ development/device/mesh-beacon-client-interface
lora_region_preset_compatibility_client_spec.md
-> development/device/region-preset-compatibility
Each is split by audience: settings pages carry the config surface in user
terms, reference pages carry firmware mechanism, and the device pages carry the
protocol a client app speaks. The region-preset spec always said it should
graduate out of this repo once its protobuf landed upstream, which it has
(FromRadio.region_presets, field 19).
nexthop-routing-reliability.md is not documentation - it is a working document
with a mitigation plan, a "files to modify" list and commit sequencing. Its
mitigations shipped in #10745, so the plan is history and the analysis is
superseded; it is dropped rather than published.
Comments that cited the deleted files now point at the published pages, and the
NextHop test header cites #10745 instead of the deleted plan.
|
||
|
hackengineerandClaude Fable 5
|
c5d3321a41 |
Fix unterminated MyNodeInfo.pio_env when APP_ENV is 40+ chars (#11468)
strncpy does not null-terminate when the source fills the destination. A PlatformIO environment name of 40 or more characters leaves pio_env unterminated, nanopb aborts the whole MyInfo encode with 'unterminated string', getFromRadio() returns 0 bytes forever, and the client app never receives any config after want_config_id. Clients that receive a redacted MyInfo (pio_env cleared before encode) are unaffected, which makes the failure look client-specific when it is not. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Ben MeadorsandThomas Göttgens
|
fa031c95dc |
perf(crypto): stop heap-allocating a cipher object per packet (#11462)
* perf(crypto): stop heap-allocating a cipher object per packet encryptAESCtr() constructed a fresh CTR<AES128/256> on the heap for every call - once per encrypted transmit and once per channel decrypt attempt on every received encrypted packet. On the platforms that use this base implementation (STM32WL, RP2040, nRF54L15, portduino) that is avoidable per-packet malloc/free churn on small heaps. Reuse lazily-created singletons instead. Safe for the same reason the function's static scratch buffer already is: every caller serializes under cryptLock, and setKey/setIV reinitialize the cipher state each call. Lazy heap pointers rather than static objects so ESP32/nRF52 (which override this method) never reserve the RAM. * Improve comments in encryptAESCtr function Refactor comments for clarity and conciseness in AES-CTR encryption implementation. --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> |
||
|
oscgonferandThomas Göttgens
|
a65d9aef39 |
Add ADS1X15 ADC (#9846)
* Add ADS1X15 class * Initialize bus and address in ADS1X15Sensor * Initialize member variables and pass bus to ads1x15 begin object * More register values options for ADS1X15 * Mark constructor as explicit * Move ADS1X15 from PowerTelemetry to EnvironmentTelemetry: * Adds ADS to Environment Telemetry * Adds possibility to use template function with multiple devices of the same type on the same bus with different addresses * Moves moduleConfig dev overrides to i2cScan function * Make logs in env telemetry only show what has been collected * Fix channel naming and logging * Remove scannerToSensorsMap for ADS1X15 * Fix ADS1X15 reclock * Fix merge * Trunk format issue * Set port * Remove status overrride * Return status on boot * Set data rate as per coderabbit request. * Add define for ADS type and use it to set SPS * Add object based on define --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> |
||
|
Andrew YongandJonathan Bennett
|
b4c2eb0b78 |
refactor(led): generalize LED_LORA init from ThinkNode-M7 (#11437)
Migrate LED_LORA init to match existing LED init patterns. Signed-off-by: Andrew Yong <me@ndoo.sg> Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz> |
||
|
Ben Meadors
|
4296d5d584 |
fix(eth): free partially initialized TLS contexts on init failure (#11452)
* fix(eth): free partially initialized TLS contexts on init failure initTlsContext() inits the four mbedtls contexts and populates them step by step, but every failure return left the already-parsed material (X.509 chain, EC key, ssl config) allocated. Since tlsReady is only set after full success, deInitEthTlsApiServer()'s cleanup - guarded by if (tlsReady) - could never reclaim that partial state, and runOnce() hard-fails with the contexts stranded for the life of the process. Also reachable via the cert-regeneration path when a DHCP lease change bumps the cert generation and the rebuild fails. Factor the four frees into freeTlsContexts() (safe on init-but- unpopulated contexts) and call it from every initTlsContext failure return; deInit now uses the same helper. * docs(eth): shorten freeTlsContexts comment per review |