-
Notifications
You must be signed in to change notification settings - Fork 7.6k
RainMaker - Improve sysProvEvent function #7932
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uploading actual logs while provisioning will help.
#endif | ||
break; | ||
case ARDUINO_EVENT_PROV_INIT: | ||
wifi_prov_mgr_disable_auto_stop(10000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code was added to handle provisioning failures when the log level is set to "Debug". This shouldn't be removed.
switch (sys_event->event_id) | ||
{ | ||
case ARDUINO_EVENT_WIFI_STA_GOT_IP: | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please look at the code format in the example and format it accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update other examples as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please look at the code format in the example and format it accordingly.
Hi!
Could you add a lint with clang-format or similar to Github action to auto format according to Espressif style on PR?
Best regards.
wifi_prov_mgr_disable_auto_stop(10000); | ||
break; | ||
case ARDUINO_EVENT_PROV_CRED_SUCCESS: | ||
wifi_prov_mgr_stop_provisioning(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be removed
Add ARDUINO_EVENT_PROV_INIT and ARDUINO_EVENT_PROV_CRED_SUCCESS back
Hi @VojtechBartoska ! Which answer are you waiting for? Best regards. |
Hello @FernandoGarcia , excuse me but I've missed your last commit with updates up to requested changes. We'll review your code soon. |
This PR does provide more useful information to the user but does not help in solving the WiFi connection failure issue addressed here. Solving that issue would require changes in the Arduino WiFi library. The WiFi disconnection error code 202 (AUTH_FAIL) is considered a non-reconnectable reason and only one reconnection attempt is made. Adding the reconnection code in the rainmaker sketch won't solve the issue. |
@sanketwadekar AUTH_FAIL means that PASS was wrong. No? If so, then how is that reconnectable? |
You're right, it should not be reconnectable. However, @FernandoGarcia reported that the device did not connect to WiFi after a reboot. See comment here. I am not sure what could be the reason behind it. |
@sanketwadekar if the ESP returns AUTH_FAIL and then later on connect with the same credentials, then the issue is somewhere else. If we patch this, it would fix the issue for @FernandoGarcia but it will break the general functionality for everyone else. I would rather have this resolved upstream in ESP-IDF and have it working correctly. |
@me-no-dev , AUTH_FAIL is sometimes returned even when the signal strength is poor. The esp-idf examples' common component does not have any check for disconnect reason as you can see here |
Hi! It isn't the case because I was pretty close from router and my board has external antenna. Best regards. |
@FernandoGarcia What I mean here is that wrong credentials may not necessarily be the reason for AUTH_FAIL. It could be received in other cases too. |
But you will get it when credentials are wrong, so that would cause pointless reconnect on loop. |
how about we address AUTH_FAIL issue separately. Seems unconnected to this PR. BTW @FernandoGarcia please fix the conflicts. AUTH_FAIL: I propose a method to be added to WiFiSTA to control wether reconnects will be attempted on AUTH_FAIL. This way it can help the folks with that issue, while not causing pointless reconnects to the rest of the users. |
Done! |
@FernandoGarcia please fix the build errors so we can merge it :) |
@shahpiyushv @sanketwadekar I am looking at our code and do not see a reason why WiFi would not reconnect on AUTH_FAIL |
Improve sysProvEvent function to prevent this type of error.