naofireblade / homebridge-weather-plus

A comprehensive weather plugin for homebridge.
MIT License
315 stars 63 forks source link

Fix Tempest Weatherflow issues #273

Closed dacarson closed 1 year ago

dacarson commented 1 year ago

Fix the JSON parse issue when it is raining - evt_precip event tries to use a function that is not in scope. Fix #269, use the locationCity correctly to make a unique weather station serial number, and avoiding history information from being merged.

naofireblade commented 1 year ago

Hi, is this ready to merge?

dacarson commented 1 year ago

Yes, it is ready to merge. A cursory code review would also be appreciated :-)

wimleers commented 1 year ago

Sorry, didn't see https://github.com/naofireblade/homebridge-weather-plus/issues/269#issuecomment-1712153347 until now!

  1. I have only 1 weather station, so did not run into this bug. Fixing it by ensuring a unique serial number makes sense 👍
  2. I see that the bug I reported was diagnosed by you and also fixed here 👏

I see that the changeling says

  • Tempest: Fix JSON parse issue for rain events

… but that's specifically only for the bug I reported wrt getConditionCategory, right? Because another bug I've noticed is that it "forgets" the rain that happened in the last hour, although the total rain remains accurate. I've been unable to reproduce this for weeks now because it's been very dry.

Lucky me, it's raining non-stop here as of today 😂 ☔

☝️ As you can see: total rain keeps going up (✅ ), but rain of the last hour is simply forgotten/erased (🐛).

dacarson commented 1 year ago

Yes, I fixed the specific getConditionCategory JSON issue.

Can you file a new issue for the rain being forgotten. I can try to take a look today.

dacarson commented 1 year ago

Can you also turn on debugging because it looks like I log this line everytime I get a new observation event: this.log.debug("getHourlyAccumulatedRain last minute: " + mmOfRainInLastMinute + " last hour: " + accumulation);

dacarson commented 1 year ago

Also keep in mind that it is a moving window - it reports the rain that has fallen over the last 60mins. It is quite possible to go down. If it rained heavy 59 mins ago, then very little, I would expect the number to go down after 60 mins.

wimleers commented 1 year ago

Also keep in mind that it is a moving window

Yep, I realize that!

But in this case, it was raining heavily non-stop, so it should've gone up, not down, in the two screenshots I posted. When it's raining again, I'll turn on debug logging 👍

wimleers commented 10 months ago

@dacarson One new bug report: #275. No news on the rain yet because my battery is dead; just received the "Power Booster Accessory", still need to set it up. So I have to report the bug (and can test a fix) until I set that up.