Skip to content

add http client configurable timeout usefull when download is slow on server side#4705

Merged
devyte merged 3 commits into
esp8266:masterfrom
marian-craciunescu:fix-esp8266HttpUpdate-client-timeout
May 4, 2018
Merged

add http client configurable timeout usefull when download is slow on server side#4705
devyte merged 3 commits into
esp8266:masterfrom
marian-craciunescu:fix-esp8266HttpUpdate-client-timeout

Conversation

@marian-craciunescu

Copy link
Copy Markdown
Contributor

This Pull request fixes #1676 referenced on openenergymonitor/EmonESP#3 (fix stream error).

@marian-craciunescu

Copy link
Copy Markdown
Contributor Author

@igrr @me-no-dev @d-a-v can you provide some feedback

@d-a-v d-a-v left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity how much do you use ?

@marian-craciunescu

Copy link
Copy Markdown
Contributor Author

@d-a-v I've put it to 30s since the wifi router signal is quite bad at my place.


ESP8266HTTPUpdate::ESP8266HTTPUpdate(int httpClientTimeout)
{
this->_http_client_timeout = httpClientTimeout;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please don't use constructor body for member init. Instead:

ESP8266HTTPUpdate::ESP8266HTTPUpdate(int httpClientTimeout)
    : _http_client_timeout(httpClientTimeout)
{}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.thank you

int _lastError;
bool _rebootOnUpdate = true;
private:
int _http_client_timeout;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please use camelCase.

@devyte devyte merged commit bd87970 into esp8266:master May 4, 2018
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.

[httpUpdate] Update.writeStream failed! (ERROR[6]: Stream Read Timeout)

3 participants