led: one shared PWM write helper, shorter comments

The heartbeat and activity LED each computed the PWM pulse inline.
zephcore_led_pwm_write() in led_gate.c now does it once.

Comments added for the LED work described branch history, dates and
release notes; keep only what the code needs. led_gate.c still said the
brightness was RAM-only, which is no longer true, and the server decode
comment gave the old 10% default.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
ZephCore T096
2026-10-08 18:27:01 +02:00
co-authored by Claude Sonnet 5
parent 5425ce83aa
commit fc0723a8ec
6 changed files with 28 additions and 79 deletions
+8 -35
View File
@@ -58,21 +58,12 @@
#endif
#endif
/* LoRa radio activity LED (optional — defined per-board via DT alias). Still
* called tx_led after "set leds.radio" gave it an RX mode, because the DT
* alias it comes from is named lora-tx-led on every board that has one.
*
* PWM path (DT_ALIAS(lora_tx_pwm_led)) takes priority when it exists — used
* on boards where the TX LED is the same physical LED as the heartbeat
* (e.g. T096, see helpers/ui/ui_common.c), so both share the one brightness
* in helpers/led_gate.h instead of the plain GPIO path fighting over the
* pin. */
/* LoRa activity LED, optional per board. The PWM alias wins over the GPIO one. */
#if DT_NODE_EXISTS(DT_ALIAS(lora_tx_pwm_led))
static const struct pwm_dt_spec tx_led_pwm = PWM_DT_SPEC_GET(DT_ALIAS(lora_tx_pwm_led));
#define HAS_TX_LED_PWM 1
#define HAS_TX_LED 0
/* Init deferred to the combined tx_led_init() below (with the GPIO path),
* since both now also need to arm s_rx_pulse_off -- see that block. */
/* Initialised in tx_led_init() below, with the GPIO path. */
#elif DT_NODE_EXISTS(DT_ALIAS(lora_tx_led))
static const struct gpio_dt_spec tx_led =
GPIO_DT_SPEC_GET(DT_ALIAS(lora_tx_led), gpios);
@@ -83,12 +74,8 @@ static const struct gpio_dt_spec tx_led =
#define HAS_TX_LED 0
#endif
/* True when this board wires the activity LED to the same pin as the heartbeat
* (8 of the supported boards do, counting only the plain-GPIO ones -- see the
* PWM branch below for the rest). Only those pay for the arbitration hold in
* led_gate.c — everywhere else the two LEDs are independent and the calls
* compile out. led1 is checked as well because ui_common.c falls back to it
* when a board has no led0. */
/* Heartbeat and activity LED on one pin: only then is the arbitration hold in
* led_gate.c needed. led1 counts too, since ui_common.c falls back to it. */
#if HAS_TX_LED && DT_NODE_EXISTS(DT_ALIAS(led0)) && \
DT_SAME_NODE(DT_ALIAS(led0), DT_ALIAS(lora_tx_led))
#define ZEPHCORE_LED_PIN_SHARED 1
@@ -96,10 +83,7 @@ static const struct gpio_dt_spec tx_led =
DT_NODE_EXISTS(DT_ALIAS(led1)) && \
DT_SAME_NODE(DT_ALIAS(led1), DT_ALIAS(lora_tx_led))
#define ZEPHCORE_LED_PIN_SHARED 1
/* PWM equivalent: true when heartbeat and TX activity are aliased to the same
* PWM LED node (T096, Wireless Tracker V2). Not the case on every PWM board --
* RAK3401 has two independent PWM LEDs (green/blue), so this stays 0 there and
* the hold below is correctly never taken. */
/* Same for one PWM LED node shared by both (T096, Wireless Tracker V2). */
#elif HAS_TX_LED_PWM && DT_NODE_EXISTS(DT_ALIAS(heartbeat_pwm_led)) && \
DT_SAME_NODE(DT_ALIAS(heartbeat_pwm_led), DT_ALIAS(lora_tx_pwm_led))
#define ZEPHCORE_LED_PIN_SHARED 1
@@ -107,17 +91,10 @@ static const struct gpio_dt_spec tx_led =
#define ZEPHCORE_LED_PIN_SHARED 0
#endif
/* PWM-or-GPIO write for the activity LED, mirroring heartbeat_led_write() in
* helpers/ui/ui_common.c -- same reasoning: the write itself must not care
* which path is compiled in, only the callers (onBeforeTransmit/
* onAfterTransmit/onPacketReceived/rx_pulse_off_handler below) do. */
#if HAS_TX_LED_PWM
static inline void tx_led_write(bool on)
{
uint32_t pulse = on ? (uint32_t)((uint64_t)tx_led_pwm.period *
zephcore_led_brightness_pct() / 100)
: 0;
pwm_set_pulse_dt(&tx_led_pwm, pulse);
zephcore_led_pwm_write(&tx_led_pwm, on);
}
#elif HAS_TX_LED
static inline void tx_led_write(bool on)
@@ -195,9 +172,7 @@ static const struct device *const pmic_charger_dev =
* interlock that keeps an RX one-shot from clearing a pin that TX still owns:
* the two are driven from different threads (radio TX path vs system work
* queue), so without it a pulse landing mid-transmit would blank the LED for
* the rest of the packet. Applies equally to the PWM-shared boards (T096,
* Wireless Tracker V2): same race, same fix, just written through
* tx_led_write() instead of a bare gpio_pin_set_dt(). */
* the rest of the packet. */
static atomic_t s_tx_lit;
static struct k_work_delayable s_rx_pulse_off;
@@ -418,9 +393,7 @@ void ZephyrBoard::onBeforeTransmit()
* ("set leds.radio"). On a headless repeater this is the only LED that ever
* lights, so both have to be checked here and not just in the UI layer.
* onAfterTransmit() still clears the pin unconditionally, so a gate or mode
* flipped mid-transmit can't strand it lit. Same interlock on PWM-shared
* boards (T096, Wireless Tracker V2) as on plain-GPIO ones -- only the
* final write (tx_led_write()) differs. */
* flipped mid-transmit can't strand it lit. */
#if HAS_TX_LED_PWM || HAS_TX_LED
uint8_t mode = zephcore_leds_radio_mode();
if (!zephcore_leds_disabled() &&
-15
View File
@@ -993,12 +993,6 @@ void CommonCLI::handleGetCmd(uint32_t sender_timestamp, const char* command, cha
cliModeName(LEDS_HB_NAMES, 4, _prefs->leds_hb_mode),
CLI_HAS_HB_LED ? "" : " (no heartbeat LED on this board)");
} else if (memcmp(config, "leds.brightness", 15) == 0) {
/* Separate from "leds" on purpose: keeps the on/off switch a
* pure word command and the dimmer a pure number command,
* instead of one command parsing both — see led_gate.h. Read
* from _prefs, same as leds.radio/leds.hb above, not from the
* RAM getter directly -- the two are always in sync, the "set"
* handler below writes both together. */
snprintf(reply, CLI_REPLY_SIZE, "> %u%%", (unsigned)_prefs->led_brightness);
} else if (memcmp(config, "leds", 4) == 0) {
snprintf(reply, CLI_REPLY_SIZE, "> %s", _prefs->leds_disabled ? "off" : "on");
@@ -1272,15 +1266,6 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, const char* command, cha
CLI_HAS_HB_LED ? "" : " (no heartbeat LED on this board)");
}
} else if (memcmp(config, "leds.brightness ", 16) == 0) {
/* Separate from "leds" on purpose: keeps the on/off switch a pure
* word command and the dimmer a pure number command, instead of
* one command parsing both. Persisted (since 1.17.4): a fresh
* node still starts at ZEPHCORE_LED_DEFAULT_BRIGHTNESS_PCT (the
* append-only prefs format leaves this byte at the
* initNodePrefs() default on any file that predates this field),
* but once set it survives reboots and firmware updates like
* leds.radio/leds.hb above. Checked before "leds " below so this
* longer, more specific prefix is never shadowed by it. */
const char *val = &config[16];
char *endptr = (char *)val;
long pct = strtol(val, &endptr, 10);
+2 -5
View File
@@ -283,7 +283,7 @@ size_t serverPrefsEncode(const NodePrefs &prefs, uint8_t *buf, size_t cap)
/* LED activity/heartbeat modes (offsets 309-310) */
w.put(&prefs.leds_radio_mode, sizeof(prefs.leds_radio_mode));
w.put(&prefs.leds_hb_mode, sizeof(prefs.leds_hb_mode));
/* led_brightness (ZephCore extension, since 1.17.4), offset 311. */
/* led_brightness at offset 311. */
w.put(&prefs.led_brightness, sizeof(prefs.led_brightness));
return w.off;
}
@@ -360,10 +360,7 @@ void serverPrefsDecode(NodePrefs &prefs, const uint8_t *buf, size_t len)
r.get(&prefs.tz_offset, sizeof(prefs.tz_offset));
r.get(&prefs.leds_radio_mode, sizeof(prefs.leds_radio_mode));
r.get(&prefs.leds_hb_mode, sizeof(prefs.leds_hb_mode));
/* led_brightness (ZephCore extension, since 1.17.4). Absent in files
* written before this field existed; the no-op EOF read leaves the
* initNodePrefs() default ZEPHCORE_LED_DEFAULT_BRIGHTNESS_PCT (10%),
* same value a fresh node already showed before this was persisted. */
/* led_brightness at offset 311: a shorter file leaves the initNodePrefs() default. */
r.get(&prefs.led_brightness, sizeof(prefs.led_brightness));
/* Only the explicit "off" magic disables LEDs; a legacy AGC interval or an
+9 -3
View File
@@ -6,6 +6,7 @@
#include "led_gate.h"
#include <zephyr/drivers/pwm.h>
#include <zephyr/kernel.h>
#include <zephyr/sys/atomic.h>
@@ -75,9 +76,6 @@ void zephcore_led_radio_hold_pin(bool held)
atomic_set(&s_radio_holds_pin, held ? 1 : 0);
}
/* Same cross-thread readers/writers as the gate above (heartbeat work queue,
* TX path, CLI) — atomic for the same reason. RAM-only on purpose: no
* savePrefs() call anywhere near this, see led_gate.h. */
static atomic_t s_led_brightness_pct = ATOMIC_INIT(ZEPHCORE_LED_DEFAULT_BRIGHTNESS_PCT);
uint8_t zephcore_led_brightness_pct(void)
@@ -92,3 +90,11 @@ void zephcore_led_set_brightness_pct(uint8_t pct)
}
atomic_set(&s_led_brightness_pct, pct);
}
void zephcore_led_pwm_write(const struct pwm_dt_spec *led, bool on)
{
uint32_t pulse = on ? (uint32_t)((uint64_t)led->period *
zephcore_led_brightness_pct() / 100)
: 0;
pwm_set_pulse_dt(led, pulse);
}
+7 -8
View File
@@ -34,19 +34,18 @@ bool zephcore_leds_disabled(void);
* progress. Safe to call from any role, with or without a UI. */
void zephcore_leds_set_disabled(bool disabled);
/* Shared PWM brightness (0-100) for boards whose heartbeat/TX LED is
* PWM-capable (DT_ALIAS(heartbeat_pwm_led) / DT_ALIAS(lora_tx_pwm_led)).
* Persisted via NodePrefs (prefs.led_brightness); ZEPHCORE_LED_DEFAULT_BRIGHTNESS_PCT
* only seeds the RAM cache on a fresh node with no saved value yet (100%,
* matching the brightness every PWM LED ran at before this setting existed).
* Independent of the on/off gate above: brightness only matters while the
* gate is open. No-op on boards without a PWM-capable LED (the getter still
* returns the compiled default, simply unused). */
/* Shared PWM brightness (0-100) for PWM-capable heartbeat/TX LEDs. Persisted
* as prefs.led_brightness; the default only applies to a node with no saved
* value. Independent of the on/off gate above. */
#define ZEPHCORE_LED_DEFAULT_BRIGHTNESS_PCT 100
uint8_t zephcore_led_brightness_pct(void);
void zephcore_led_set_brightness_pct(uint8_t pct);
struct pwm_dt_spec;
/* Drive a PWM LED at the shared brightness when on, dark when off. */
void zephcore_led_pwm_write(const struct pwm_dt_spec *led, bool on);
/* Called by zephcore_leds_set_disabled() after the flag changes. Weak no-op in
* led_gate.c; helpers/ui/ui_common.c overrides it to stop/restart the heartbeat
* cycle and refresh the UI's LED page. Not meant to be called directly. */
+2 -13
View File
@@ -77,12 +77,7 @@ void ui_play_startup_chime(void)
* messages; with both LEDs present, companions light led1 for unread.
*/
/* PWM path (brightness-capable, shared with the LoRa TX LED via the same
* DT_ALIAS(heartbeat_pwm_led)/DT_ALIAS(lora_tx_pwm_led) pair when a board's
* heartbeat and TX indicator are the same physical LED, e.g. T096) takes
* priority over the plain digital led0/led1 fallback. Brightness comes from
* helpers/led_gate.h — one dimmer for every event on this LED, not a
* heartbeat-only one. */
/* PWM takes priority over the plain led0/led1 fallback. */
#if DT_NODE_EXISTS(DT_ALIAS(heartbeat_pwm_led))
static const struct pwm_dt_spec s_heartbeat_led_pwm =
PWM_DT_SPEC_GET(DT_ALIAS(heartbeat_pwm_led));
@@ -103,9 +98,6 @@ static const struct gpio_dt_spec s_heartbeat_led =
#define HAS_HEARTBEAT_LED 0
#endif
/* "Some heartbeat LED exists, PWM or plain GPIO" — used where the on/off
* write differs but everything else (work queue plumbing, readiness check)
* is identical either way. */
#define HAS_ANY_HEARTBEAT_LED (HAS_HEARTBEAT_LED_PWM || HAS_HEARTBEAT_LED)
#if HAS_HEARTBEAT_LED_PWM
@@ -115,10 +107,7 @@ static inline bool heartbeat_led_is_ready(void)
}
static inline void heartbeat_led_write(bool on)
{
uint32_t pulse = on ? (uint32_t)((uint64_t)s_heartbeat_led_pwm.period *
zephcore_led_brightness_pct() / 100)
: 0;
pwm_set_pulse_dt(&s_heartbeat_led_pwm, pulse);
zephcore_led_pwm_write(&s_heartbeat_led_pwm, on);
}
#elif HAS_HEARTBEAT_LED
static inline bool heartbeat_led_is_ready(void)