search

philbowles / PangolinMQTT

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

Reorder functions to make example compile. #13

Closed luebbe closed 4 years ago

luebbe commented 4 years ago

Fix version output.

philbowles commented 4 years ago

It compiles fine with ArduinoIDE. Complain to PlatformIO and get them to fix their build system.

philbowles commented 4 years ago

Also, please read the documentation! "Pangolin is an Arduino library, and is 100% compatible with the ArduinoIDE and build system. PlatformIO, sadly, 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 over 3 years 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 over 3 years they have failed to fix it,so I refuse to provide any support on any of my 100% Arduino compatible libraries for users of PlatformIO until they do.

For this reason, I will not accept any issues relating to build problems with PlatformIO, nor any pull requests or other suggestions which involve any changes that render the library less than 100% ArduinoIDE compatible. "

luebbe commented 4 years ago

Phil, I don't understand your reasoning. What's wrong with a PR that makes your example compile in the Arduino IDE and PIO, fixing a wrong output on the way? Did you even check the PR or did you just reject it, because it makes it compile in both environments and you don't want your library or examples to work under PIO? They do, trust me. And if a compiler's default warning/error settings are more strict in one environment, it's even more to my taste, because it prevents nasty bugs from oversights. If you honestly want people to spread the word, you could try to be a bit more supportive instead of brushing off valid points. You can be sure that a lot of the people coming here will be using PIO, Sloeber and not Arduino.

qcasey commented 4 years ago

Respectfully @philbowles, I understand PIO has ignored your suggestions for their build systems and therefore you don't want to waste your time rewriting for them. I get that.

What I don't understand is dening people like @luebbe from doing that work for you. This PR is incredibly minor and fixes builds for a very popular environment. "Embedded MQTT for the real world" should mean more than the one world you approve of.

If there's a reason for your stance besides stubbornness please let me know because I'm quite curious.

pfeerick commented 4 years ago

Especially since this change does not in any way "render the library less than 100% Arduino IDE compatible. " ... this is the sort of reasoning I'd expect to hear from the white house orangutangen, not an open-source programmer!

Misiu commented 4 years ago

@philbowles ESPHome is searching for a new MQTT library and your library is one of the candidates. The submitted PR doesn't break Arduino IDE compatibility. Many large projects like Tasmota, ESPHome, ESPEasy, ESPurna are using PlatformIO. Your library can only benefit from this PR because all of those projects could use your library.

You can always develop your library to be only Arduino IDE compatible, but there are users that will submit PR to make it compatible with PlatformIO, all without your work and without breaking anything.

Please reconsider this PR.

martgras commented 4 years ago

As mentioned above the library itself works great for me in pio.
The only issue I found is compiling the ino examples because the ino to cpp conversation doesn't seem to honor all #ifdef ... blocks correctly I just created https://github.com/platformio/platformio-core/issues/3706 to report it.

IMHO it would be a pitty if we have to maintain another fork of this great lib just for pio.