pixelit-project / PixelIt

PixelIt is an ESP8266/ESP32 and WS2812B LED Matrix based PixelArt display
https://pixelit-project.github.io
MIT License
314 stars 52 forks source link

Request: use WiFiManager v2.0.17 #359

Closed jekader closed 2 weeks ago

jekader commented 1 month ago

Currently, the esp32 target is using WiFiManager v2.0.15-rc.1 while esp8266 is stuck with 0.16.0 for some reason. The library supports both platforms officially and there's been a new release not that long ago: https://github.com/tzapu/WiFiManager/releases/tag/v2.0.17

The proposal is to use the latest version across all targets.

I don't have all board types on my hands to test the changes so want to discuss it here first. Is there a reason for an ancient version of WiFiManager to be used for esp8266?

foorschtbar commented 1 month ago

No special reason, the "old" just works. If someone tests the new lib, we could upgrade.

o0shojo0o commented 1 month ago

I don't have all the targets available and therefore can't test it.

jekader commented 1 month ago

It would be enough to test this on a bare esp8266 without even setting up the clock part: as long as it's able to set up the initial connection and raise wifi on boot we are good.

Will look through my drawers to see if I can find a devboard, otherwise they're dirt cheap to order.

jekader commented 3 weeks ago

Got my hands on a esp8266 and tried out various WiFiManager versions with codebase from main:

Upgraded the 8266 core version to the latest avalable espressif8266@4.2.1 and WiFiManager v2.0.17 works just fine with proper wifi setup and reliable connections, so the issues were likely due to an outdated SDK.

The new core version broke the build so minor fixes were added to some webSocket.sendTXT invocations.

Now the issue I'm facing is the text color being set to #000000 possibly due to an issue with ColorConverter not handling the # sign properly on this compiler version. Looking into this, the library seems to be deprecated so let's see if there is an easy fix on pixelit side.

In the test area, when I send a JSON with "hexColor": "FFFFFF" the string shows up but with the default "hexColor": "#FFFFFF" format it doesn't. Same with backup-restore: if I remove the hash from the clockColor parameter value, it does get saved into memory however the live preview still shows nothing. If I roll back to pixelit 2.5.1 and save the value, then upgrade to the test build, the color gets reset to black again.

ColorConverter::HexToRgb is supposed to handle hashes and this works fine with the current SDK version: https://github.com/luisllamasbinaburo/Arduino-ColorConverter/blob/master/src/ColorConverterLib.cpp#L166

Any ideas welcome, did not test peripherals yet before the color can be saved reliably from the UI.

Here's a warning I see during compile time, not sure if related:

.pio/libdeps/ESP8266_generic/ColorConverter/src/ColorConverterLib.cpp: In static member function 'static void ColorConverter::RgbToHex(uint8_t, uint8_t, uint8_t, String&)':
.pio/libdeps/ESP8266_generic/ColorConverter/src/ColorConverterLib.cpp:174:33: warning: 'sprintf' writing a terminating nul past the end of the destination [-Wformat-overflow=]
  174 |  sprintf(hexArray, "%02X%02X%02X", red, green, blue);
      |                                 ^
.pio/libdeps/ESP8266_generic/ColorConverter/src/ColorConverterLib.cpp:174:9: note: 'sprintf' output 7 bytes into a destination of size 6
  174 |  sprintf(hexArray, "%02X%02X%02X", red, green, blue);
      |  ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
foorschtbar commented 2 weeks ago

Thanks for the tests so far. The only thing I could do (I lack the time), I can open you a develop-3.0 branch and test the builds on my ESP8266 PixelIts.

The warning makes sense as sprintf return a null terminated string (the 7th byte) and hexArray appears to be only 6 bytes long. If hexArray is used as a string in an output, it should be one byte longer to account for this

jekader commented 2 weeks ago

Issues sorted out, PR #366 out for review and tested on several of my devices.

foorschtbar commented 2 weeks ago

Thank u. I build and released it as beta release 3.0.0 and will test it also the next days :)