-
Notifications
You must be signed in to change notification settings - Fork 3k
Add code to verify if external Wifi module is still responsible #11819
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
I generally like this solution more than the blind sleep. |
@tymoteuszblochmobica, thank you for your changes. |
42b1db5
to
4823624
Compare
@tymoteuszblochmobica Please provide these details in the commit msg. I've noticed in another PR, similar thin: very brief commit messages. It only helps if you can add these there. Once updated, we will start CI |
4823624
to
502daa9
Compare
Updated commit msg. |
Set to preceding PR, will wait |
after DNS Query flooding during DNS timeout test. DNS timeout test requires flooding device with DNS queries. This causes problem to ESP8266 module causing it to stuck for 11 sec. In result all following tests fails. To avoid this "smart delay" is added. If device preforms gethostbyname correctly then tests can proceed. Otherwise after 1 sec sleep gethostbyname is repeated until success or re-check limit (set to 15). Thanks to this all ethernet and non ESP8266 wireless devices don't need to wait but ESP must wait.
502daa9
to
678b57c
Compare
@0xc0170 , can you help us figure out what the problem is? When we click "Details" we get 404 error. |
pr-merge fixed (was internal error). @michalpasztamobica As this still waits for another PR , is there anything we can do here? |
@0xc0170 , I thought the other PR would come in faster, so perhaps I take back my original idea to wait... This PR is approved by Antti, lets merge it in. |
@AnttiKauppila Please review the suggestion above and let us know if we proceed with this one for 5.15 ? |
@AnttiKauppila , would you please, review again, this time - just to approve that this can go in now, without waiting for the ESP8266 fix being validated (that might take a while)? |
@0xc0170 this seems ready for CI 🚀 |
CI restarted |
Test run: SUCCESSSummary: 5 of 5 test jobs passed |
DNS timeout test fixed to mitigate ESP8266 Wifi module low performance.
Description (required)
DNS timeout test requires flooding device with DNS queries.
After this test finishes gethostbyname check is performed to ensure the device is still alive.
Summary of change (What the change is for and why)
DNS timeout test requires flooding device with DNS queries.
This causes problem to ESP8266 module causing it to stuck for 11 sec.
In result all following tests fails. To avoid this "smart delay" is added.
If device preforms gethostbyname correctly then tests can proceed, otherwise after 1 sec sleep gethostbyname is repeated until success or re-check limit (set to 15).
Thanks to this all ethernet and non ESP8266 wireless devices don't need to wait but ESP must wait.
Documentation (Details of any document updates required)
Pull request type (required)
Test results (required)
Reviewers (optional)
@SeppoTakalo
@AnttiKauppila
Release Notes (required for feature/major PRs)
Summary of changes
DNS timeout test module fix
Impact of changes
Migration actions required