-
Notifications
You must be signed in to change notification settings - Fork 171
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
Fix for hanging response body download #57
base: master
Are you sure you want to change the base?
Fix for hanging response body download #57
Conversation
Hi @TechNyquist, thanks for opening this pull request! Do you have an example sketch to reproduce the issue? Also, what version of the MKRGSM library are you using? |
Hi @sandeepmistry, thanks to you for bringing up all this stuff. So, my MKRGSM library version is 1.3.3 and I did not provide any example because I did nothing special but the normal usecase of GSM/GPRS connection, then a simple GET to some HTTPS client. By the way, actually I did something: just added GSMSecurity support to GPRS in order to enable GSMSSLClient for connecting to HTTPS host. Anyway below is the script used for testing.
Aside: also note that it's not the only problem: I noticed that on SSL transport, sometimes servers drop connection at the end of body while responding, and response reading routine maybe does not expect this (content-length?) and continues to wait for data, hanging till timeout. Anyway this is not related to this PR, just a bonus tip. |
Hi @TechNyquist, I'm having trouble reproducing this, here's my example sketch:
Would you be able to share the response headers and body of the server you are using? Also,
These API's are part of the 1.3.3 of the library ... |
I've tried another test sketch, this time using HTTPS:
All seems ok, here's the output of the serial monitor:
|
So long... but I found the time to test and I'm here again! I took your ino code @sandeepmistry and tested on it in a brand new project, so I'm here to share the results with you all. I replaced the endpoint with my one from AWS (just for more succinct reply, but tested microsoft robots file too and the behaviour is the same as well, despite the different durations). The final result is that the issue showed. It is successful 4 times, then from the fifth on it fails. Follows the original print out from your code:
So I tried to go deeper and printed out all the chars the client gets back from server (char followed by its ASCII code in square brackets), and the print out is the following:
Then I had the suspect that connection to server was failing after a preset amount of previous sessions, so I added a outcome test on connection to host, and the following is the print out:
So I tought that the problem was GSM session drop, so I placed a re-registration on GSM when connection fails, and the final print out is the following:
So re-registration actually does not solve the problem. As conclusion I'm not so sure about what's going on here. The problem seems to show after 4 retries for me and seems different from the first assumptions.
I attach the final code file of the last test, both "ino" file and modified "HttpClient.cpp" for chars print out. I hope to have shed some more light on the issue. |
Hi everyone,
Scenario
I was using ArduinoHttpClient library to handle HTTPS over GSM on a Arduino MKR 1400 and MKRGSM library to simple custom AWS endpoint, whose body was "hello". Soon I noticed that retrieving that body (after a header section of about 7 entries) took a very very long time (apparently hanging forever).
Exact download time was: 4 minutes and 32 seconds.
Debugging
I debugged the library and found why it was taking so long: when the chars scan is matching pivot strings (so for example "Content-Length:") it goes fast; then when no pivot matches and a simple chars collection takes place (that is the "default" case for the switch in
readHeader()
), for some reason chars are not available at every loop ofskipResponseHeaders()'s
while, so the
if(available())returns
false, moving the flow in the
elsesection where lies a
delay` that gives breath before searching for other chars.I don't know exactly why chars are not available at every loop (didn't go so deep), but the actual result was that the delay takes place at every downloaded char.
A delay of 1 second per char is very much for an HTTP response (headers + body).
Fixing
So I fetched the constant with the amount of milliseconds to wait in that delay and reduced the wait time from 1 second to 30 milliseconds (fairly above 20 milliseconds that is the manual suggested minimum wait time between requests to the uBlox chip).
Now download time to the same request is: 12 seconds.
Don't know if this involves side-effects that I'm not aware of, but I rely on your wisdom for this.
Greetings
By the way, this is my first ever pull-request, gotta learn how this is done properly... so go easy on me, okay? ;)
Cheers
Sidenoting
Searching through the issues I found that few could be related to this, links follow: