ExternalNotification and StatusLED now call AmbientLighting to update…

[jp-bennett]

… RGB LEDs. Add optional heartbeat

[Copilot]

Pull request overview

This PR aims to route RGB LED updates through AmbientLightingThread from ExternalNotificationModule and StatusLEDModule, and adds an optional “heartbeat” behavior.

Changes:

• ExternalNotificationModule now delegates RGB LED updates to AmbientLightingThread (and removes per-hardware direct LED writes).
• StatusLEDModule triggers ambient lighting updates for a heartbeat/battery indicator (guarded by a new macro).
• Legacy RGB LED helper headers (RAKled.h, NeoPixel.h) are effectively removed/emptied while AmbientLightingThread is expanded to own/init LED drivers.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/modules/StatusLEDModule.h	Adds AmbientLightingThread include and declares an extern ambient lighting instance (under RGB_LED_POWER).
src/modules/StatusLEDModule.cpp	Adds heartbeat-driven calls into ambient lighting (under RGB_LED_POWER).
src/modules/ExternalNotificationModule.h	Adds AmbientLightingThread include and declares an extern ambient lighting instance (under HAS_RGB_LED).
src/modules/ExternalNotificationModule.cpp	Removes per-device RGB writes and calls ambient lighting from setExternalState().
src/main.cpp	Removes RAKled.h include.
src/graphics/RAKled.h	Emptied (previous extern declaration removed).
src/graphics/NeoPixel.h	Emptied (previous extern declaration removed).
src/AmbientLightingThread.h	Adds include guards, adds driver instantiation, refactors setLighting(...) to take parameters, and adds friend access for modules.

Comments suppressed due to low confidence (2)

src/modules/ExternalNotificationModule.cpp:127

• The RGB fade state is computed each runOnce() (red/green/blue/white updates), but after removing the per-hardware writes there is no call that applies these updated values to AmbientLighting. This means the animation won’t actually show (LED only updates when setExternalState() is called on output toggles). Apply ambientLightingThread.setLighting(...) after computing the new colors (or move the animation into AmbientLightingThread).

#if defined(HAS_RGB_LED)
            red = (colorState & 4) ? brightnessValues[brightnessIndex] : 0;          // Red enabled on colorState = 4,5,6,7
            green = (colorState & 2) ? brightnessValues[brightnessIndex] : 0;        // Green enabled on colorState = 2,3,6,7
            blue = (colorState & 1) ? (brightnessValues[brightnessIndex] * 1.5) : 0; // Blue enabled on colorState = 1,3,5,7
            white = (colorState & 12) ? brightnessValues[brightnessIndex] : 0;
            if (ascending) { // fade in
                brightnessIndex++;
                if (brightnessIndex == (sizeof(brightnessValues) - 1)) {
                    ascending = false;
                }
            } else {
                brightnessIndex--; // fade out
            }
            if (brightnessIndex == 0) {
                ascending = true;
                colorState++; // next color
                if (colorState > 7) {
                    colorState = 1;
                }
            }
            // we need fast updates for the color change
            delay = EXT_NOTIFICATION_FAST_THREAD_MS;
#endif

src/AmbientLightingThread.h:92

• The preprocessor/brace structure around the _type checks is broken: when both HAS_NCP5623 and HAS_LP5562 are defined, the if (_type == ScanI2C::NCP5623) and if (_type == ScanI2C::LP5562) blocks become improperly nested and only one closing brace is emitted, which will not compile (and also prevents initializing the selected device correctly). Restructure these as independent #ifdef guarded if (...) { ... } blocks with their own braces.

#ifdef HAS_RGB_LED
        LOG_DEBUG("AmbientLighting init");
#ifdef HAS_NCP5623
        if (_type == ScanI2C::NCP5623) {
            rgb.begin();
#endif
#ifdef HAS_LP5562
            if (_type == ScanI2C::LP5562) {
                rgbw.begin();
#endif
#ifdef RGBLED_RED
                pinMode(RGBLED_RED, OUTPUT);
                pinMode(RGBLED_GREEN, OUTPUT);
                pinMode(RGBLED_BLUE, OUTPUT);
#endif
#ifdef HAS_NEOPIXEL
                pixels.begin(); // Initialise the pixel(s)
                pixels.clear(); // Set all pixel colors to 'off'
                pixels.setBrightness(moduleConfig.ambient_lighting.current);
#endif
                if (!moduleConfig.ambient_lighting.led_state) {
                    LOG_DEBUG("AmbientLighting Disable due to moduleConfig.ambient_lighting.led_state OFF");
                    disable();
                    return;
                }
                setLighting(moduleConfig.ambient_lighting.current, moduleConfig.ambient_lighting.red,
                            moduleConfig.ambient_lighting.green, moduleConfig.ambient_lighting.blue);
#endif
#if defined(HAS_NCP5623) || defined(HAS_LP5562)
            }
#endif
        }