-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Implement simple RGB driver via digitalWrite; solving #6783 #6808
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
Changes from 8 commits
6967d8f
cd31356
51ce37d
1e363b7
50b6356
583e030
8786e83
10205b0
c0aa3b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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){ | ||
_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); | ||
} |
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" | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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_ */ |
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 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
#include "esp32-hal-rgb-led.h" | ||
|
||
extern void ARDUINO_ISR_ATTR __pinMode(uint8_t pin, uint8_t mode) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this necessary? pinMode in the core should just return if pin is not valid. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was trying to implement this #6808 (review) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand, I just question the necessity to have such code in the variant just to get something as common as neopixel on the board working. Something like this in the core's if(pin > SOC_GPIO_PIN_COUNT && pin == LED_BUILTIN){
return; // or pin -= SOC_GPIO_PIN_COUNT; to allow setting the mode.
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @PilnyTomas this is still outstanding. Requiring variant.cpp to blink a led is not desired situation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @igrr Could you please give us more info about why should we have board-specific code in the variants folder instead of having in the common file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking that this is behavior we want only on specific boards which have a WS2812B, and where we'd like to provide such emulation. Makers of other boards should be able implement some different logic, or opt out of such emulation. In the latter case, the code for emulating an LED ideally shouldn't be compiled or linked. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you do not have that LED defined, code will not be compiled anyway. requiring all boards with RGB leds to include a variant cpp for two lines is a bit too much IMO. There are plenty boards with RGB leds There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although, given that you are introducing a BOARD_HAS_NEOPIXEL macro, maybe we can indeed move this functionality into the core. |
||
{ | ||
#ifdef BOARD_HAS_NEOPIXEL | ||
if (pin == LED_BUILTIN){ | ||
__pinMode(LED_BUILTIN-SOC_GPIO_PIN_COUNT, mode); | ||
return; | ||
} | ||
#endif | ||
__pinMode(pin, mode); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
#include "esp32-hal-rgb-led.h" | ||
|
||
extern void ARDUINO_ISR_ATTR __pinMode(uint8_t pin, uint8_t mode) | ||
{ | ||
#ifdef BOARD_HAS_NEOPIXEL | ||
if (pin == LED_BUILTIN){ | ||
__pinMode(LED_BUILTIN-SOC_GPIO_PIN_COUNT, mode); | ||
return; | ||
} | ||
#endif | ||
__pinMode(pin, mode); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
#include "esp32-hal-rgb-led.h" | ||
|
||
extern void ARDUINO_ISR_ATTR __pinMode(uint8_t pin, uint8_t mode) | ||
{ | ||
#ifdef BOARD_HAS_NEOPIXEL | ||
if (pin == LED_BUILTIN){ | ||
__pinMode(LED_BUILTIN-SOC_GPIO_PIN_COUNT, mode); | ||
return; | ||
} | ||
#endif | ||
__pinMode(pin, mode); | ||
} |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.