philbowles / PangolinMQTT

PangolinMQTT - ArduinoIDE client library for ESP8266, ESP32 and STM32-NUCLEO
Other
71 stars 21 forks source link

does not compile with `-Werror-reorder` on platformIO #3

Closed clementnuss closed 4 years ago

clementnuss commented 4 years ago

The library does not compile by default in PlatformIO for a STM32, because the reorder warning is considered as error by default. A simple workaround is to put:

build_unflags = -Werror=reorder

in the platformio.ini.

Could order of the arguments in be reordered to avoid this mistake in the future ? (probably many user will face it as it is the default compile option).

Thanks anyway for this library and your hard work 😊👌👏

philbowles commented 4 years ago

"Could order of the arguments in be reordered to avoid this mistake in the future ? (probably many user will face it as it is the default compile option)."

No. And there's a good reason why: Pangolin is an Arduino library, and as such is 100% compatible with the ArduinoIDE and build system. PlatformIO is not. If PlatformIO has problems with code that compiles and runs correctly under ArduinoIDE, then it is a PlatformIO problem, not an issue with this - or any other - valid Arduino library.

For about 3 years now I have been notifying the PlatformIO team of errors in their build setting related to the use of non-standard and non ArduinoIDE-compatible architecture #defines which break many vaild Arduino libraries. For 3 years they have not fixed it. So I refuse to provide any support on any of my 100% Arduino compatible libraries for users of PlatformIO.

I suggest you take it up with the PIO team - it is they who have a non-standard setup, choose ridiculous warning levels on their build settings etc. Compiler warnings are warnings. They can be ignored where they have no effect on the behaviour of the code.
If you have issues with code behaviour, I am happy to fix. Problems with PlatformIO should be taken up with their development team.

philbowles commented 4 years ago

Also the STM32 support is not yet complete. I have test programs running successfully but I have not yet made a formal release. I will update the docs to reflect that shortly, but for anything else PlatformIO related, see above.

pfeerick commented 4 years ago

It doesn't fail when compiled against the NUCLEO F429ZI with PlatformIO, once the old/invalid library.json is updated to indicate STM32 rather than espressif8266 compatibility.

The re-order warning is still valid (and is not an error by default, but still only a warning), and has absolutely nothing to do with PlatformIO, as it is a GCC compiler warning, which you can see in the Arduino IDE if you change from the default 'none' to 'all', and then focus on the warnings specific to your library - which are mostly warnings about initialisation order and comparing signed and unsigned variables. Which all library authors should do, in order to ensure they are writing error-free code.

You never did reply to the question asked by the development team in the issue your raised earlier in the year, which is why it was closed. I also fail to understand what defines are missing, since all the standard ones are defined... i.e.

-DESP8266
-DARDUINO_ARCH_ESP8266
-DARDUINO_ESP8266_WEMOS_D1MINI
-DARDUINO=10805

The only real issue you are likely to encounter when migrating from the Arduino IDE to PlatformIO is that of the non-standard C++ the Arduino pre-processor encourages you to write (although you can ignore this and stick with .ino files), and processing of C/C++ Preprocessor conditional syntax (ifdef wrapped includes is the usual one that fails)... which can be enabled on a per library basis, meaning the library author can prevent it being an issue, or via a project configuration value. I suspect this is the real issue you encountered, not the generation of the ARDUINO_<PROCESSOR-DESCRIPTOR>_<BOARDNAME> define.

I'm not saying PlatformIO is perfect, it has it's bugs and issues... but after being a long time Arduino IDE user and finding it very limiting, I find PlatformIO to be a lot faster, a lot more flexible, allows me to use a single proper IDE for all my embedded development, and works exactly the same across OSs and architectures(x86/armhf/aarch64). The few issues I've had are well worth the benefits, plus with it now all being open source, more people are contributing to it. I have found the development team very responsive if you can provide a clear example of what is wrong, and very receptive to suggestions on how to fix it.

CODeRUS commented 4 years ago

Can you provide "Zero-warning" stable code instead of pointing to other "wrong" IDE/compiler, which have proper c++ support not possible with Arduino IDE "sketches"?