Improve RGB Driver

[santaimpersonator]

Related area

Implement RGB_BUILTIN variable for (#6808)

Hardware specification

Boards with standard LED and RGB LED

Is your feature request related to a problem?

With the implemented solution (#6808) to issue #6783, users are limited to only controlling a regular LED or RGB LED. Boards with both types of LEDs are now constrained to only utilizing one as the built-in LED.

Describe the solution you'd like

Define a new variable RGB_BUILTIN for the new RGB driver feature (#6808)

• Replaces the use of the LED_BUILTIN variable in the implementation
• Removes the need for BOARD_HAS_NEOPIXEL
• Also, rename LED_BRIGHTNESS to match new variable name RGB_BRIGHTNESS

static const uint8_t LED_BUILTIN = SOC_GPIO_PIN_COUNT+2;
#define BUILTIN_LED  LED_BUILTIN // backward compatibility
#define LED_BUILTIN LED_BUILTIN
#define RGB_BUILTIN LED_BUILTIN
#define RGB_BRIGHTNESS 65

This way users that have a regular LED and RGB LED, will be able to utilize the digitalWrite functionality on both LEDs. Also, the Blink.ino and BlinkRGB.ino example sketches could operate on the LEDs separately.

static const uint8_t LED_BUILTIN = 13;
static const uint8_t RGB_BUILTIN = SOC_GPIO_PIN_COUNT+2;
#define BUILTIN_LED  LED_BUILTIN // backward compatibility
#define LED_BUILTIN LED_BUILTIN
#define RGB_BUILTIN RGB_BUILTIN
#define RGB_BRIGHTNESS 65

Affected files/lines of code:

• https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-gpio.c#L94-L99

  #ifdef  RGB_BUILTIN
     if (pin == RGB_BUILTIN){
         __pinMode( RGB_BUILTIN-SOC_GPIO_PIN_COUNT, mode);
         return;
     }
  #endif
  

• https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-gpio.c#L137-L144

  #ifdef RGB_BUILTIN
      if(pin == RGB_BUILTIN){
          //use RMT to set all channels on/off
          const uint8_t comm_val = val != 0 ? RGB_BRIGHTNESS : 0;
          neopixelWrite(RGB_BUILTIN, comm_val, comm_val, comm_val);
          return;
      }
  #endif
  

• https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-rgb-led.c#L10-L14

  #ifdef RGB_BUILTIN
    if(pin == RGB_BUILTIN){
      _pin = RGB_BUILTIN-SOC_GPIO_PIN_COUNT;
    }
  #endif
  

• https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-rgb-led.h#L10-L12

  #ifndef RGB_BRIGHTNESS
    #define RGB_BRIGHTNESS 64
  #endif
  

• https://github.com/espressif/arduino-esp32/blob/master/libraries/ESP32/examples/GPIO/BlinkRGB/BlinkRGB.ino
  • Replace LED_BUILTIN with RGB_BUILTIN
  • Replace LED_BRIGHTNESS with RGB_BRIGHTNESS
• Update pin definitions for the ESP32-S2, ESP32-S3, ESP32-C3
  • Remove BOARD_HAS_NEOPIXEL
  • Add #define RGB_BUILTIN LED_BUILTIN
  • Replace LED_BRIGHTNESS with RGB_BRIGHTNESS

I've tested the proposed changes with the regular Blink.ino and BlinkRGB.ino example sketches, with the following pin definitions:

• Regular and RGB LED:

  static const uint8_t LED_BUILTIN = 13;
  static const uint8_t RGB_BUILTIN = SOC_GPIO_PIN_COUNT+2;
  #define BUILTIN_LED  LED_BUILTIN // backward compatibility
  #define LED_BUILTIN LED_BUILTIN
  #define RGB_BUILTIN RGB_BUILTIN
  #define RGB_BRIGHTNESS 65
  

• Only RGB LED:

  static const uint8_t LED_BUILTIN = SOC_GPIO_PIN_COUNT+2;
  #define BUILTIN_LED  LED_BUILTIN // backward compatibility
  #define LED_BUILTIN LED_BUILTIN
  #define RGB_BUILTIN LED_BUILTIN
  #define RGB_BRIGHTNESS 65
  

• Only regular LED:

  static const uint8_t LED_BUILTIN = 13;
  #define BUILTIN_LED  LED_BUILTIN // backward compatibility
  #define LED_BUILTIN LED_BUILTIN
  

Both sketches operate as expected.

Describe alternatives you've considered

No response

Additional context

I have a pull request ready; however, I thought that I'd start with creating a feature request first.

I have checked existing list of Feature requests and the Contribution Guide

• I confirm I have checked existing list of Feature requests and Contribution Guide.

[JimDrewGH]

Instead of having RGB_BRIGHTNESS, I might suggest using RGB_COLOR (rr, gg, bb). That would give you complete control of the brightness as well as the color.

[santaimpersonator]

Instead of having RGB_BRIGHTNESS, I might suggest using RGB_COLOR (rr, gg, bb). That would give you complete control of the brightness as well as the color.

@JimDrewGH
While I am open to this suggestion, I don't think it is necessary. Users can easily implement their own variable (such as RGB_COLOR) to control the brightness/color in their sketch. I believe the original intention of the brightness variable is in its simplicity:

• Provide a default brightness level for the RGB LED
• Control the default RGB LED brightness for the digitalWrite() function in the Blink.ino built-in example code
• Control the default RGB LED brightness for the digitalWrite() function and demonstrate color control with the neopixelWrite() function in the BlinkRGB.ino built-in example code

Please, feel free to elaborate further on your suggestion.

[SuGlider]

@PilnyTomas - PTAL, thanks.

[JimDrewGH]

I do like the simplicity of the OP's proposal. Maybe to keep it simple you could have some options like RGB_BUILTIN_BLUE that could be used with the simple digitalWrite() function. This way you could at least have a different color other than WHITE. :)

[PilnyTomas]

Hi @JimDrewGH, the color can be controlled by using neopixelWrite found here
As you can see in proposal #6808 I wanted the option for digitalWrite to be able to control separate color channels as if it was discreet LED, however, this idea was abandoned for the sake of simplicity of usage.
@santaimpersonator If you have already the changes and they work, please create a pull request.