Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement simple RGB driver via digitalWrite; solving #6783 #6808

Merged
merged 9 commits into from
Jun 24, 2022
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ set(CORE_SRCS
cores/esp32/esp32-hal-matrix.c
cores/esp32/esp32-hal-misc.c
cores/esp32/esp32-hal-psram.c
cores/esp32/esp32-hal-rgb-led.c
cores/esp32/esp32-hal-sigmadelta.c
cores/esp32/esp32-hal-spi.c
cores/esp32/esp32-hal-time.c
Expand Down
15 changes: 15 additions & 0 deletions cores/esp32/esp32-hal-gpio.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,13 @@ static InterruptHandle_t __pinInterruptHandlers[SOC_GPIO_PIN_COUNT] = {0,};

extern void ARDUINO_ISR_ATTR __pinMode(uint8_t pin, uint8_t mode)
{
#ifdef BOARD_HAS_NEOPIXEL
Copy link
Member

Choose a reason for hiding this comment

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

Could we consider moving the board-specific code to the variants folder?

I'm worried that the more similar features we add for specific boards, the more complex the core code will become. It would be better to handle this in the board variant folder. It would also demonstrate authors of 3rd party board packages how to add such custom functionality.

To avoid duplication of code, you can still define RGBLedWrite somewhere in the core, but preferably in a separate source file (esp32-hal-rgb-led.c?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm a little bit confused about what should go where.
pinMode -> variants/ ... new file?
RGBLedWrite -> esp32-hal-rgb-led.c ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be solved in 583e030

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

if (!GPIO_IS_VALID_GPIO(pin)) {
log_e("Invalid pin selected");
return;
Expand Down Expand Up @@ -127,6 +134,14 @@ extern void ARDUINO_ISR_ATTR __pinMode(uint8_t pin, uint8_t mode)

extern void ARDUINO_ISR_ATTR __digitalWrite(uint8_t pin, uint8_t val)
{
#ifdef BOARD_HAS_NEOPIXEL
if(pin == LED_BUILTIN){
//use RMT to set all channels on/off
const uint8_t comm_val = val != 0 ? LED_BRIGHTNESS : 0;
neopixelWrite(LED_BUILTIN, comm_val, comm_val, comm_val);
return;
}
#endif
gpio_set_level((gpio_num_t)pin, val);
}

Expand Down
2 changes: 2 additions & 0 deletions cores/esp32/esp32-hal-gpio.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ extern "C" {

#include "esp32-hal.h"
#include "soc/soc_caps.h"
#include "pins_arduino.h"

#if (CONFIG_IDF_TARGET_ESP32S2 || CONFIG_IDF_TARGET_ESP32S3)
#define NUM_OUPUT_PINS 46
Expand Down Expand Up @@ -63,6 +64,7 @@ extern "C" {
#define ONLOW_WE 0x0C
#define ONHIGH_WE 0x0D


#define digitalPinIsValid(pin) GPIO_IS_VALID_GPIO(pin)
#define digitalPinCanOutput(pin) GPIO_IS_VALID_OUTPUT_GPIO(pin)

Expand Down
47 changes: 47 additions & 0 deletions cores/esp32/esp32-hal-rgb-led.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#include "esp32-hal-rgb-led.h"


void neopixelWrite(uint8_t pin, uint8_t red_val, uint8_t green_val, uint8_t blue_val){
rmt_data_t led_data[24];
static rmt_obj_t* rmt_send = NULL;
static bool initialized = false;

uint8_t _pin = pin;
#ifdef BOARD_HAS_NEOPIXEL
if(pin == LED_BUILTIN){
Copy link
Member

Choose a reason for hiding this comment

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

this should be evaluated only if the board has neopixel, else LED_BUILTIN would be lower than SOC_GPIO_PIN_COUNT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The entire function was safeguarded #ifdef BOARD_HAS_NEOPIXEL
But I will remove safeguard for the entire function a wrap only this.

_pin = LED_BUILTIN-SOC_GPIO_PIN_COUNT;
}
#endif

if(!initialized){
if((rmt_send = rmtInit(_pin, RMT_TX_MODE, RMT_MEM_64)) == NULL){
log_e("RGB LED driver initialization failed!");
rmt_send = NULL;
return;
}
rmtSetTick(rmt_send, 100);
initialized = true;
}

int color[] = {green_val, red_val, blue_val}; // Color coding is in order GREEN, RED, BLUE
int i = 0;
for(int col=0; col<3; col++ ){
for(int bit=0; bit<8; bit++){
if((color[col] & (1<<(7-bit)))){
// HIGH bit
led_data[i].level0 = 1; // T1H
led_data[i].duration0 = 8; // 0.8us
led_data[i].level1 = 0; // T1L
led_data[i].duration1 = 4; // 0.4us
}else{
// LOW bit
led_data[i].level0 = 1; // T0H
led_data[i].duration0 = 4; // 0.4us
led_data[i].level1 = 0; // T0L
led_data[i].duration1 = 8; // 0.8us
}
i++;
}
}
rmtWrite(rmt_send, led_data, 24);
}
20 changes: 20 additions & 0 deletions cores/esp32/esp32-hal-rgb-led.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#ifndef MAIN_ESP32_HAL_RGB_LED_H_
#define MAIN_ESP32_HAL_RGB_LED_H_

#ifdef __cplusplus
extern "C" {
#endif

#include "esp32-hal.h"

Copy link
Member

Choose a reason for hiding this comment

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

You should have safeguard against LED_BRIGHTNESS not being defined:

#ifndef LED_BRIGHTNESS
#define LED_BRIGHTNESS 64
#endif

#ifndef LED_BRIGHTNESS
#define LED_BRIGHTNESS 64
#endif

void neopixelWrite(uint8_t pin, uint8_t red_val, uint8_t green_val, uint8_t blue_val);

#ifdef __cplusplus
}
#endif

#endif /* MAIN_ESP32_HAL_RGB_LED_H_ */
1 change: 1 addition & 0 deletions cores/esp32/esp32-hal.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ void yield(void);
#include "esp32-hal-timer.h"
#include "esp32-hal-bt.h"
#include "esp32-hal-psram.h"
#include "esp32-hal-rgb-led.h"
#include "esp32-hal-cpu.h"

void analogWrite(uint8_t pin, int value);
Expand Down
39 changes: 39 additions & 0 deletions libraries/ESP32/examples/GPIO/BlinkRGB/BlinkRGB.ino
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
BlinkRGB

Demonstrates usage of onboard RGB LED on some ESP dev boards.

Calling digitalWrite(LED_BUILTIN, HIGH) will use hidden RGB driver.

RGBLedWrite demonstrates controll of each channel:
void neopixelWrite(uint8_t pin, uint8_t red_val, uint8_t green_val, uint8_t blue_val)

WARNING: After using digitalWrite to drive RGB LED it will be impossible to drive the same pin
with normal HIGH/LOW level
*/
//#define LED_BRIGHTNESS 64 // Change white brightness (max 255)

// the setup function runs once when you press reset or power the board

void setup() {
// No need to initialize the RGB LED
}

// the loop function runs over and over again forever
void loop() {
#ifdef BOARD_HAS_NEOPIXEL
digitalWrite(LED_BUILTIN, HIGH); // Turn the RGB LED white
delay(1000);
digitalWrite(LED_BUILTIN, LOW); // Turn the RGB LED off
delay(1000);

neopixelWrite(LED_BUILTIN,LED_BRIGHTNESS,0,0); // Red
delay(1000);
neopixelWrite(LED_BUILTIN,0,LED_BRIGHTNESS,0); // Green
delay(1000);
neopixelWrite(LED_BUILTIN,0,0,LED_BRIGHTNESS); // Blue
delay(1000);
neopixelWrite(LED_BUILTIN,0,0,0); // Off / black
delay(1000);
#endif
}
7 changes: 7 additions & 0 deletions variants/esp32c3/pins_arduino.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,18 @@
#define Pins_Arduino_h

#include <stdint.h>
#include "soc/soc_caps.h"

#define EXTERNAL_NUM_INTERRUPTS 22
#define NUM_DIGITAL_PINS 22
#define NUM_ANALOG_INPUTS 6

static const uint8_t LED_BUILTIN = SOC_GPIO_PIN_COUNT+8;
#define BUILTIN_LED LED_BUILTIN // backward compatibility
#define LED_BUILTIN LED_BUILTIN
#define BOARD_HAS_NEOPIXEL
#define LED_BRIGHTNESS 64

#define analogInputToDigitalPin(p) (((p)<NUM_ANALOG_INPUTS)?(analogChannelToDigitalPin(p)):-1)
#define digitalPinToInterrupt(p) (((p)<NUM_DIGITAL_PINS)?(p):-1)
#define digitalPinHasPWM(p) (p < EXTERNAL_NUM_INTERRUPTS)
Expand Down
8 changes: 8 additions & 0 deletions variants/esp32s2/pins_arduino.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,19 @@
#define Pins_Arduino_h

#include <stdint.h>
#include "soc/soc_caps.h"

#define EXTERNAL_NUM_INTERRUPTS 46
#define NUM_DIGITAL_PINS 48
#define NUM_ANALOG_INPUTS 20

static const uint8_t LED_BUILTIN = SOC_GPIO_PIN_COUNT+18; // GPIO pin for Saola-1 & DevKitM-1 = 18
//static const uint8_t LED_BUILTIN = SOC_GPIO_PIN_COUNT+45; // GPIO pin for Kaluga = 45
#define BUILTIN_LED LED_BUILTIN // backward compatibility
#define LED_BUILTIN LED_BUILTIN
#define BOARD_HAS_NEOPIXEL
#define LED_BRIGHTNESS 64

#define analogInputToDigitalPin(p) (((p)<20)?(analogChannelToDigitalPin(p)):-1)
#define digitalPinToInterrupt(p) (((p)<48)?(p):-1)
#define digitalPinHasPWM(p) (p < 46)
Expand Down
10 changes: 10 additions & 0 deletions variants/esp32s3/pins_arduino.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define Pins_Arduino_h

#include <stdint.h>
#include "soc/soc_caps.h"

#define USB_VID 0x303a
#define USB_PID 0x1001
Expand All @@ -10,6 +11,15 @@
#define NUM_DIGITAL_PINS 48
#define NUM_ANALOG_INPUTS 20

// Some boards have too low voltage on this pin (board design bug)
// Use different pin with 3V and connect with 48
// and change this setup for the chosen pin (for example 38)
static const uint8_t LED_BUILTIN = SOC_GPIO_PIN_COUNT+48;
#define BUILTIN_LED LED_BUILTIN // backward compatibility
#define LED_BUILTIN LED_BUILTIN
#define BOARD_HAS_NEOPIXEL
#define LED_BRIGHTNESS 64

#define analogInputToDigitalPin(p) (((p)<20)?(analogChannelToDigitalPin(p)):-1)
#define digitalPinToInterrupt(p) (((p)<48)?(p):-1)
#define digitalPinHasPWM(p) (p < 46)
Expand Down