Repository navigation
Conversation
added 14 commits
September 5, 2026 00:20
Single LED (P0.28) doubles as heartbeat, message flash, shutdown flash and LoRa TX indicator, all sharing the same physical pin. Adds one PWM channel on that pin (heartbeat-pwm-led/lora-tx-pwm-led aliases) and a single 0-100 brightness knob (get/set led) covering every one of those events at once, deliberately RAM-only: resets to a 10% default on every reboot regardless of CLI changes made in a previous session. The existing persisted on/off gate (set leds on|off) is untouched. Boards without heartbeat-pwm-led/lora-tx-pwm-led aliases keep their current plain-GPIO behavior unchanged.
Reuses the existing leds on|off switch for brightness too, same "one command, wording vs number" convention as led.tx/led.hb on _scratch_leds_test, rather than introducing a separate led command. Drops the legacy '1'/'0' single-char aliases from set leds while at it: same 1%/0% shadowing bug already found and fixed on led.tx/led.hb on 2026-08-16, latent here until a numeric value became meaningful. get leds now reports both: "on N%" / "off (N%)".
Reverts the previous merge: "leds" now only ever parses on/off (a pure word command again, no numeric fallback), and brightness moves to its own "leds.brightness <0-100>" command instead of overloading "leds" with a value that could be either a word or a number. Checked before "leds " in the chain since it shares the same 4-byte prefix. Still RAM-only, no savePrefs() call in the brightness branch. This is the syntax settled on after comparing two options: reuse "leds" via a namespaced brightness command (chosen), vs a full per-LED led.hb/led.tx split like the RAK3401 branch (not chosen, decided to apply the "leds"/"leds.brightness" shape everywhere without exception, including any future RAK3401 rework).
Ports the T096 "Option 1" design (single LED, get/set leds on|off persisted + get/set leds.brightness 0-100 RAM-only) to the Wireless Tracker V2, which has the same single-LED-multi-purpose situation (status_led doubles as heartbeat/message-flash/shutdown-flash/TX indicator, same as T096's P0.28). Adds a new ledc0 PWM channel on GPIO18 (same physical pin as the existing plain-GPIO status_led) with heartbeat-pwm-led/lora-tx-pwm-led aliases -- the shared C++ logic (helpers/ui/ui_common.c, adapters/board/ZephyrBoard.cpp, helpers/led_gate.c/h, helpers/CommonCLI.cpp) already existed from the T096 commits and needed no changes, being board-agnostic behind DT_NODE_EXISTS checks. CONFIG_PWM=y deliberately NOT in the board defconfig: WT2 builds via --sysbuild (mcuboot + app as two images sharing that defconfig), and mcuboot's minimal kernel config has no k_sem_give()/k_sem_take(), which the ESP32 PWM LED driver needs -- CONFIG_PWM=y there links the app image fine but fails mcuboot's link. New boards/common/esp32s3_pwm_led.conf scopes it to the app image only via EXTRA_CONF_FILE, alongside the existing esp32s3_usb.conf.
leds.brightness (PWM-capable boards) was RAM-only since it was added: it always reset to ZEPHCORE_LED_DEFAULT_BRIGHTNESS_PCT (10%) on every boot, regardless of what a user had last set it to. New field led_brightness on NodePrefs, following the exact same append-only pattern as leds_radio_mode/leds_hb_mode added just before it: offset 311 in RepeaterDataStore.cpp (repeater/room-server/observer, shared file), offset 174 in ZephyrDataStore.cpp (companion). Both are backward compatible by construction: a prefs file written before this field existed simply leaves it at the initNodePrefs() default (10%), which is exactly the behaviour every already-deployed node has today -- no special-casing needed to keep "10% on a new/not-yet-upgraded node" as the effective default. CommonCLI.cpp's "set leds.brightness" handler now writes the prefs field and calls savePrefs(), same as the leds.radio/leds.hb handlers right above it; "get leds.brightness" now reads from prefs instead of the RAM getter directly, for the same reason those two already do (always in sync, prefs is the source of truth). The four boot call sites (main_companion.cpp, main_repeater.cpp, main_room_server.cpp, main_observer.cpp) each gained a zephcore_led_set_brightness_pct() call seeding the RAM cache from the persisted value, mirroring the existing leds_radio_mode/leds_hb_mode lines right next to them. Verified building for heltec_t096/nrf52840 (companion) and rak3401_1watt (repeater), the two roles/datastores touched by this change.
The RX-pulse/pin-hold interlock (s_tx_lit/s_rx_pulse_off/ zephcore_led_radio_hold_pin) was gated by HAS_TX_LED alone, so it never compiled in for any PWM-capable board (HAS_TX_LED_PWM). Left two real gaps: - T096 and Wireless Tracker V2 alias heartbeat and TX activity to the SAME physical PWM LED (ZEPHCORE_LED_PIN_SHARED's intent exactly), but had none of the arbitration that protects every plain-GPIO board with the same wiring -- a transmit and a heartbeat tick could still clip each other on the shared PWM channel. - All PWM boards, RAK3401 included, had no onPacketReceived() blink at all: "set leds.radio rx" (or "all") silently did nothing on the activity LED, since that whole code path only existed under HAS_TX_LED. Fixed by widening every "#if HAS_TX_LED" guard around this machinery to "#if HAS_TX_LED_PWM || HAS_TX_LED", and adding a small tx_led_write(bool) helper (mirroring heartbeat_led_write() in helpers/ui/ui_common.c) so the shared interlock logic doesn't need to know which path is compiled in -- only the final write differs. ZEPHCORE_LED_PIN_SHARED itself gained a PWM branch: true when heartbeat-pwm-led and lora-tx-pwm-led alias the same DT node (T096, WT2), same as it already was for the GPIO led0/lora-tx-led case. RAK3401's two PWM LEDs are on distinct nodes, so it correctly stays unshared -- no arbitration overhead there, but it does now get the RX blink like every other PWM board. The heartbeat side (helpers/ui/ui_common.c) needed no change: it already checks zephcore_led_radio_holds_pin() unconditionally through heartbeat_led_write(), from the LED brightness persistence commit earlier today. Verified building for heltec_t096/nrf52840 (shared PWM LED), heltec_wireless_tracker_v2/esp32s3 (shared PWM LED), and rak3401_1watt (two independent PWM LEDs, confirms ZEPHCORE_LED_PIN_SHARED stays 0 there).
heltec_wifi_lora32_v43_procpu.dts already declared a pwm-leds node for its single onboard LED (pwm_led_white, LEDC0/GPIO35, generic pwm-led0 alias) -- it just never had the two project-specific aliases (heartbeat-pwm-led / lora-tx-pwm-led) that helpers/ui/ui_common.c and adapters/board/ZephyrBoard.cpp actually look for. T096 and Wireless Tracker V2 have had this wired for a while; V4.3 was simply never included in that earlier rollout, not blocked by anything in its hardware -- the PWM path was sitting there unused this whole time, confirmed by grepping for any other consumer of pwm-led0/pwm_led_white before touching it (none). Aliased both roles to the same node, same as T096/WT2: this board has only one physical LED, shared between heartbeat and LoRa TX activity. ZEPHCORE_LED_PIN_SHARED and the RX-pulse/pin-hold arbitration (extended to PWM boards earlier today, commit a57d4c7) apply automatically since both aliases point at the same node. Gives V4.3 the full leds.radio/leds.hb/leds.brightness set for the first time: leds.radio previously had no effect at all on this board (no activity LED aliased whatsoever), leds.brightness was accepted but inert (no PWM-capable LED reachable). Verified building for heltec_wifi_lora32_v43/esp32s3 with boards/common/esp32s3_pwm_led.conf (same flag already used for WT2).
ZEPHCORE_LED_DEFAULT_BRIGHTNESS_PCT was 10 on the branch this was developed on (a local preference for that project's own compiles). For general use it should default to what every PWM LED already ran at before this setting existed: full brightness. Also fixes a stale led_gate.h comment that still said "RAM-only, not persisted" after brightness became a persisted NodePrefs field.
Long press on the LEDs page now enters a 3-level menu instead of toggling on/off directly: row select (4 rows: on/off + three brightness step sizes, +/-20%/+/-5%/+/-1%), then value edit for whichever brightness row was picked. Tap = next row / +step, 2 taps = back one level / -step, long press = select / confirm. Brightness changes apply live via the existing lock-free zephcore_led_set_brightness_pct() and persist through a new deferred action (UI_ACTION_LED_BRIGHTNESS_SAVE / mesh_set_led_brightness_pct), mirroring the existing leds_disabled toggle's pattern rather than the unrelated display-backlight UI_ACTION_BRIGHTNESS_SAVE. Compile-checked on T096, Wireless Tracker V2 and V4.3 (all three share this button-UI code; the joystick UI is untouched).
Keep the on/off toggle as row 0 of the row-select list (long press toggles, stays at level 1) rather than splitting it into a read-only header -- confirmed against real hardware that losing cursor access to it was a regression from the first iteration. Also: "LEDs" plural everywhere (matches the "set/get leds" CLI regardless of how many physical LEDs a board has), ON/OFF uppercase, "Bright" abbreviation with 2-digit zero-padded steps for column alignment on the row list, full "Brightness" on the plain LEDs page (shows the live percentage there too), and on the color-panel boards (T096, WT2) the on/off status text/badge use UI_COLOR_OK (cyan on T096)/UI_COLOR_ERROR (red) instead of the plain selection highlight. Iterated live against T096 and V4.3 screens over several flash/test cycles.
Revert the earlier UI_COLOR_ERROR (red) choice for "off" on the LEDs page. Every other on/off page in this UI (BLE, GPS, offgrid...) uses UI_COLOR_DISABLED per liquidraver's own convention (see af96039); red stays reserved for actual critical thresholds (low battery, RX error count). UI_COLOR_OK is untouched either way -- still cyan on T096 (local, officialized 29/08), still green everywhere else including WT2 (liquidraver default, bd35d9d8).
- Row 0 (LEDs: ON/OFF) on color panels now follows the same selection-based color rule as every other row (green only means "cursor is here"), dropping the earlier always-on status-color special case that could be read as the cursor sitting there when it wasn't. - Level 1 and level 2 anchor flush at the top (manual y, no centered_row()), matching every other page's layout instead of visually starting one line down. - Level 2 hints reformatted to "1x TAP : +" / "2x TAPs : -" / "Hold : OK" (aligned colons). Dropped the blank line between title and hints tried in the previous iteration -- it would not fit V4.3's shorter mono screen without further changes there. - Color panels (T096, WT2) show "Bright." instead of the full "Brightness" tried previously; V4.3's mono "Bright" is unchanged. Iterated live against WT2 hardware.
Bilingual (English then French) walkthrough of the 3-level on-screen LED menu, screenshots from Wireless Tracker V2, same code path as T096/V4.3. For testers following the linked PRs.
Committed one level too deep in the previous commit (zephcore/docs/ instead of docs/, this repo's actual root-level documentation folder alongside ADAPTIVE_CAD.md, ARCHITECTURE.md, etc).
bisbille
force-pushed
the
pr-led-brightness-pwm
branch
from
September 28, 2026 20:11
5261aab to
268bd01
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on liquidraver#79: base branch here is
pr-led-brightness-pwm, notmaster,so this diff only shows the 4 commits below. Rebase onto
masteronce liquidraver#79merges (or retarget this PR at that point).
What
An on-screen settings menu for LED brightness on button-UI boards (single
button, no joystick): T096, Wireless Tracker V2, V4.3. Long press on the
LEDs page now opens a 3-level menu instead of only toggling on/off.
current brightness. 1 tap / 2 taps cycle pages as usual; long press
enters the menu.
(loops), 2 taps exits back to level 1, long press either toggles on/off
(row 0) or enters level 3 for the selected brightness row.
0-100%), long press confirms and returns to level 2.
Brightness applies live on every tap (same lock-free path as the
leds.brightnessCLI command) and persists through the existing deferredsave mechanism, mirroring how
leds_disabledalready does it.On color-panel boards (T096, WT2) the on/off status uses the same
selection-based highlight as every other row, and brightness rows read
"Bright +/-N% NN%"; on the V4.3's smaller mono OLED the wording and
line budget are unchanged from that constraint.
Testing
Compiled clean on
heltec_t096/nrf52840,heltec_wireless_tracker_v2/esp32s3,and
heltec_wifi_lora32_v43/esp32s3.Iterated live against real hardware over several flash/test cycles on T096
and Wireless Tracker V2 (screenshot above is from the Tracker V2); V4.3 is
compile-verified with the same code path (mono OLED, smaller screen) but
not separately re-flashed after the latest wording pass.
Not included
A one-line fix for a separate, unrelated bug found while testing this menu
(the shutdown/DFU/offgrid double-press confirmation window is too short on
single-button boards using a longpress input filter) will be proposed as
its own PR rather than bundled in here.