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

Fix build.board variables for NodeMCU and others #1878

Closed
focalintent opened this issue Apr 8, 2016 · 13 comments
Closed

Fix build.board variables for NodeMCU and others #1878

focalintent opened this issue Apr 8, 2016 · 13 comments

Comments

@focalintent
Copy link

I'm maintaining a library that does, among other things, direct pin accesses for higher performance. As part of this, I do my own mapping of pins on the board to the underlying hardware. The adafruit Huzzah boards label each pin on the board with the actual gpis pin, while many nodemcu based boards instead label their GPIO pins D0 - D10 (which then map, under the hood, to the actual set of gpio pins).

Because there is no way, as far as I can tell, at compile time to differentiate when someone has selected the Huzzah variant from, say, the NodeMCU variant - I can't provide the correct mapping for one platform or the other, in part this is because, in your boards.txt, you identify all these boards (adafruit huzzah, nodemcu, d1_mini) as being build.board=ESP8266_ESP12. There's no differentiation between the boards.

Meanwhile, if you look at the selection of attiny8 boards that adafruit has, their build.board values are set to AVR_GEMMA, or AVR_TRINKET3, or AVR_TRINKET5, etc... which allows me to differentiate between their different attiny8 boards and the pin to gpio mappings.

Right now, I get to either tell users of the adafruit huzzah esp boards to add a special define to their sketches to get the "right" mappings, or switch things around and tell users of the nodemcu/weemos/etc... boards to add a special define to their sketches to get the right mappings, etc...

(Also, you haven't defined the Dx mappings for the adafruit board, so someone who has pinMode(D3, OUTPUT); will fail to compile on the adafruit boards. Of course, if they change their code to pinMode(3, OUTPUT); to get it to compile, then go back to the NodeMCU board, pin D9 is what will get set to output (since that's the pin that maps to GPIO 3))

@WereCatf
Copy link
Contributor

WereCatf commented Apr 8, 2016

This is something I bumped into a while ago, but forgot about it. The variable build.variant reflects much better the actual module that's in use, not build.board, so maybe platform.txt should include a -DARDUINO_VARIANT_{build.variant} so you can refer to the specific variant being used with ARDUINO_VARIANT_generic or ARDUINO_VARIANT_nodemcu or such.

Another, IMHO better, way of doing it would be doing -DARDUINO_VARIANT="{build.variant}" so you can just do things like Serial.println(String("Board variant: ") + String(ARDUINO_VARIANT));

For completeness sakes maybe both approaches should be done?

@focalintent
Copy link
Author

Honestly, I'm not really attached to any particular solution - as long as there are defines I can work off of in header files to tweak things for the slightly different board setups

@WereCatf
Copy link
Contributor

WereCatf commented Apr 8, 2016

Well, I made a pull-request for this. We'll see if they accept it or not.

@igrr
Copy link
Member

igrr commented Apr 9, 2016

@WereCatf Thanks for the pull request, however I'm not sure I understand what is the use case for having board variant defined as a string. You can not do any preprocessor checks against a string macro, so this will hardly help @focalintent with his original issue (mapping pins differently at compile time depending on board variant). The only thing i find this useful for is to print the variant name to serial port, but is this really useful? Besides, no other Arduino core does this, so this will not lead to portable code, whereas ARDUINO_VARIANT_{build.variant} is the most portable macro we can get.

@igrr
Copy link
Member

igrr commented Apr 9, 2016

@focalintent The reason NodeMCU "pin numbers" are not used as real pin numbers, which may be passed into digitalRead, digitalWrite, etc., was pretty simple. ESP8266 GPIOs are very non-uniform, for instance what you can do with pin 4, you can not do with pin 0. Hiding real GPIO pin numbers behind an arbitrary mapping makes it harder for the user to remember and check specific properties of any given pin. I think introducing this mapping was a bad design decision in the NodeMCU board, which is quite good in other aspects.
Maybe introducing Dx constants was a bad idea, i guess i didn't give it much thought back then.

@focalintent
Copy link
Author

Personally, I don't mind having a difference between GPIO pin numbers (and ports, a number of the chips FastLED supports have multiple GPIO ports, as well) - I just need a way to differentiate between the boards being compiled for, so I can provide the library users a mapping between the pin numbers given on the board, and the underlying GPIO pins (and, for chipsets that have them, ports).

It's only a problem in a case like currently, where there's no way for me to distinguish between when someone is compiling for a board where, say, pin 0 is GPIO 0 vs. pin 0 being GPIO 16 :)

@focalintent
Copy link
Author

(I bypass digitalWrite in FastLED because I'm interested in the fastest pin cycle times that I can get)

@igrr
Copy link
Member

igrr commented Apr 9, 2016

OK, got that, i think we will clean up build.board values so that boards with different pin mapping get different compile time defines.

@igrr igrr added this to the 2.2.0 milestone Apr 9, 2016
@igrr igrr self-assigned this Apr 9, 2016
@igrr igrr changed the title Compile time indication of which variant is being built Fix build.board variables for NodeMCU and others Apr 9, 2016
@WereCatf
Copy link
Contributor

WereCatf commented Apr 9, 2016

@igrr Ah, so you're not going with the -DARDUINO_VARIANT_{build.variant} - approach, then? If not then I'll just close my pull-request as redundant.

@focalintent
Copy link
Author

@igrr thank you! I'll keep an eye on this ticket and add the appropriate defines/pin mappings into FastLED as soon as I start getting them.

@igrr
Copy link
Member

igrr commented Apr 13, 2016

@focalintent I changed the board name for NodeMCU-style boards in 90f933b. Please let me know if this is sufficient.

@focalintent
Copy link
Author

That looks great - thank you! I'll update FastLED to work with these as well.

@mkeyno
Copy link

mkeyno commented Apr 13, 2016

well done igrr , with this modification I hope @focalintent could put couple of example in his fastLED library specifically for ESP module

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants