You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
ESP_ATcontrol.connect() tries to connect multiple times within a loop. It has a retries-variable which is meant to control the number of retries, but it is reset to 3 within the loop, so the loop is actually endless and the retries-variable is meaningless. I think this is a programming-error which can be solved easily.
The situation with ESPAT_WiFiManager.connect() is a bit different. It has a failure-count and once it is greater than the threshold attempts which is set in the constructor, the device is reset, the failure-count is cleared and the loop starts again. This would also result in an endless loop. But since this connect-method calls the ESP_ATcontrol.connect() internally, the reset and retry here is never executed anyhow. So fixing the endless loop in ESP_ATcontrol.connect() would then trigger the endless loop here.
I would suggest changing the logic: after every failure, do a reset but don't reset the failure-count. Once the failure count is larger than attempts, the method should give up and raise a RuntimeError.
The funny thing is that these two retry-loops are not the only ones involved. ESP_ATcontrol.connect() calls ESP_ATcontrol.join_AP() which itself passes a retries-parameter to the AT-firmware of the device. So ignoring the fact of the endless loops the current code has 18=2x3x3 retries, 15 seconds each.
A clean solution should probably get rid of all the loops in the upper-levels of the code and just pass the relevant parameters down to join_AP, i.e. do the looping in one place. Personally I would also get rid of the reset()-logic within ESPAT_WiFiManager.connect() in favor of failing faster.
Any objections or maybe better suggestions? If not I will be happy to provide a pull request.
The text was updated successfully, but these errors were encountered:
I think it makes sense to centralize the retry logic in one place just as a general practice. The fact that the current implementation is broken in multiple ways just reinforces that. 😄 My vote would be to go ahead and submit a PR!
ESP_ATcontrol.connect()
tries to connect multiple times within a loop. It has aretries
-variable which is meant to control the number of retries, but it is reset to 3 within the loop, so the loop is actually endless and theretries
-variable is meaningless. I think this is a programming-error which can be solved easily.The situation with
ESPAT_WiFiManager.connect()
is a bit different. It has a failure-count and once it is greater than the thresholdattempts
which is set in the constructor, the device is reset, the failure-count is cleared and the loop starts again. This would also result in an endless loop. But since this connect-method calls theESP_ATcontrol.connect()
internally, the reset and retry here is never executed anyhow. So fixing the endless loop inESP_ATcontrol.connect()
would then trigger the endless loop here.I would suggest changing the logic: after every failure, do a reset but don't reset the failure-count. Once the failure count is larger than
attempts
, the method should give up and raise a RuntimeError.The funny thing is that these two retry-loops are not the only ones involved.
ESP_ATcontrol.connect()
callsESP_ATcontrol.join_AP()
which itself passes a retries-parameter to the AT-firmware of the device. So ignoring the fact of the endless loops the current code has 18=2x3x3 retries, 15 seconds each.A clean solution should probably get rid of all the loops in the upper-levels of the code and just pass the relevant parameters down to
join_AP
, i.e. do the looping in one place. Personally I would also get rid of the reset()-logic withinESPAT_WiFiManager.connect()
in favor of failing faster.Any objections or maybe better suggestions? If not I will be happy to provide a pull request.
The text was updated successfully, but these errors were encountered: