ph1p / ikea-led-obegraensad

ESP32/Arduino hack for the ikea OBEGRÄNSAD led wall lamp
MIT License
578 stars 78 forks source link

feat: use wifi manager to configure AP #59

Closed ahuber21 closed 6 months ago

ahuber21 commented 7 months ago

This change adds https://github.com/tzapu/WiFiManager. It works without problems on ESP32. On ESP8266 I see compile errors due to conflicting symbols. The HTTP request types are defined multiple times. Looks like ESPAsyncWebServer.h and ESP8266WebServer.h are incompatible (error attached below).

Before I investigate this further, I wanted to ask if this contribution has a chance of being accepted. If so, I see two options, feel free to comment:

.pio/libdeps/d1_mini_pro/ESP Async WebServer/src/ESPAsyncWebServer.h:62:18: error: 'HTTP_GET' conflicts with a previous declaration
   62 |   HTTP_GET     = 0b00000001,
      |                  ^~~~~~~~~~
In file included from .pio/libdeps/d1_mini_pro/WiFiManager/WiFiManager.h:74,
                 from src/main.cpp:3:
/Users/ahuber/.platformio/packages/framework-arduinoespressif8266/libraries/ESP8266WebServer/src/ESP8266WebServer.h:47:29: note: previous declaration 'HTTPMethod HTTP_GET'
   47 | enum HTTPMethod { HTTP_ANY, HTTP_GET, HTTP_HEAD, HTTP_POST, HTTP_PUT, HTTP_PATCH, HTTP_DELETE, HTTP_OPTIONS };

Edit: Please note I had to rename webserver.{h,cpp} -> asyncwebserver.{h,cpp} as the wifi manager includes WebServer.h. The name asyncwebserver is of course up for discussion.

NemoN commented 7 months ago

@ahuber21 Maybe this helps https://github.com/me-no-dev/ESPAsyncWebServer/issues/418#issuecomment-667976368 for the ESP8266 issue

ph1p commented 7 months ago

I love it! This is definitely a change that should be merged. The incompatibility issue will come up every time anyway. There are so many different boards. There should just be a description in the README for most boards in case a feature works differently or not at all. A section that says: "If you have this board, then do this and this..." etc.

kohlsalem commented 7 months ago

Hmm, once this is mereged, wound'nt this be the ideal base to add aditionall parameters like timezone?

ahuber21 commented 7 months ago

@ph1p this now builds successfully on both platforms -- on ESP8266 we just keep the old solution.

Unfortunately, the web server started by wifi manager and the one from this package don't like each other. So whenever wifi manager starts a server, we need to reboot the device. This should only happen when no WiFi is found though, so after a successful configuration we should be good.

Maybe there's a cleaner solution I didn't think of. But for now, this runs stably on my display.

ahuber21 commented 7 months ago

I don't fully understand the mDNS for which I had to fix a few conflicts. I should probably do some more testing if everything still works...

ahuber21 commented 7 months ago

@schw4rzlicht can you take a quick look at my changes--see if anything looks like it would break your mDNS contribution?

schw4rzlicht commented 7 months ago

@schw4rzlicht can you take a quick look at my changes--see if anything looks like it would break your mDNS contribution?

Looks good to me :)

ahuber21 commented 6 months ago

In this case I'd be ready to merge @ph1p

ph1p commented 6 months ago

In this case I'd be ready to merge @ph1p

Alrighty :) Thank you very much for that! I will test it and then it will be merged

MauiKano commented 6 months ago

I'm not experienced in github and therefore do not know how to review the changes from Andreas Huber. However, I very much like the addition of the WiFi Manager. I made a clone copy of that repository and all works well for me. So, please merge all enhancements back to the original code tree.