Skip to content

AudioTools.h incorrectly defines LED_BUILTIN #680

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

Closed
LenStruttmann opened this issue Mar 14, 2023 · 5 comments
Closed

AudioTools.h incorrectly defines LED_BUILTIN #680

LenStruttmann opened this issue Mar 14, 2023 · 5 comments

Comments

@LenStruttmann
Copy link

In AudioTools.h, the symbol LED_BUILTIN is defined in lines 220-223 and 282-285. This causes an issue when compiling for the board "lolin_s2_mini" using platformIO.

Is it really appropriate for AudioTools.h to even be defining that symbol? Should not it rely on the default definition provided in pins_arduino.h?

Thanks,

Len

@LenStruttmann LenStruttmann changed the title AudioTools.h incorrectly defines AudioTools.h incorrectly defines LED_BUILTIN Mar 14, 2023
@pschatzmann
Copy link
Owner

This definition is only used when LED_BUILTIN is not defined!
So I don't understand why this would be an issue: please explain.

@LenStruttmann
Copy link
Author

  1. When I #include "AudioTools.h", as is, LED_BUILTIN gets incorrectly defined as 13. VisualStudio identifies the definition as coming from AudioConfig.h.

  2. If I modify AudioConfig.h to remove the #define LED_BUILTIN lines, LED_BUILTIN is correctly defined to 15 by pins_arduino.h.

This is because pins_arduino.h does NOT define the symbol LED_BUILTIN but, instead allocates an int for it:

static const uint8_t LED_BUILTIN = 15;

Therefore, in AudioConfig.h, the test:

#ifndef LED_BUILTIN is true, since there is no LED_BUILTIN symbol defined.

@pschatzmann
Copy link
Owner

pschatzmann commented Mar 14, 2023

Oh good, why would anybody do something like this and define LED_BUILTIN as const!
I commented them out in my code...

espressif/arduino-esp32#4134

@LenStruttmann
Copy link
Author

LenStruttmann commented Mar 14, 2023

Thanks for making that change.

As for why, for most boards supported by Platformio, it appears that it is defined thus:

`heltec_wifi_kit_32_v3/pins_arduino.h:static const uint8_t LED_BUILTIN = 35;

heltec_wifi_kit_32_v3/pins_arduino.h:#define BUILTIN_LED LED_BUILTIN // backward compatibility

heltec_wifi_kit_32_v3/pins_arduino.h:#define LED_BUILTIN LED_BUILTIN`

...which works with your code.

There are some boards, though, that omit that second define, which doesn't play well with your code.

BTW, AudioTools is great! I'm using it to develop a theater prop "amulet" that has lights and sound, for a local community theater production of "Peter and the Starcatcher".

@pschatzmann
Copy link
Owner

pschatzmann commented Mar 14, 2023

Oh, cool...when you're done you can maybe share your project in show and tell

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

No branches or pull requests

2 participants