Skip to content
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

Bugfix/esp8266 http client #6457

Closed
wants to merge 20 commits into from
Closed

Bugfix/esp8266 http client #6457

wants to merge 20 commits into from

Conversation

Jeroen88
Copy link
Contributor

See PR #6176, this PR was merged while there was still an issue to solve.

With this PR:

  • ::getString() no more returns an error but returns an empty String() if the server sends a '204 no content'
  • if a connection is being reused and this connection gets lost (this can be detected because e.g. ::GET() returns a value less than zero), the connection can be restored by calling ::end() followed by ::begin(). An example of this usage is added as ReuseConnectionV2.ino

This behavior of this example is tested with both a WiFiClient and a (BearSSL) WiFiClientSecure. The lost connection was simulated with switching off the router and switching it on again, and with stopping my HTTP(S) server and restarting it. In the first case a connection refused was reported, in the second case a connection lost. Switching the router back on or restarting the server caused the program to resume showing the ::GET() payload as expected.

@Jeroen88
Copy link
Contributor Author

@d-a-v as promised a new PR.

Could you take a look at ::disconnect() here and here?
I think the two reset()'s should only be done if !preserveClient and should be added to the second block of code as well. What do you think?

@Jeroen88
Copy link
Contributor Author

Last commit fixes style check Travis build error. Other Travis failed tests are not caused by my PR

@Jeroen88
Copy link
Contributor Author

@d-a-v Have you got any idea why the Travis build is failing?

@earlephilhower
Copy link
Collaborator

Arduino IDE nightly just changed, breaking our builds. We're looking into it.

@Jeroen88
Copy link
Contributor Author

@earlephilhower Thnx for your reaction Earle!

Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mklittlefs.exe is back in the PR, somehow.

@Jeroen88
Copy link
Contributor Author

@earlephilhower I do not know where it comes from, but I will try to get it out. Could you have a look at this question:

@d-a-v as promised a new PR.

Could you take a look at ::disconnect() here and here?
I think the two reset()'s should only be done if !preserveClient and should be added to the second block of code as well. What do you think?

@Jeroen88
Copy link
Contributor Author

@earlephilhower please help with git: now I got rid of mklittlefs but got lwip2/builder instead...

@earlephilhower
Copy link
Collaborator

You're in git hell, I think. I'd roil back commits to before you did a push containing lwip and try again.

git reset --hard 85d836b # Go back to GH merge a while back
git rm <path to the mklittlefs.exe you've still got here in this hash> # get rid of mkfs
git pull upstream master # merge w/upstream
git submodule update # will pull in the lwip change in the master branch which you missed and remove from the commit later
git commit -a
git push --force

Jeroen88 pushed a commit to Jeroen88/Arduino that referenced this pull request Sep 1, 2019
@Jeroen88
Copy link
Contributor Author

Jeroen88 commented Sep 1, 2019

Close because of git problems. Created a new PR #6476

@Jeroen88 Jeroen88 closed this Sep 1, 2019
devyte pushed a commit that referenced this pull request May 15, 2020
* Because of git problems, start from a new fork and create a new PR. This was PR #6457

* Style update to pass Travis

* Update ReuseConnectionV2.ino

* fix + enforce testing http code

per @earlephilhower review

* Close connection before ::connecting on HTTP/1.0

HTTPClient never actually closes the TCP connection on its own. It will leave the TCP connection open unless you explicitly do a getString which makes a StreamString and stuffs it with the HTTP server response, at which point the HTTP server itself will close the connection.

If you check the HTTP error code and find failure, unless you do a getString and throw it away, it won't disconnect.  Even in HTTP/1.0 or in cases when you haven't enabled _reuse.

Change the logic in ::connect to only reuse the connection when it is specifically allowed.  Otherwise, fall back to re-connection.

* Adjust example per request

Do single URL get in each loop, avoid infinite for loop at end.

* Fix astyle

* Clean up final pass notice

* Fix example syntax error

Editing code in a web textbox without running it is a painful process.


Co-authored-by: Earle F. Philhower, III <earlephilhower@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants