Return DIO1 to the digital GPIO block after every light sleep

zephcore/adapters/radio/LoRaRadioBase.cpp

@@ -5,6 +5,7 @@
 
 #include "LoRaRadioBase.h"
 #include "radio_common.h"
+#include "pm_sleep_guard.h"
 #include <mesh/LoRaConfig.h>
 #include <mesh/MeshCore.h>   /* MAX_TRANS_UNIT */
 #include <zephyr/kernel.h>
@@ -139,6 +140,13 @@ void LoRaRadioBase::txWaitThreadFn(void *p1, void *p2, void *p3)
 	for (;;) {
 		k_sem_take(&self->_tx_start_sem, K_FOREVER);
 
+		/* startSendRaw() took one sleep lock for the transmit it just
+		 * handed over; it is released when this iteration ends, on
+		 * whichever of the paths below it takes. */
+		struct TxSleepRelease {
+			~TxSleepRelease() { zc_pm_unblock_sleep(); }
+		} tx_sleep_release;
+
 		if (!atomic_get(&self->_tx_active)) {
 			continue;
 		}
@@ -806,6 +814,17 @@ bool LoRaRadioBase::startSendRaw(const uint8_t *bytes, int len)
 	 * out, never to this one -- upstream's STATE_IDLE reset in the same
 	 * place. */
 	atomic_set(&_tx_complete, 0);
+
+	/* No SoC light sleep from here until the wait thread has concluded this
+	 * transmit (it releases the lock), or until the failure path below.
+	 * Covers the blocking LBT CAD inside hwSendAsync() as well as the
+	 * airtime.  With nothing else runnable during a transmit, an ESP32
+	 * repeater otherwise sleeps through it and meets TX_DONE as a wake
+	 * event rather than an interrupt, and the completion lands late or not
+	 * at all (see the EXT1 note in the sx126x HAL).  Costs nothing worth
+	 * counting: the PA dominates the current draw while keyed.  A no-op
+	 * without CONFIG_PM. */
+	zc_pm_block_sleep();
 	_board->onBeforeTransmit();
 	_last_tx_start_ms = k_uptime_get_32();
 
@@ -848,6 +867,7 @@ bool LoRaRadioBase::startSendRaw(const uint8_t *bytes, int len)
 		}
 		_board->onAfterTransmit();
 		atomic_set(&_tx_active, 0);
+		zc_pm_unblock_sleep();
 		/* startReceive() is safe to call here regardless of failure
 		 * cause: on SX126x, recv_async early-returns if the driver
 		 * already restored RX on CAD-busy (Phase 2 idempotent fast

zephcore/patches/zephyr/0003-lora-sx126x-native.patch

@@ -124,6 +124,36 @@ above that (xiao_esp32s3 GPIO39, station_g2 GPIO48, thinknode_m9 GPIO42) log
 "Pin N is not wakeup capable" and cannot use light sleep with LoRa RX at all.
 Heltec V3/V4/V43 and the Wireless Trackers put DIO1 on GPIO14 and are fine.
 
+== Return DIO1 to the digital GPIO block after every light sleep ==
+
+The wake above is only half of what EXT1 needs.  Before each light sleep,
+ESP-IDF's ext1_wakeup_prepare() (esp_hw_support/sleep_modes.c) routes the
+wake pad to the RTC IO mux (RTCIO_LL_FUNC_RTC) and, with RTC_PERIPH powered
+down, engages its hold.  Nothing on the way out undoes either:
+esp_light_sleep_start() leaves both, and Zephyr's esp32_sleep_gpio_restore()
+releases only pins flagged ESP32_GPIO_SLEEP_HOLD_EN.  Espressif puts the
+obligation on the caller -- sleep_modes.rst, EXT1 warnings: rtc_gpio_deinit()
+before using the pad as a digital GPIO again, rtc_gpio_hold_dis() after a
+light-sleep wake.  Nothing called either, so from the first sleep on DIO1 sat
+on the RTC mux, held.
+
+sx126x_hal.c now registers a pm_notifier (CONFIG_PM on
+SOC_FAMILY_ESPRESSIF_ESP32 only) whose state_exit does both, and then checks
+the DIO1 level.  The edge that woke the SoC happened while the pad belonged to
+the RTC domain, so the digital edge detector never saw it; SX126x IRQs are
+sticky, so DIO1 still being high means an IRQ is pending and the handler is run
+by hand.  Gated on the level, not unconditional: the handler reads IRQ status
+over SPI, and an NSS edge in the duty-cycle sleep phase disturbs the chip's own
+RX cycle.  A duplicate kick is harmless (k_work_submit on a queued item is a
+no-op; a handler that finds the status clear does nothing).
+
+Symptom it addresses: on the allowlisted Heltec repeaters a TX_DONE that lands
+while the SoC sleeps -- the common case, nothing else runs during airtime -- is
+lost; the adapter's wait thread times out and the packet, which did go out,
+is counted nowhere (reported on a Heltec V4.3 as "sent never increments",
+1.17.4/1.17.5c).  The C++ adapter additionally holds a PM lock across each
+transmit.  Not yet verified on hardware.
+
 == Bound the lifetime of the RX-busy latch (payload-phase deadline) ==
 
 sx126x_is_receiving() reports "busy" for the whole payload phase from the
@@ -3611,10 +3641,108 @@ index 0122958c3..9d39100a7 100644
  
  #endif /* ZEPHYR_DRIVERS_LORA_SX126X_SX126X_INTERNAL_H_ */
 diff --git a/drivers/lora/native/sx126x/sx126x_hal.c b/drivers/lora/native/sx126x/sx126x_hal.c
-index 34f708958..394578956 100644
+index 34f708958..c21085e37 100644
 --- a/drivers/lora/native/sx126x/sx126x_hal.c
 +++ b/drivers/lora/native/sx126x/sx126x_hal.c
-@@ -150,7 +150,13 @@ int sx126x_hal_init(const struct device *dev)
+@@ -10,6 +10,12 @@
+ 
+ #include "sx126x.h"
+ 
++#if defined(CONFIG_PM) && defined(CONFIG_SOC_FAMILY_ESPRESSIF_ESP32)
++#include <zephyr/pm/pm.h>
++#include <driver/rtc_io.h>
++#define SX126X_HAL_ESP32_EXT1_WAKE 1
++#endif
++
+ #include <zephyr/logging/log.h>
+ LOG_MODULE_REGISTER(sx126x_hal, CONFIG_LORA_LOG_LEVEL);
+ 
+@@ -107,6 +113,84 @@ void sx126x_hal_dio1_irq_enable(const struct device *dev)
+ 	ARG_UNUSED(dev);
+ }
+ 
++#ifdef SX126X_HAL_ESP32_EXT1_WAKE
++/* Hand DIO1 back to the digital GPIO block after every light sleep.
++ *
++ * GPIO_INT_WAKEUP (sx126x_hal_init) makes the Zephyr ESP32 GPIO driver arm
++ * DIO1 as an EXT1 wake source.  Before each light sleep, ESP-IDF's
++ * ext1_wakeup_prepare() then routes the pad to the RTC IO mux
++ * (RTCIO_LL_FUNC_RTC) and, because RTC_PERIPH is powered down, puts it in
++ * hold.  Nothing on the way out undoes either: esp_light_sleep_start() leaves
++ * both in place, and Zephyr's esp32_sleep_gpio_restore() only releases pins
++ * carrying ESP32_GPIO_SLEEP_HOLD_EN.  Espressif documents the obligation on
++ * the caller (sleep_modes.rst, EXT1 warnings): call rtc_gpio_deinit() before
++ * using the pad as a digital GPIO again, and rtc_gpio_hold_dis() after a
++ * light-sleep wake.  Nobody did, so from the first sleep on DIO1 sat on the
++ * RTC mux with its hold engaged.
++ *
++ * Restoring the pad is not enough on its own.  The DIO1 edge that woke us
++ * happened while the pad belonged to the RTC domain, so the digital edge
++ * detector never saw it and the interrupt is gone.  An SX126x IRQ is sticky
++ * -- DIO1 stays asserted until ClearIrqStatus -- so the level is the
++ * authoritative record: if DIO1 is high after the restore, an IRQ is pending
++ * and the handler has to be run by hand.  Without it a TX_DONE that lands
++ * while the SoC sleeps (the common case: nothing else runs during airtime)
++ * is lost -- the C++ adapter's wait thread times out and the dispatcher never
++ * counts a packet that did go out on air -- and an RX_DONE waits until
++ * something else clears the chip's IRQ status.
++ *
++ * Deliberately gated on the level rather than kicking the handler on every
++ * wake: the handler reads IRQ status over SPI, and an NSS edge during the
++ * duty-cycle sleep phase disturbs the chip's own RX cycle.  A duplicate kick
++ * when the digital side did catch the edge is harmless -- k_work_submit on a
++ * queued item is a no-op, and a handler that finds the status already
++ * cleared does nothing.
++ *
++ * Runs from pm_state_notify(false), which may be in the waking ISR; every
++ * call here is ISR-safe.  One radio per board, so a single static device. */
++static const struct device *sx126x_pm_wake_dev;
++
++static int sx126x_hal_dio1_gpio_num(const struct sx126x_hal_config *config)
++{
++#if DT_NODE_EXISTS(DT_NODELABEL(gpio1))
++	if (config->dio1.port == DEVICE_DT_GET(DT_NODELABEL(gpio1))) {
++		return 32 + config->dio1.pin;
++	}
++#endif
++	return config->dio1.pin;
++}
++
++static void sx126x_hal_pm_state_exit(enum pm_state state)
++{
++	const struct device *dev = sx126x_pm_wake_dev;
++	const struct sx126x_hal_config *config;
++	struct sx126x_hal_data *data;
++	int num;
++
++	if (state != PM_STATE_STANDBY || dev == NULL) {
++		return;
++	}
++
++	config = dev->config;
++	data = get_hal_data(dev);
++	num = sx126x_hal_dio1_gpio_num(config);
++	if (!rtc_gpio_is_valid_gpio(num)) {
++		return;
++	}
++
++	(void)rtc_gpio_hold_dis(num);
++	(void)rtc_gpio_deinit(num);
++
++	if (data->dio1_callback != NULL && gpio_pin_get_dt(&config->dio1) > 0) {
++		data->dio1_callback(dev);
++	}
++}
++
++static struct pm_notifier sx126x_pm_notifier = {
++	.state_exit = sx126x_hal_pm_state_exit,
++};
++#endif /* SX126X_HAL_ESP32_EXT1_WAKE */
++
+ int sx126x_hal_init(const struct device *dev)
+ {
+ 	const struct sx126x_hal_config *config = dev->config;
+@@ -150,7 +234,13 @@ int sx126x_hal_init(const struct device *dev)
  		LOG_ERR("DIO1 GPIO not ready");
  		return -ENODEV;
  	}
@@ -3629,7 +3757,21 @@ index 34f708958..394578956 100644
  	if (ret < 0) {
  		LOG_ERR("Failed to configure DIO1 GPIO: %d", ret);
  		return ret;
-@@ -183,6 +189,14 @@ int sx126x_hal_init(const struct device *dev)
+@@ -164,6 +254,13 @@ int sx126x_hal_init(const struct device *dev)
+ 		return ret;
+ 	}
+ 
++#ifdef SX126X_HAL_ESP32_EXT1_WAKE
++	if (sx126x_pm_wake_dev == NULL) {
++		sx126x_pm_wake_dev = dev;
++		pm_notifier_register(&sx126x_pm_notifier);
++	}
++#endif
++
+ 	/* Configure optional GPIOs */
+ 	ret = sx126x_hal_configure_gpio(&config->antenna_enable, GPIO_OUTPUT_INACTIVE,
+ 					"antenna enable");
+@@ -183,6 +280,14 @@ int sx126x_hal_init(const struct device *dev)
  		return ret;
  	}
  
@@ -3644,7 +3786,7 @@ index 34f708958..394578956 100644
  	LOG_DBG("HAL initialized");
  	return 0;
  }
-@@ -220,15 +234,54 @@ int sx126x_hal_configure_tx_params(const struct device *dev, int8_t power,
+@@ -220,15 +325,54 @@ int sx126x_hal_configure_tx_params(const struct device *dev, int8_t power,
  		}
  		tx_power = CLAMP(power, SX1261_MIN_POWER, SX1261_MAX_POWER_TX_PARAM);
  	} else {

zephcore/src/Dispatcher.cpp

@@ -130,6 +130,13 @@ uint32_t Dispatcher::getTxStarvationDuration() const
 void Dispatcher::loop()
 {
 	if (outbound) {
+		/* Read BEFORE isSendComplete(): the radio publishes its
+		 * completion before it drops isTxActive(), so a "not active"
+		 * seen here guarantees a successful transmit is already latched
+		 * and collected by the call below.  The other order can see the
+		 * latch still empty, then the flag already clear, and abandon a
+		 * transmit that finished in between. */
+		bool tx_active = _radio->isTxActive();
 		if (_radio->isSendComplete()) {
 			/* Airtime is the modulation time of the packet that just
 			 * went out, not the wall-clock width of the send.
@@ -170,7 +177,18 @@ void Dispatcher::loop()
 			}
 			releasePacket(outbound);
 			outbound = nullptr;
-		} else if (millisHasNowPassed(outbound_expiry)) {
+		} else if (!tx_active && millisHasNowPassed(outbound_expiry)) {
+			/* Give up only once the radio itself has concluded.  Upstream
+			 * abandons at outbound_expiry alone because RadioLib has no
+			 * watchdog of its own; ours does — the TX wait thread always
+			 * ends with isTxActive() false, on a timeout of
+			 * max(TX_TIMEOUT_MS, 2 x airtime + 1 s).  Abandoning while it
+			 * still runs raced that thread: a completion published after
+			 * the expiry was then wiped by the next startSendRaw(), so a
+			 * packet that went out on air was counted nowhere — neither
+			 * sent_flood/direct nor the radio's total.  Radios that do not
+			 * implement isTxActive() report false and keep the upstream
+			 * behaviour exactly. */
 			_radio->onSendFinished();
 			logTxFail(outbound, 2 + outbound->getPathByteLen() + outbound->payload_len);
 			releasePacket(outbound);