From 6b9d205b00ee84468baefdbe70f1e60707b4fdaa Mon Sep 17 00:00:00 2001 From: ZephCore T096 Date: Thu, 8 Oct 2026 17:48:15 +0200 Subject: [PATCH] leds: do not reclaim a PWM-shared pin as plain GPIO in main() server_main_common.cpp and main_observer.cpp both configured led0 as a plain GPIO output at boot, unconditionally. On ESP32 that calls esp_rom_gpio_matrix_out() under the hood, which routes the pin's output to the GPIO signal and takes it away from whatever peripheral had it before, including the LEDC channel a heartbeat-pwm-led or lora-tx-pwm-led alias had already claimed during the PWM driver's own init. The PWM write then succeeds (no error, no warning) but never reaches the pin: confirmed on V4.3 hardware, where the same pin lights up fine over plain GPIO, and over PWM in a different (non-Zephyr) firmware, but stays dark over Zephyr's PWM once this GPIO configure has run. Companion builds never hit this (they don't use server_main_common.cpp) and nRF52 isn't affected (no shared GPIO matrix), which is why T096 tested fine while V4.3 didn't. Fix is alias-based, not a board list, so any future board with either PWM alias is covered without extra wiring. Also: CLI_HAS_RADIO_LED only checked the plain lora-tx-led alias, so "set leds.radio" replied "(no radio LED on this board)" on boards whose radio LED exists only via lora-tx-pwm-led (V4.3). Check both. Documents the two PWM aliases in the board porting guide, and leds.brightness plus the corrected LED topology counts in docs/CLI_commands.md. Co-Authored-By: Claude Sonnet 5 --- docs/CLI_commands.md | 25 ++++++++--- zephcore/app/main_observer.cpp | 12 +++++- zephcore/boards/example_board/README.md | 1 + zephcore/boards/example_board/board.overlay | 48 +++++++++++++++++++++ zephcore/helpers/CommonCLI.cpp | 6 ++- zephcore/src/server_main_common.cpp | 12 +++++- 6 files changed, 91 insertions(+), 13 deletions(-) diff --git a/docs/CLI_commands.md b/docs/CLI_commands.md index 7849899..f094afc 100644 --- a/docs/CLI_commands.md +++ b/docs/CLI_commands.md @@ -358,8 +358,9 @@ The observer role (ESP32, `boards/common/observer.conf`) has its own CLI and non | `get owner.info` | Owner/contact info (pipes `\|` display as newlines) | | `get int.thresh` | Interference threshold | | `get leds` | *(ZephCore only)* LED master switch: `on` or `off` | -| `get leds.radio` | *(ZephCore only)* Activity-LED mode: `tx`, `rx`, `all` or `off`. Appends `(no radio LED on this board)` where the board has no `lora-tx-led` alias | +| `get leds.radio` | *(ZephCore only)* Activity-LED mode: `tx`, `rx`, `all` or `off`. Appends `(no radio LED on this board)` where the board has neither a `lora-tx-led` nor a `lora-tx-pwm-led` alias | | `get leds.hb` | *(ZephCore only)* Heartbeat-LED mode: `all`, `hb`, `unread` or `off`. Appends `(no heartbeat LED on this board)` where the board has neither `led0` nor `led1` | +| `get leds.brightness` | *(ZephCore only)* Brightness of a PWM-capable heartbeat/activity LED, `0` to `100` as a percentage. Boards without a `heartbeat-pwm-led` / `lora-tx-pwm-led` alias still report a value (nothing reads it) | | `get buzzer` | *(ZephCore only; room server only)* Buzzer/vibration mode as ` ()`: `0 (silent)`, `1 (sound+vib)`, `2 (vibrate)`, `3 (sound)`. Compiled out on repeater builds (`#ifndef ZEPHCORE_REPEATER`) — a repeater answers `unknown config: buzzer`. | | `get agc.reset.interval` | Removed — replies `Removed - Automatic AGC reset is on`. Periodic AGC recalibration was deleted (it reset the noise floor to its unseeded sentinel on every fire). Use `set rxduty` to cut RX current. | | `get multi.acks` | Extra ACK transmit count (`0` or `1`) | @@ -441,8 +442,9 @@ four radio parameters together, since they are one interop-critical set. | `set int.thresh ` | | Interference detection threshold | | `set buzzer <0\|1\|2\|3>` | or `off` / `on` / `vibrate` / `sound` | *(ZephCore only; room server only)* `0`/`off` silent, `1`/`on` sound + vibration, `2`/`vibrate` vibration only, `3`/`sound` sound only. Modes 2 and 3 need a vibration motor; without one the node replies `Error: no vibration motor on this board - use 0 or 1`. Applied live and persisted. Compiled out on repeater builds. | | `set leds ` | default **on** | *(ZephCore only)* Master switch for every LED on the node, applied live and persisted: heartbeat, unread-message and LoRa TX-activity LEDs, plus the message and shutdown flashes. Works on every role, including headless repeaters where the TX LED is the only one that ever lights. Does **not** cover the display backlight, which is a separate UI brightness setting. | -| `set leds.radio ` | default **tx** | *(ZephCore only)* What the LoRa activity LED reacts to, applied live and persisted. `tx` lights it for the duration of each transmit (the behaviour before this setting existed), `rx` gives a 30 ms blink per valid packet received, `all` does both, `off` keeps it dark. Sits **below** `set leds` — the master switch off keeps it dark whatever this says. Only boards defining the `lora-tx-led` alias have this LED; elsewhere the value is stored but does nothing, and the reply says so. | +| `set leds.radio ` | default **tx** | *(ZephCore only)* What the LoRa activity LED reacts to, applied live and persisted. `tx` lights it for the duration of each transmit (the behaviour before this setting existed), `rx` gives a 30 ms blink per valid packet received, `all` does both, `off` keeps it dark. Sits **below** `set leds`: the master switch off keeps it dark whatever this says. Only boards defining a `lora-tx-led` or `lora-tx-pwm-led` alias have this LED; elsewhere the value is stored but does nothing, and the reply says so. | | `set leds.hb ` | default **all** | *(ZephCore only)* What the heartbeat LED reacts to, applied live and persisted. `all` is the 4 s liveness tick that widens from 20 ms to 200 ms while messages are unread (the behaviour before this setting existed), `hb` never widens, `unread` stays dark until there are unread messages, `off` keeps it dark. Also sits below `set leds`. See the LED-topology notes below for what this does on single-LED boards. | +| `set leds.brightness <0-100>` | default **100** | *(ZephCore only)* Brightness of a PWM-capable heartbeat/activity LED, applied live and persisted, `%` suffix accepted. Only boards defining a `heartbeat-pwm-led` or `lora-tx-pwm-led` alias have a dimmable LED; elsewhere the value is stored but has no effect. Also sits below `set leds`. | | `set agc.reset.interval ` | Accepted, ignored | Removed — replies `Removed - Automatic AGC reset is on`. The prefs byte is still read and written so the on-flash layout stays byte-exact, but nothing acts on it. | | `set multi.acks <0\|1>` | | Enable extra ACK transmits | | `set path.hash.mode ` | 0, 1, or 2 | Path hashing algorithm | @@ -477,9 +479,9 @@ four radio parameters together, since they are one interop-critical set. ## Notes -- **LED topology differs by board, and it changes what `leds.radio` / `leds.hb` can do.** Two DT aliases - decide it: `lora-tx-led` drives the radio activity LED, and `led0` (falling back to `led1`) drives the - heartbeat. Of the 35 boards in tree: +- **LED topology differs by board, and it changes what `leds.radio` / `leds.hb` can do.** DT aliases + decide it: `lora-tx-led` or `lora-tx-pwm-led` drives the radio activity LED, and `led0` (falling back + to `led1`) drives the heartbeat. Of the 35 boards in tree: - **Separate pins (6)** — `rak4631`, `sensecap_solar`, `thinknode_m1`, `thinknode_m6`, `lilygo_techo`, `xiao_nrf52840`. Both settings are fully independent. `xiao_nrf52840` has three LEDs: blue heartbeat, green unread, red radio. @@ -487,8 +489,17 @@ four radio parameters together, since they are one interop-critical set. `gat562_30s`, `rak3401_1watt`, `rak_wismesh_tag`, `lilygo_timpulse_plus`. Here `lora-tx-led` **is** the heartbeat pin, so both settings drive one LED. Radio activity takes priority and the heartbeat yields while the radio holds the pin, so a transmit is never blanked mid-packet by the heartbeat's off-timer - (and vice versa). Use `set leds.hb off` for an unambiguous radio indicator. - - **No radio LED (20)** — including `wio_tracker_l1`, `t1000_e`, `meshtracker_x1`, Heltec V3/V4/V43 and + (and vice versa). Use `set leds.hb off` for an unambiguous radio indicator. `heltec_t096` and + `heltec_wireless_tracker_v2` also alias the same LED to `heartbeat-pwm-led`/`lora-tx-pwm-led`, so it is + dimmable with `leds.brightness`. + - **One shared PWM-only pin (1)**: `heltec_wifi_lora32_v43` has no plain `lora-tx-led`, only + `lora-tx-pwm-led` aliased to the same PWM LED as the heartbeat: a radio LED that exists only through + the PWM path, dimmable with `leds.brightness` like the boards above. On ESP32, a board in either PWM + category must not configure that pin as plain GPIO in `main()`, or the GPIO matrix takes the pin back + from the LEDC peripheral and the PWM output goes dark; `src/server_main_common.cpp` and + `app/main_observer.cpp` skip it automatically (`ZEPHCORE_HAS_PWM_SHARED_LED`) whenever a PWM LED alias + is present. + - **No radio LED (19)**: including `wio_tracker_l1`, `t1000_e`, `meshtracker_x1`, Heltec V3/V4 and both ProMicros. `set leds.radio` is accepted and stored (so the setting survives onto a board that does have the LED) but does nothing; the reply says `(no radio LED on this board)`. - **No heartbeat LED (1)** — `lilygo_t3s3` has `lora-tx-led` but neither `led0` nor `led1`, so `set leds.hb` diff --git a/zephcore/app/main_observer.cpp b/zephcore/app/main_observer.cpp index f83e6d9..7f133ae 100644 --- a/zephcore/app/main_observer.cpp +++ b/zephcore/app/main_observer.cpp @@ -44,7 +44,15 @@ LOG_MODULE_REGISTER(zephcore_observer_main, CONFIG_ZEPHCORE_MAIN_LOG_LEVEL); /* ========== LED (optional) ========== */ -#if DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) +/* True when a board aliases its heartbeat or TX-activity LED to a PWM node. + * On ESP32, configuring led0 as plain GPIO after the PWM driver has already + * routed the pin takes the pin back from the LEDC peripheral (the GPIO + * matrix only serves one signal at a time), so on those boards led0 must be + * left to the PWM driver instead of reconfigured here. */ +#define ZEPHCORE_HAS_PWM_SHARED_LED \ + (DT_NODE_EXISTS(DT_ALIAS(heartbeat_pwm_led)) || DT_NODE_EXISTS(DT_ALIAS(lora_tx_pwm_led))) + +#if DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) && !ZEPHCORE_HAS_PWM_SHARED_LED #include #define LED0_NODE DT_ALIAS(led0) static const struct gpio_dt_spec led0 = GPIO_DT_SPEC_GET(LED0_NODE, gpios); @@ -320,7 +328,7 @@ int main(void) LOG_INF("=== ZephCore Observer starting ==="); /* Configure LED */ -#if DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) +#if DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) && !ZEPHCORE_HAS_PWM_SHARED_LED if (gpio_is_ready_dt(&led0)) { gpio_pin_configure_dt(&led0, GPIO_OUTPUT_INACTIVE); } diff --git a/zephcore/boards/example_board/README.md b/zephcore/boards/example_board/README.md index 573f4e3..c5a572c 100644 --- a/zephcore/boards/example_board/README.md +++ b/zephcore/boards/example_board/README.md @@ -142,6 +142,7 @@ Only `lora0` is required. Everything else enables its feature by being present. | `lfs_partition` + `filesystem.dtsi` | `/lfs`: identity, prefs, contacts, channels | | chosen `zephyr,settings-partition` | NVS for BLE bonds (nRF52, ESP32). nRF54L and MG24 keep bonds in a file on `/lfs` | | aliases `led0`, `led1`, `lora-tx-led` | Heartbeat, unread, TX blink | +| aliases `heartbeat-pwm-led`, `lora-tx-pwm-led` | Same heartbeat/TX blink, brightness-adjustable (`leds.brightness`). Point both at one `pwm-leds` node to share a single physical LED. On ESP32, leave `led0`/`led1` out of the board's `main()` GPIO setup when either alias is present: configuring the same pin as plain GPIO after the PWM driver has claimed it takes the pin back from the LEDC peripheral (`ZEPHCORE_HAS_PWM_SHARED_LED` in `src/server_main_common.cpp` and `app/main_observer.cpp` already skips this for you) | | alias `sw0` | User button; wake source from nRF System OFF and ESP32 light sleep | | chosen `zephyr,display` | Display UI | | node labelled `buzzer` + alias `buzzer` | PWM buzzer | diff --git a/zephcore/boards/example_board/board.overlay b/zephcore/boards/example_board/board.overlay index cf482d9..706e26d 100644 --- a/zephcore/boards/example_board/board.overlay +++ b/zephcore/boards/example_board/board.overlay @@ -65,6 +65,9 @@ * led0 heartbeat LED * led1 second LED (unread messages on a companion) * lora-tx-led blinks while transmitting + * heartbeat-pwm-led, lora-tx-pwm-led + * same two roles, brightness-adjustable (leds.brightness); + * point both at one pwm-leds node to share a physical LED * sw0 user button; also the wake source from nRF System OFF * and from ESP32 light sleep * buzzer PWM buzzer (the node must also be LABELLED buzzer) @@ -604,6 +607,51 @@ zephyr_udc0: &usbd { }; #endif +#if 0 /* ---- Heartbeat/TX LED, brightness-adjustable (heltec_t096, heltec_wifi_lora32_v43) ---- + * One physical LED standing in for both led0 and lora-tx-led, but wired + * through a pwm-leds node instead of gpio-leds so leds.brightness can + * dim it. Point heartbeat-pwm-led and lora-tx-pwm-led at the same node + * to share it, same as the plain-GPIO led0/lora-tx-led pair above. + * + * ESP32 only: also declare a plain led0 (gpio-leds) for this pin if the + * board wants one, but do NOT add GPIO setup for it in the board's + * main() -- src/server_main_common.cpp and app/main_observer.cpp skip + * that already (ZEPHCORE_HAS_PWM_SHARED_LED) when either PWM alias is + * present, because on ESP32 configuring the pin as plain GPIO after + * the PWM driver has claimed it takes the pin back from the LEDC + * peripheral: the GPIO matrix only routes one signal to a pin at a + * time. nRF52 PWM and GPIO don't share hardware that way, so this does + * not apply there. */ +/ { + aliases { + heartbeat-pwm-led = &pwm_led_white; + lora-tx-pwm-led = &pwm_led_white; + }; + + pwmleds { + compatible = "pwm-leds"; + + pwm_led_white: pwm_led_0 { + pwms = <&ledc0 0 PWM_MSEC(10) PWM_POLARITY_NORMAL>; + label = "White PWM LED"; + }; + }; +}; + +&ledc0 { + status = "okay"; + pinctrl-0 = <&ledc0_default>; + pinctrl-names = "default"; + #address-cells = <1>; + #size-cells = <0>; + + channel0@0 { + reg = <0x0>; + timer = <0>; + }; +}; +#endif + #if 0 /* ---- Battery voltage (rak4631) ---- * * mV = raw * vbat-mv-multiplier / 4096, so the multiplier is the ADC's diff --git a/zephcore/helpers/CommonCLI.cpp b/zephcore/helpers/CommonCLI.cpp index 1d8f621..af12f3c 100644 --- a/zephcore/helpers/CommonCLI.cpp +++ b/zephcore/helpers/CommonCLI.cpp @@ -68,9 +68,11 @@ static const NodePrefs* cliDefaults() { /* ---- leds.radio / leds.hb mode names ----------------------------------- */ /* * Which LEDs this board has, so the CLI can say when a setting it stored - * does nothing here. Mirrors the aliases the drivers use. + * does nothing here. Mirrors the aliases the drivers use: lora-tx-led or + * lora-tx-pwm-led, led0 with an led1 fallback. */ -#define CLI_HAS_RADIO_LED DT_NODE_EXISTS(DT_ALIAS(lora_tx_led)) +#define CLI_HAS_RADIO_LED (DT_NODE_EXISTS(DT_ALIAS(lora_tx_led)) || \ + DT_NODE_EXISTS(DT_ALIAS(lora_tx_pwm_led))) #define CLI_HAS_HB_LED (DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) || \ DT_NODE_HAS_PROP(DT_ALIAS(led1), gpios)) diff --git a/zephcore/src/server_main_common.cpp b/zephcore/src/server_main_common.cpp index 1ea9351..81068f5 100644 --- a/zephcore/src/server_main_common.cpp +++ b/zephcore/src/server_main_common.cpp @@ -71,8 +71,16 @@ extern "C" void bt_ctlr_assert_handle(char *file, uint32_t line) #include "wifi_ota.h" #endif +/* True when a board aliases its heartbeat or TX-activity LED to a PWM node. + * On ESP32, configuring led0 as plain GPIO after the PWM driver has already + * routed the pin takes the pin back from the LEDC peripheral (the GPIO + * matrix only serves one signal at a time), so on those boards led0 must be + * left to the PWM driver instead of reconfigured here. */ +#define ZEPHCORE_HAS_PWM_SHARED_LED \ + (DT_NODE_EXISTS(DT_ALIAS(heartbeat_pwm_led)) || DT_NODE_EXISTS(DT_ALIAS(lora_tx_pwm_led))) + /* LED configuration */ -#if DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) +#if DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) && !ZEPHCORE_HAS_PWM_SHARED_LED #define LED0_NODE DT_ALIAS(led0) static const struct gpio_dt_spec led0 = GPIO_DT_SPEC_GET(LED0_NODE, gpios); #endif @@ -610,7 +618,7 @@ int server_main(const ServerRole &role) #endif /* Configure LEDs */ -#if DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) +#if DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) && !ZEPHCORE_HAS_PWM_SHARED_LED if (gpio_is_ready_dt(&led0)) { gpio_pin_configure_dt(&led0, GPIO_OUTPUT_INACTIVE); }