Skip to content

Corrected board definitions to include ESP8266 #6972

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Corrected board definitions to include ESP8266 #6972

wants to merge 2 commits into from

Conversation

ttytyper
Copy link

@ttytyper ttytyper commented Jan 1, 2020

According to the sonoff_*.json board files from PlatformIO, the build flags are supposed to be ARDUINO_ESP8266_SONOFF_BASIC etc. However, pins_arduino.h expects them to be ARDUINO_SONOFF_BASIC etc, without ESP8266. This causes sketches that make use of i2c with Wire.h to fail, as the necessary pins do not get defined.

I suspect that pins_arduino.h is incorrect and propose this correction. My apologies if this is not the correct way to fix the issue - it's difficult to wrap my head around how the different frameworks, IDEs, build tools etc interact :)

Reference: https://github.com/platformio/platform-espressif8266/tree/develop/boards

@ttytyper
Corrected board definitions to include ESP8266
a83fa1c 
According to the sonoff_*.json board files from PlatformIO, the build flags are supposed to be ARDUINO_ESP8266_SONOFF_BASIC etc. However, pins_arduino.h expects them to be ARDUINO_SONOFF_BASIC etc, without ESP8266. This causes sketches that make use of i2c with Wire.h to fail, as the necessary pins do not get defined.

I suspect that pins_arduino.h is incorrect and propose this correction. My apologies if this is not the correct way to fix the issue - it's difficult to wrap my head around how the different frameworks, IDEs, build tools etc interact :)

Reference: https://github.com/platformio/platform-espressif8266/tree/develop/boards
@mcspr
Copy link
Collaborator

mcspr commented Jan 1, 2020

It is a bit more complicated than that, since Arduino IDE uses a different build config through platform.txt and boards.txt ini-like files:

Arduino/platform.txt

Line 104 in fa5040d

recipe.cpp.o.pattern="{compiler.path}{compiler.cpp.cmd}" {compiler.cpreprocessor.flags} {compiler.cpp.flags} -D{build.sdk}=1 -DF_CPU={build.f_cpu} {build.lwip_flags} {build.debug_port} {build.debug_level} -DARDUINO={runtime.ide.version} -DARDUINO_{build.board} -DARDUINO_ARCH_{build.arch} -DARDUINO_BOARD="{build.board}" {build.led} {build.flash_flags} {compiler.cpp.extra_flags} {build.extra_flags} {includes} "{source_file}" -o "{object_file}"
(note the -DARDUINO_{build.board}, which means that the IDE will use the variable provided by the menu entry / variable depending on the selected board type)

Resulting boards.txt is generated by this script:
/tools/boards.txt.py#L866 (part with sonoff boards)
So you'd need to change it too and regenerate boards.txt so that IDE uses correct definitions

Having _ESP8266_ seems like a correct way to name boards, but not sure what are implications for backwards compatibility are.

@mhightower83
Copy link
Contributor

@ttytyper If you like, I can supply you with the boards.txt.py changes for your PR or I can create a PR and include your changes?

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 17, 2020

@mcspr you are right, in this PR board.txt.py needs this change:

            '.menu.BoardModel.sonoffSV.build.board': 'SONOFF_SV',

to

            '.menu.BoardModel.sonoffSV.build.board': 'ARDUINO_SONOFF_SV',

(and similarly with other itead/sonoff models)
@mhightower83 you are the initial author for the itead board update, would you agree with this changes ?
@ttytyper would you be able to update your PR with these changes ? Or leave @mhightower83 do it as proposed ?

@mhightower83
Copy link
Contributor

@d-a-v actually, it looks like the recipe in platform.txt adds the ARDUINO_ part, we need to add the ESP8266_ to SONOFF_SV to complete the correction, etc.

'.menu.BoardModel.sonoffSV.build.board': 'ESP8266_SONOFF_SV',

## Compile c++ files
recipe.cpp.o.pattern=... -DARDUINO_{build.board} -DARDUINO_ARCH_{build.arch} -DARDUINO_BOARD="{build.board}" ...

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 17, 2020

True, ESP8266_ not ARDUINO_

@ttytyper
Copy link
Author

It looks like you have a much better idea of what needs to be done than I do :) Thanks for looking into it.

@mhightower83 Feel free to make the necessary changes. This is all a bit beyond what I'm comfortable messing around with.

mhightower83 added a commit to mhightower83/Arduino that referenced this pull request Jan 19, 2020
@mhightower83
Made changes to include "ESP8266_" with ITEAD board names, `{build.bo…
59ad2ea 
…ard}`,

as proposed by esp8266#6972 (comment).
@ttytyper 's changes have been incorporate into this PR
The build flag ARDUINO_SONOFF_... should now appear as ARDUINO_ESP8266_SONOFF_...
@ttytyper, @mcspr, and @d-a-v thanks!
@mhightower83 mhightower83 mentioned this pull request Jan 19, 2020
Merged
earlephilhower pushed a commit that referenced this pull request Jan 27, 2020
@mhightower83 @earlephilhower
Made changes to include "ESP8266_" with ITEAD board names, `{build.bo…
0c6be9e 
…ard}`, (#7024)

as proposed by #6972 (comment).
@ttytyper 's changes have been incorporate into this PR
The build flag ARDUINO_SONOFF_... should now appear as ARDUINO_ESP8266_SONOFF_...
@ttytyper, @mcspr, and @d-a-v thanks!
@earlephilhower
Copy link
Collaborator

Superseded by #7024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ttytyper @mcspr @mhightower83 @d-a-v @earlephilhower