Skip to content

Conversation

doedel123
Copy link

Adds WS2812 NeoPixel LEDHandler for Freenove ESP32-S3 boards, updates README with instructions

My first commit and PR on github, please check if I did everything right.

Thank you very much


This firmware includes support for a single WS2812 (NeoPixel) LED as a system status indicator.
It has been tested on the **Freenove ESP32-S3 Audio Kit**, where the LED is connected to **GPIO48**.

Copy link
Owner

@akdeb akdeb Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add a comment here saying that this repo is compatible with both a common-anode 3 GPIO pin RGB LED and a WS2812 LED. What do you think?

The regular LED is supported in files RGBHandler.cpp and RGBHandler.h and WS2812 in WS2812Handler.cpp and WS2812Handler.h.

And we can use the existing LEDHandler.cpp and LEDHandler.h for routing to the above RGB vs WS2812 LEDs.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add this -->

This repo supports both WS2812 (1-pin) and Common Anode RGB (3-pin) LEDs.

  • To use a 3-pin RGB LED, define the macro USE_3PIN_LED in LEDHandler.h.
  • If the macro is left commented out, the code will default to using a WS2812 LED.

Files:

  • 3-Pin RGB support: RGBLEDHandler.cpp/.h
  • WS2812 support: WS2812Handler.cpp/.h
  • Shared wrapper: LEDHandler.cpp/.h (calls the correct backend automatically)

```ini
lib_deps =
adafruit/Adafruit NeoPixel @ ^1.12.0

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file looks good to me otherwise 👍

Copy link
Owner

@akdeb akdeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@doedel123 This looks great and it's almost ready to merge. The highlights are:

  1. We should give users a way to run a 3-pin RGB LED and a 1-pin WS2812 LED. So we can use LEDHandler.cpp/h to route to RGBLEDHandler.cpp/h or WS2812Handler.cpp/h (added code examples in the comments)
  2. Minor nits about keeping comments language same.
  3. We should remove the .env file but keep the .env.example

Let me know when you've tested these changes on your local setup and we should be ready to merge this PR to Elato!

@@ -1,313 +1,90 @@
#include "LEDHandler.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking is we should create new files here to support both the 1 GPIO pin WS2812 and a common-anode 3 GPIO pin RGB LED.

Steps:

  1. Define a macro at the top of LEDHandler.h:
// Uncomment to use standard 3-PIN RGB LED (common anode)
// #define USE_3PIN_LED
  1. Inside LEDHandler.h, we can route function calls based on the macro:
#ifdef USE_3PIN_LED
#include "RGBLEDHandler.h"
#else
#include "WS2812Handler.h"
#endif
  1. Our function declarations in LEDHandler.h stay the same:
void setLEDColor(uint8_t r, uint8_t g, uint8_t b);
void turnOffLED();
void turnOnLED();
void setupRGBLED();
void ledTask(void *parameter);
void setStaticColor(StaticColor color);
  1. In LEDHandler.cpp, we can forward calls to the appropriate implementation:
#include "LEDHandler.h"

#ifdef USE_3PIN_LED
    #include "RGBLEDHandler.h"
    namespace LED = RGB;
#else
    #include "WS2812Handler.h"
    namespace LED = WS2812;
#endif

void setLEDColor(uint8_t r, uint8_t g, uint8_t b) {
    LED::setLEDColor(r, g, b);
}

void turnOffLED() {
    LED::turnOffLED();
}

void turnOnLED() {
    LED::turnOnLED();
}

void setupRGBLED() {
    LED::setupRGBLED();
}

void ledTask(void *parameter) {
    LED::ledTask(parameter);
}

void setStaticColor(StaticColor color) {
    LED::setStaticColor(color);
}
  1. In RGBLEDHandler.h we have
#ifndef RGBLEDHANDLER_H
#define RGBLEDHANDLER_H

#include <stdint.h>
#include "Config.h"

namespace RGB {
    void setupRGBLED();
    void setLEDColor(uint8_t r, uint8_t g, uint8_t b);
    void turnOffLED();
    void turnOnLED();
    void ledTask(void *parameter);
    void setStaticColor(StaticColor color);
}

#endif
  1. In WS2812Handler.h we have
#ifndef WS2812HANDLER_H
#define WS2812HANDLER_H

#include <stdint.h>
#include "Config.h"

namespace WS2812 {
    void setupRGBLED();
    void setLEDColor(uint8_t r, uint8_t g, uint8_t b);
    void turnOffLED();
    void turnOnLED();
    void ledTask(void *parameter);
    void setStaticColor(StaticColor color);
}

#endif

#include "Config.h"
#include <stdint.h>

// Grundfarben per Name
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind converting the inline comments in the files to English?

Since this repo is intended for open-source use worldwide, keeping everything in one language helps ensure accessibility and readability for all contributors and future devs and users across different countries.

No need to change anything else here, just the comments will do. The variable names are clear to me. Appreciate it!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I will translate the comments to EN today :)

@@ -0,0 +1 @@

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this file

@@ -1,22 +0,0 @@
# If you want to skip device registration, set this to True. Set to False in production.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, lets keep this file as it serves as an example for what ENV variables people will need when they run the UI / server locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants