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

Fix parsing OWM current data #140

Closed
marcelstoer opened this issue Oct 17, 2018 · 10 comments
Closed

Fix parsing OWM current data #140

marcelstoer opened this issue Oct 17, 2018 · 10 comments

Comments

@marcelstoer
Copy link
Member

Follow-up to https://support.thingpulse.com/799/currentweather-openweathermap-shows-espaper-everything

The proposed fix is to drop the outer loop at https://github.com/ThingPulse/esp8266-weather-station/blob/master/src/OpenWeatherMapCurrent.cpp#L68. The more I think about it the less I understand what the benefit actually is. Checking for available data should be enough.

@G6EJD
Copy link

G6EJD commented Oct 17, 2018

I agree , I studied the code and although I code mine identically, I’m now thinking the same. If there was significant amounts of data to stream the connection would not be dropped until the client had received it all (TCP handles that aspect), similarly for small amounts of data the fact that the connection was dropped after the transfer is irrelevant as the data is in the buffer and processed the same as usual.

It’s an interesting issue and not really a network issue as the behaviour is correct and neither is it a coding issue, but it could well be the .connected() status has until now been incorrect and the latest update 2.4.2 has changed things as you originally thought.

@marcelstoer
Copy link
Member Author

@G6EJD @SuperEugen see the enlightening feedback in the linked core issue.

@SuperEugen
Copy link

Good! Thanks that you clarified that with the WiFiClient developers!

@G6EJD
Copy link

G6EJD commented Oct 18, 2018

Good to hear the core was changed and that the testing we did concluded the same.
So now coding techniques need to change to accommodate this.
I shall have to re-programme myself :)

@sorriso93
Copy link

It is not clear to me if now the library has been fixed.... many thanks

@marcelstoer
Copy link
Member Author

We‘ll close this once it‘s fixed.

@G6EJD
Copy link

G6EJD commented Oct 18, 2018

I think no fix is required and this is now the new behaviour, which is correct in TCP terms, previously it was incorrect.

@marcelstoer
Copy link
Member Author

We have to adjust this library.

@G6EJD
Copy link

G6EJD commented Oct 18, 2018

My mistake, I was thinking of the ESP core

@marcelstoer
Copy link
Member Author

1.6.3 is available for download and in the Arduino Library Manager.

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

No branches or pull requests

4 participants