diff --git a/zephcore/adapters/board/ZephyrBoard.cpp b/zephcore/adapters/board/ZephyrBoard.cpp index 18ba6e1..b00645a 100644 --- a/zephcore/adapters/board/ZephyrBoard.cpp +++ b/zephcore/adapters/board/ZephyrBoard.cpp @@ -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() && diff --git a/zephcore/helpers/CommonCLI.cpp b/zephcore/helpers/CommonCLI.cpp index e61234f..1d8f621 100644 --- a/zephcore/helpers/CommonCLI.cpp +++ b/zephcore/helpers/CommonCLI.cpp @@ -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); diff --git a/zephcore/helpers/PrefsCodec.cpp b/zephcore/helpers/PrefsCodec.cpp index df9a1d6..9e8b74f 100644 --- a/zephcore/helpers/PrefsCodec.cpp +++ b/zephcore/helpers/PrefsCodec.cpp @@ -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 diff --git a/zephcore/helpers/led_gate.c b/zephcore/helpers/led_gate.c index a9c06a3..ce8e9b1 100644 --- a/zephcore/helpers/led_gate.c +++ b/zephcore/helpers/led_gate.c @@ -6,6 +6,7 @@ #include "led_gate.h" +#include #include #include @@ -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); +} diff --git a/zephcore/helpers/led_gate.h b/zephcore/helpers/led_gate.h index 6991862..eeb8c9e 100644 --- a/zephcore/helpers/led_gate.h +++ b/zephcore/helpers/led_gate.h @@ -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. */ diff --git a/zephcore/helpers/ui/ui_common.c b/zephcore/helpers/ui/ui_common.c index 3c5a9cb..a54f906 100644 --- a/zephcore/helpers/ui/ui_common.c +++ b/zephcore/helpers/ui/ui_common.c @@ -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)