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

ESP32: Using RGB_BUILTIN define as pin number in the constructor fail… #361

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KurtE
Copy link
Contributor

@KurtE KurtE commented Jun 29, 2023

…s and resets S2 and S3
I was experimenting with a few different ESP32 boards (C3, S2, S3) and was playing with the on board neopixel and got tired of changing the pin number (C3->8, S2->18, S3->48), and noticed they had a define RGB_BUILTIN and hoped that you could use it in the constructor.

The neopixel failed to work, and on the S2 and S3 it guru-meditated. Found out they set these values as the pin number plus the count of pins...

So in setPin I checked for this and offset back to the actual pin number.

Scope of change:

void Adafruit_NeoPixel::setPin(int16_t p) {
#if defined(ESP32) && defined(RGB_BUILTIN)
if((p == RGB_BUILTIN) && (RGB_BUILTIN > SOC_GPIO_PIN_COUNT)){
p -= SOC_GPIO_PIN_COUNT;
}
#endif

if (begun && (pin >= 0))
pinMode(pin, INPUT); // Disable existing out pin
pin = p;
if (begun) {
pinMode(p, OUTPUT);
digitalWrite(p, LOW);
}
#if defined(AVR)
port = portOutputRegister(digitalPinToPort(p));
pinMask = digitalPinToBitMask(p);
#endif
#if defined(ARDUINO_ARCH_STM32) || defined(ARDUINO_ARCH_ARDUINO_CORE_STM32)
gpioPort = digitalPinToPort(p);
gpioPin = STM_LL_GPIO_PIN(digitalPinToPinName(p));
#endif
}
Added the check for ESP32 and RGB_BUILTIN and the code within it.
So should only potentially impact ESP32 builds. Note: I added the check for
RGB_BUILTIN > SOC_GPIO_PIN_COUNT
On off chance they change hot this works to be the actual pin number. But if they do, they would also need to change:
neopixelWrite(RGB_BUILTIN, r, g, b);

I built and ran the test code on an C3, S2 and an S3 dev board.

Note: Second try - my github project was originally cloned from PaulStoffregen version. So redid it cloned off of this one.

…s and resets S2 and S3

I was experimenting with a few different ESP32 boards (C3, S2, S3) and was playing with the on board neopixel and got tired of changing the pin number (C3->8, S2->18, S3->48), and noticed they had a define RGB_BUILTIN and hoped that you could use it in the constructor.

The neopixel failed to work, and on the S2 and S3 it guru-meditated.  Found out they set these values as the pin number plus the count of pins...

So in setPin I checked for this and offset back to the actual pin number.
@caternuson
Copy link

The neopixel failed to work, and on the S2 and S3 it guru-meditated. Found out they set these values as the pin number plus the count of pins...

That sounds more like an issue with the board definition. Which specific ESP32 board were you using?

@KurtE
Copy link
Contributor Author

KurtE commented Jun 29, 2023

The neopixel failed to work, and on the S2 and S3 it guru-meditated. Found out they set these values as the pin number plus the count of pins...

That sounds more like an issue with the board definition. Which specific ESP32 board were you using?

You would think that (I did), but if you look at Issue up on ESP32 that I commented on:
espressif/arduino-esp32#7767 (comment)
Or the thread on their forum:
https://esp32.com/viewtopic.php?f=19&t=34343&sid=b4bbeea4f6c9f645ee4ed222da6cd7ab

Their current setup/defines for this as well as LED_BUILTIN is the actual pin number Plus the count of pins...
Their internal functions like neopixelWrite, look for this and subtract off the count of pins.

So far I have done it for the first three dev modules in their list of ESP32's
image

@caternuson
Copy link

That's a really weird pin numbering scheme. Not sure it's worth adding logic in this library to automagically deal with that. Best that translation be done in user code. Can use the suggest code snippet here:

uint8_t RBG_pin = RGB_BUILTIN - SOC_GPIO_PIN_COUNT;

which will still let the pin number be set automatically.

I guess ESP's RGB_BUILTIN is for use with their native neopixelWrite() which uses that odd pin numbering scheme.

For this library, it's just the pin number. The Adafruit boards have a different predefine for the NeoPixel - PIN_NEOPIXEL. This examples works on a QT Py ESP32:

#include <Adafruit_NeoPixel.h>

Adafruit_NeoPixel strip(1, PIN_NEOPIXEL);

void setup() {
  strip.begin();
}

void loop() {
  strip.fill(0xFF0000); strip.show();
  delay(100);
  strip.fill(0x00FF00); strip.show();
  delay(100);
  strip.fill(0x0000FF); strip.show();
  delay(100);
}

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