LED_BUILTIN must be macro not const int

[bperrybap]

The symbol LED_BUILTIN is supposed to be a macro not a const int
This was discussed at length on the Arduino developers mailing list years ago and final conclusion is that it must be a macro.
The reason is that some boards have a builtin led and some don't.
The only way for code, particularly library code that runs a many different platforms/core/boards, to "know" if the board that the code is being compiled for has a built in led is to use a macro for LED_BUILTIN so that the code can do compile time conditionals and enable / disable any led code appropriately when being built for various boards.

If you go look at the variants that arduino.cc supplies and even the esp8266 variants, you will see that LED_BUILTIN is a macro rather than a const int

variants that use const int instead of a macro will "trick" code into thinking that there is no built in led.

[CelliesProjects]

I have been bitten by this issue too. Even inside the esp32 eco system it causes confusion.

[bperrybap]

If it is desired to leave it as a const int, an easy thing to do is to add this define:
#define LED_BUILTIN LED_BUILTIN
That will give the best of both worlds.
A macro as needed but that gets replace with the const int value.

And for all those that are unfamiliar with the cpp macro parsing rules defined by the standard, it is not a circular definition.
cpp will replace the LED_BUILTIN in the source with LED_BUILTIN, essentially leaving it alone.
The compiler will replace LED_BUILTIN with the const int value during compilation.

--- bill

[stale]

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[bperrybap]

bump.
This is still an issue. Why should it automatically be closed?

[stale]

[STALE_CLR] This issue has been removed from the stale queue. Please ensure activity to keep it openin the future.

[CelliesProjects]

bump.

[stale]

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[bperrybap]

Still an issue.
The fix is simple.
In all the variant pins_arduino.h files change the LED_BUILTIN lines to include a #define LED_BUILTIN

example:

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

would become:

static const uint8_t LED_BUILTIN = 5;
#define BUILTIN_LED  LED_BUILTIN // backward compatibility
#define LED_BUILTIN LED_BUILTIN // create macro to indicate presence of built in LED

[stale]

[STALE_CLR] This issue has been removed from the stale queue. Please ensure activity to keep it openin the future.

[atanisoft]

@bperrybap submit a PR containing the fix.

[bperrybap]

I would consider it if the project owners would comment and agree to the fix.
This involves changing every single variant file. Not worth the work, if the project owners will simply ignore it.

[atanisoft]

@bperrybap you do realize this is a community supported project right? The community drives most of the development via PRs and issues.

I agree it would be a bit of work to update the various pins files, but if this is a critical feature it would be worth it to invest the time. Future pin headers would need to be validated for the same before merge as well

[bperrybap]

I am fully aware of how community supported projects typically work.
However, you do realize that just because it is community supported project doesn't mean that those in control of the project (the ones that can merge in PRs) are actually open to accepting PRs from the community. I have been involved with several 3rd party Arduino platforms and the openness of the developers to accept PRs from outsiders varies substantially. I have been through cases where I have gone through the effort to document an issue/bug and even provided the PR to fix it and yet have the main developer not only reject the fix, but actively deny that there was even an issue that needed to be fixed.

This is not the only case of this. In fact some of the worst offenders of this behavior / attitude are the original arduino.cc developers. It took me more than a year to get them accept some changes to fix digitalWrite() to make register updates atomic and to this day, I believe that one of them still does not understand atomicity issures related to RISC instructions and never accepted that there was an atomicity issue related to i/o register updates being done in foreground code.

This is why, now, before I go off and generate a PR to fix an issue, particularly one that involves touching several modules, I like to get some sort of acknowledgement up front that

1. there is an issue that needs to be resolved
2. that they are open to receiving a PR to correct the issue.

[me-no-dev]

we are pretty open for PRs ;) few do not get merged (sometimes takes a while, but happens non the less)

[bperrybap]

@me-no-dev
Thank you, I can generate a PR to fix this.
My concern right now is the commit that @lbernstone lbernstone@b6402ad has just done
as that commit will break all the code that uses LED_BUILTIN as it didn't create the LED_BUILTIN macro correctly.

Can that commit be backed out so a proper PR can be pulled in?

BTW, there are a few other macro symbols that need fixing as well.
Can I correct those at the same time?

[me-no-dev]

new PR was merged with proper definitions :) issue should now be resolved ;)

[bperrybap]

Looks good. In the future, I'll not be hesitant to create a PR.