search

arduino-libraries / Arduino_ConnectionHandler

GNU General Public License v3.0
97 stars 36 forks source link

Arduino_ConnectionHandler does not handle open networks correctly #114

Open thomasvdv opened 8 months ago

thomasvdv commented 8 months ago

At line 101 in Arduino_WiFiConnectionHandler.cpp:

https://github.com/arduino-libraries/Arduino_ConnectionHandler/blob/2b21c3c361428f38b95b9a3f506a4cf7e7a85c87/src/Arduino_WiFiConnectionHandler.cpp#L101

The code should check for an empty password and, if so, call WiFi.begin(_ssid) instead of WiFi.begin(_ssid, _password).

WiFi.begin(_ssid, "") results in a failed connection on open networks that do not require a password to connect. WiFi.begin(_ssid) connects successfully.

pennam commented 8 months ago

Hi @thomasvdv what board are you using? I thing this should be handled at WiFi class level.

thomasvdv commented 8 months ago

Hi @pennam . I am using a Arduino UNO R4 Wifi. I can file a ticket with the Wifi library. Would it make sense to also include this check in Arduino_ConnectionHandler?

pennam commented 8 months ago

Hi @thomasvdv i thought a bit about your request and i've changed my mind. I think you was right and this issue would be better handled in this library because is not correct for WiFi.begin("ssid", "pwd") to fallback into WiFi.begin("ssid") in case of empty password; that call should fails as is doing.

My proposal is to add a new WiFiConnectionHandler constructor without the "pwd" parameter that will be initialized to nullptr so it chan be handled in the update_handleConnecting function.

thomasvdv commented 8 months ago

Hi @pennam If you move it to a different constructor, then ArduinoIoTCloud will need to make changes to their thingProperties.h code generation. They currently call WiFiConnectionHandler ArduinoIoTPreferredConnection(SSID, PASS);

Line 38 in https://github.com/arduino-libraries/ArduinoIoTCloud/blob/master/examples/ArduinoIoTCloud-Advanced/thingProperties.h

pennam commented 8 months ago

yes you are right, this would be the correct way to handle it.

thomasvdv commented 8 months ago

Hi @pennam I am teaching a class in two weeks that depends on this fix. Would it be possible to get the fix release before then? I am happy to work on a patch if that helps.

pennam commented 8 months ago

Hi @thomasvdv If you prepare a pr with the new WiFiConnectionHandler constructor without the pwd parameter i can review it and merge if everything works, but this will not solve your problem until the changes will be adopted in the cloud generated sketch.

For your class you should consider to load a custom Arduino_ConnectionHandler library to the cloud editor that implements the fallback logic.