mirror of
https://github.com/meshtastic/firmware.git
synced 2026-09-16 08:30:04 -04:00
Ben MeadorsandTom
df34ef1081
* fix(radio): recover from chip state loss in the RX/TX hot paths too * fix(radio): address CodeRabbit findings on the hot-path recovery PR (#11680) * fix(radio): address CodeRabbit findings on the hot-path recovery PR SX128x: startReceive() still called the old asserting setStandby() before the new trySetStandby(). The assert fired first, so the recovery path added below it could never run - the exact chip-state-loss crash this PR exists to fix was still live on SX128x. Remove the stale call. LR11x0: resolvedTcxoVoltage was set once after the primary begin() attempts, but two later paths - firmware recovery and the one-shot firmware update - call begin() again with tcxoVoltage and never updated it. On a TCXO_OPTIONAL board that only came up via one of those paths, reinitChip() would recover with the wrong oscillator setting. Update resolvedTcxoVoltage after each of those begin() calls too. LR20x0: reconfigure() discarded RadioLibInterface::reconfigure()'s result - the band-hop path always returned true regardless, and the same-band path reused the same flag for chip-programming errors, so a base-class failure could both mask itself as success and wrongly trigger a full re-init. Track the base-class result (reconfigureSuccess) separately from the chip result (standbySuccess), and return the former. Also shortens the recovery-rationale comments in RadioLibInterface.h and SX126xInterface.cpp to 1-2 lines per the repo's comment convention, the rationale now covered once in the base class. * fix(radio): finish the recovery ladder and stop recovery from rebooting Follow-up to the CodeRabbit findings, plus two gaps found auditing the branch against its own intent (never reboot on chip state loss; recover in place). RX left off was unrecoverable on an idle node. Every startReceive() call site is event-driven - RX/TX ISR, the CAD-busy branch, startSend()'s failure path, init(), reconfigure() - and a radio with RX off cannot raise an RX interrupt, so nothing re-arms it unless the node happens to transmit or the user changes config. A listen-only or quiet node stayed deaf for good, which is worse than the reboot this replaced. main.cpp's existing 60 s AGC tick now calls periodicRadioMaintenance(), which re-arms RX when rxOffline is set and otherwise does the AGC reset as before. In-place repair now gives up rather than retrying forever. After MAX_CHIP_RECOVERY_FAILURES consecutive failures - a throttle window apart, so minutes of a provably dead chip - schedule rebootAtMsec, the same deliberate reboot Portduino already uses for LoRa_in_error. A reboot re-runs init(), which redoes the power-enable GPIOs, settle delays and TCXO probing that begin() alone skips. Both counters reset in RadioLibInterface::startReceive(), the one point every driver reaches only once the chip accepts the RX start. SX128x: reconfigure()'s recovery reached reinitChip()'s region-mismatch branch, which rewrites config.lora.region, saves, and calls ESP.restart() / NVIC_SystemReset(). A runtime recovery must never reboot - that is the crash this path exists to prevent, and it would fire with a config save pending. Gated to the boot-time call via a fromInit parameter. LR20x0: a rejected setRxBoostedGainMode cleared the success flag and so forced a full fullBegin() chip reset. It is a warn-level cosmetic setting, treated as warn-only in LR11x0's equivalent, and not a lost-state signature. Also logs suppressed recovery attempts at debug level; previously a chip that stayed dead recorded one critical error and then went completely silent. * fix(radio): count RX re-arms, not re-inits, in the recovery ladder LR20x0's recoverChipStateLoss() is fullBegin(), which re-arms RX itself but reports success on begin() alone. A re-init that came back with RX still dead therefore reset chipRecoveryFailures, so a chip that could be re-inited forever while never receiving again held the ladder at zero and never reached the reboot. The other drivers had the same hole from the other side: the caller's retry startReceive() runs after the reset, so a retry that failed again left the count cleared. RadioLibInterface::startReceive() is now the only place the ladder clears, and it only runs once the chip actually accepted RX. The threshold is judged at the top of the next attempt - a throttle window later, after that attempt's retry (the caller's, or fullBegin's own) has had its chance to clear it. That also drops the old false positive where the reboot was armed before the retry that would have succeeded. RF95Interface::startReceive() set isReceiving directly instead of calling the base, so on RF95 nothing ever cleared rxOffline or the ladder: the first failed RX start left periodicRadioMaintenance() re-initing forever, and with the count now advancing it would have rebooted a working radio. --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com> --------- Co-authored-by: Tom <116762865+NomDeTom@users.noreply.github.com>