Skip to content

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

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:

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
…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!
earlephilhower pushed a commit that referenced this pull request Jan 27, 2020
…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

Labels

None yet

5 participants