search

a9183756-gh / Arduino-CMake-Toolchain

CMake toolchain for all Arduino compatible boards
MIT License
135 stars 40 forks source link

ESP8266: glue.h #error Must defined ARDUINO or OPENSDK #68

Closed TobyChaloner closed 3 days ago

TobyChaloner commented 2 weeks ago

The ESP8266 glue.h is assuming the -DARDUINO=123456 is a number. The entry in the installed Arduino /lib/version.txt in /usr/share/arduino is 1.8.19+dfsg1-1. Running the Arduino IDE, the IDE mangles this to 108019. Where are the cmake toolchain is mangling it to 108019+dfsg1-1

Which is correct? Should the -DARDUINO be a number or should it be the same as the Arduino IDE, which drops the +dfsg1-1. Is glue.h over assuming that the #define is a number, not a string?

in glue.h the following checks for a non-zero number, where the string is not a number #if !ARDUINO && !OPENSDK

glue.h

#ifndef ARDUINO
#define ARDUINO 0
#endif

#ifndef OPENSDK
#define OPENSDK 0
#endif

#if !ARDUINO && !OPENSDK
#error Must defined ARDUINO or OPENSDK
#endif

Versions ESP8266 3.1.2 Arduino 1.8.19 cmake version 3.22.1

The compilation command (abbreviated) is:- ... xtensa-lx106-elf-g++ ... -DARDUINO=108019+dfsg1-1 ... esp8266/3.1.2/ ... /Esp-version.cpp -o ... esp8266/Esp-version.cpp.o

TobyChaloner commented 2 weeks ago

Just noticed that on Technyon's fork, there is an unmerged fix

https://github.com/technyon/Arduino-CMake-Toolchain/pull/6

technyon commented 2 weeks ago

Good point, I merged it.

TobyChaloner commented 2 weeks ago

Hi @technyon thanks for merging #6. There is a similar fix for the master branch https://github.com/technyon/Arduino-CMake-Toolchain/pull/8. Which branch is active, the comment in the Readme says you would like us to exercise the release-1.1-dev branch. Looking at the activity, release-1.1-dev does not look as active as master. Would you still like us to beta-test it?

technyon commented 1 week ago

Sorry for the late reply. Use the master branch, that comment is there from the original repository

TobyChaloner commented 1 week ago

Sorry for the late reply. Use the master branch, that comment is there from the original repository

Hi, the merge from 4 weeks ago was not on the master branch.

Please can you merge #8 on the master branch. Thank you.

technyon commented 4 days ago

It's merged now