JHershey69 / OpenWeatherOneCall

Open Weather One Call library to gather current, hourly (48), and 7 day weather for your current location
GNU General Public License v3.0
37 stars 11 forks source link

Back to back historical data call returns wrong values #35

Closed PeterAckermans closed 3 years ago

PeterAckermans commented 3 years ago

Jesssica

Thanks for looking at my reported issue.

Between your update and my proposed changes we missed 1 update. If you back to back ask for historical data the rain values are not reset to 0. As only hourly data exists when rain values are not equal to 0 this means older rain value appear in the next call. PS This doesn't happen if you would call forecast in between as then the local created data is erased and replaced with current data

JHershey69 commented 3 years ago

Would this also work?

if(hourly_0["rain"]) { if(USER_PARAM.OPEN_WEATHER_UNITS == 2) { float temp = hourly_0["rain"]["1h"]; history[x].rainVolume = (temp/25.4); // 95 } else history[x].rainVolume = hourly_0["rain"]["1h"]; // 95

            } else history[x].rainVolume = 0; // if no rain data resets old to 0
PeterAckermans commented 3 years ago

Jessica

Yes I tested that implementation successfully as well.

Peter

Op wo 25 aug. 2021 00:21 schreef Jessica Hershey @.***>:

Would this also work?

if(hourly_0["rain"]) { if(USER_PARAM.OPEN_WEATHER_UNITS == 2) { float temp = hourly_0["rain"]["1h"]; history[x].rainVolume = (temp/25.4); // 95 } else history[x].rainVolume = hourly_0["rain"]["1h"]; // 95

        } else history[x].rainVolume = 0; // if no rain data resets old to 0

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JHershey69/OpenWeatherOneCall/issues/35#issuecomment-905016040, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARKWG637RDRPSLFKM636ZYLT6QLQDANCNFSM5CXQTONA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

PeterAckermans commented 3 years ago

Jessica

Within the snow values retrieval you also have 1 issue where you accidentally used the rain variable. Should be easy to find. If not let me know.

Would be good to fix this as well.

Peter

Op wo 25 aug. 2021 22:21 schreef Spam Ackermans @.***>:

Jessica

Yes I tested that implementation successfully as well.

Peter

Op wo 25 aug. 2021 00:21 schreef Jessica Hershey @.***

:

Would this also work?

if(hourly_0["rain"]) { if(USER_PARAM.OPEN_WEATHER_UNITS == 2) { float temp = hourly_0["rain"]["1h"]; history[x].rainVolume = (temp/25.4); // 95 } else history[x].rainVolume = hourly_0["rain"]["1h"]; // 95

        } else history[x].rainVolume = 0; // if no rain data resets old to 0

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JHershey69/OpenWeatherOneCall/issues/35#issuecomment-905016040, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARKWG637RDRPSLFKM636ZYLT6QLQDANCNFSM5CXQTONA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

JHershey69 commented 3 years ago

Thanks. 3.1.3 updates all of that and I caught the incorrect snow value while fixing this. :)

PeterAckermans commented 3 years ago

Jessica

I am sorry to say that I made a mistake in copying a failing code segment in my bug report. I tested several options and somehow copied the wrong section into the bug report.

Now you copied that into the release.

The segment that worked for me is exactly the same as your propasal

if(hourly_0["rain"]) { if(USER_PARAM.OPEN_WEATHER_UNITS == 2) { float temp = hourly_0["rain"]["1h"]; history[x].rainVolume = (temp/25.4); // 95 } else history[x].rainVolume = hourly_0["rain"]["1h"]; // 95

}

else

history[x].rainVolume = 0; // if no rain data resets old to 0

Op do 26 aug. 2021 15:57 schreef Jessica Hershey @.***>:

Thanks. 3.1.3 updates all of that and I caught the incorrect snow value while fixing this. :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JHershey69/OpenWeatherOneCall/issues/35#issuecomment-906438116, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARKWG62LC4O3H7A24YDPYMLT6ZB6NANCNFSM5CXQTONA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

JHershey69 commented 3 years ago

I didn’t copy any of YOUR code, I just added the else statement to reset to 0. Looking at the src now it appears to be correct for the release 3.1.3  

PeterAckermans commented 3 years ago

Jessica

Found the issue in the new release. In the hourly history section for the added else statement you used the history[0] rain and snow values in stead of the history[x] value.

Peter

Op vr 27 aug. 2021 07:22 schreef Jessica Hershey @.***>:

I didn’t copy any of YOUR code, I just added the else statement to reset to 0.Looking at the src now it appears to be correct for the release 3.1.3 Sent from Mail for Windows From: Peter AckermansSent: Thursday, August 26, 2021 7:18 PMTo: JHershey69/OpenWeatherOneCallCc: Jessica Hershey; State changeSubject: Re: [JHershey69/OpenWeatherOneCall] Back to back historical data call returns wrong values (#35) JessicaI am sorry to say that I made a mistake in copying a failing code segmentin my bug report. I tested several options and somehow copied the wrongsection into the bug report.Now you copied that into the release.The segment that worked for me is exactly the same as your propasalif(hourly_0["rain"]){if(USER_PARAM.OPEN_WEATHER_UNITS == 2){float temp = hourly_0["rain"]["1h"];history[x].rainVolume = (temp/25.4); // 95}elsehistory[x].rainVolume = hourly_0["rain"]["1h"]; // 95}elsehistory[x].rainVolume = 0; // if no rain data resets old to 0Op do 26 aug. 2021 15:57 schreef Jessica Hershey ***@***.***>:> Thanks. 3.1.3 updates all of that and I caught the incorrect snow value> while fixing this. :)>> —> You are receiving this because you authored the thread.> Reply to this email directly, view it on GitHub> < https://github.com/JHershey69/OpenWeatherOneCall/issues/35#issuecomment-906438116>,> or unsubscribe> < https://github.com/notifications/unsubscribe-auth/ARKWG62LC4O3H7A24YDPYMLT6ZB6NANCNFSM5CXQTONA>> .> Triage notifications on the go with GitHub Mobile for iOS> < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>> or Android> < https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>> .>—You are receiving this because you modified the open/close state.Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android. Virus-free. www.avast.com — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub , or unsubscribe . Triage notifications on the go with GitHub Mobile for iOS or Android .
JHershey69 commented 3 years ago

fixed in 3.1.4

PeterAckermans commented 3 years ago

Tested successfully. Thanks.

Op za 28 aug. 2021 01:43 schreef Jessica Hershey @.***>:

fixed in 3.1.4

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JHershey69/OpenWeatherOneCall/issues/35#issuecomment-907527696, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARKWG6YJCDXYYFU5QMFXYK3T7APKFANCNFSM5CXQTONA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

JHershey69 commented 3 years ago

Thank you for reporting the initial bug.