s00500 / ESPUI

A simple web user interface library for ESP32 and ESP8266
https://valencia.lbsfilm.at/midterm-presentation/
Other
896 stars 166 forks source link

Arduino 7 support #307

Closed MartinMueller2003 closed 2 months ago

MartinMueller2003 commented 2 months ago

My personal opinion is that such rests should always be enabled. In this case they allow prospective users to see how the feature works. They don't add a noticeable delay to compiles, dont add a lot of code size and if they are turned off and something breaks, we would never see the issue.

s00500 commented 2 months ago

Hm ok, I think they do not have to be added into the example as defines though... can as well just be in...

MartinMueller2003 commented 2 months ago

I very much agree. They were initially implemented as a compile option. IMO, there is no reason whey they cannot always be there and removed the conditional compilation.

MartinMueller2003 commented 2 months ago

Any chance we can get this merged?

s00500 commented 2 months ago

sure, should we remove the defines and just have these things in anyway ?

I can make a release too after the merge

MartinMueller2003 commented 2 months ago

The original intent was to support both ArduinoJson 6.x.x and 7.x.x. It seems to do this now. I would not want to turn the tests off by default. A user can turn off the tests but I am under the impression that the tests are also your "Sales Tool" showing what the library can do. Turning off those tests may result in people not seeing that the functionality is available to them.

s00500 commented 2 months ago

Turning them off was not really the intent, just removing the defines so they are always included. After all the gui sketch is just an example... what do you think?

Greetings,

Lukas Bachschwell

https://lbs.sh On 14. Jul 2024 at 16:15 +0200, Martin Mueller @.***>, wrote:

The original intent was to support both ArduinoJson 6.x.x and 7.x.x. It seems to do this now. I would not want to turn the tests off by default. A user can turn off the tests but I am under the impression that the tests are also your "Sales Tool" showing what the library can do. Turning off those tests may result in people not seeing that the functionality is available to them. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

s00500 commented 2 months ago

Meanwhile: thanks for the awesome work here, did not mean to under appreciate that 😅

s00500 commented 2 months ago

Cool, thanks, Lgtm, will do a release when near a computer again 😅

Thanks so much!

MartinMueller2003 commented 2 months ago

I just pushed a version without the conditional flags. It now builds all of the new tests.

FYI: 100% of the Arduino IDE examples fail to compile the ESPAsyncWebServer Library. I did NOT change anything there. I am also not a Arduino IDE guy so I do not really know how to fix it.