siku2 / hass-srf-weather

SRF Meteo forecasts for Home Assistant
MIT License
8 stars 3 forks source link

Going backwards? #18

Open kds69 opened 1 year ago

kds69 commented 1 year ago

Hi, The latest version has now regressed partially: no hourly, no detail next 3 days... API 2.0 was working already for me. Am I missing a point with this release? Please let me know.

siku2 commented 1 year ago

@kds69 you're absolutely right. There were a few unintended regressions with the release. I wanted to refactor the code a bit and then re-integrate the changes you made, but I completely forgot about the latter... I also forgot to include the config entry migration.

Just to clarify one thing though: up until this release the integration was using v1 of the srf meteo api. This version is deprecated and will stop working sooner or later, so the main point of this release was actually to move to the new v2 api.

siku2 commented 1 year ago

Btw, the no hourly might have something to do with the new home assistant weather entity. The forecast is no longer exposed as an attribute (that's deprecated) and instead you provide two methods for consumers to get the hourly and daily forecast separately. The srf weather will actually provide the next 3 days in hourly resolution (and then the 3-hour resolution) for the hourly method.

If you use the default weather card and switch it to "hourly forecast" you should be able to see this. I have to admit I haven't played around with this a lot yet, but this is how weather entities are supposed to work going forward.

Can you confirm that this aligns with what you're seeing, @kds69 ?

kds69 commented 1 year ago

@siku2 thanks for the explanation. The integration gets no forecasts. No idea why. image

siku2 commented 1 year ago

The integration gets no forecasts. No idea why.

That's what I'm saying, it's no longer an attribute. You can't see it in the debug view. I'm surprised by this as well. If you're interested in the technical details, you can read up on it here: https://developers.home-assistant.io/docs/core/entity/weather/#weather-forecasts

siku2 commented 1 year ago

Just saw that the new HA version 2023.09 was released today. Be sure to try it out, @kds69. The changes to the forecast system are fully implemented now.

kds69 commented 1 year ago

you are absolutely right. There was indeed a timing issue in the various migrations (my HA got totally instable for 24h). Now I am good with the entity from your integration. Well done! I might need to add some new data that you skipped: snow and irradiance, if you are ok with it. A branch would work for me as well, but it should be quite straightforward so it is a bit overkilling. Please let me know.

(I am playing around with irrandiance to create my own solar forecast... which btw is now a bit cumbersome with weather data being in cache =I'll have to create automation to call a service to populate a specific irradiance sensor. There is even a petition open to roll back this new way to manage weather entities. Sooner or later, solar forecast will move to cache as well I presume. Not my battle).

siku2 commented 1 year ago

I might need to add some new data that you skipped: snow and irradiance, if you are ok with it.

yes, that's what I was referring to in my original reply. I didn't actually intend on removing those, but I completely forgot to add them back after the tedious process of refactoring the code :)

(I am playing around with irrandiance to create my own solar forecast... which btw is now a bit cumbersome with weather data being in cache =I'll have to create automation to call a service to populate a specific irradiance sensor

I know where you're coming from, but give it some time. Btw, I'm more than happy to add a separate sensor for this to the integration as well. I have my own irradiance sensor, so I never really required it from an api, but it never hurts to have multiple options :)

siku2 commented 1 year ago

Alright, I went ahead and added the missing fields real quick. Before I release it, mind giving feedback if this is what you wanted, @kds69?

https://github.com/siku2/hass-srf-weather/blob/93bc29f3981837accfa9ccd3f3c8a41acebcf8e1/custom_components/srf_weather/weather.py#L210-L224

These fields will be available in the forecast as well as in the attributes of the weather entity.

kds69 commented 1 year ago

yes! I looked quickly into the code, saw the class extension as per above but I can't relay how extra data gets into entity while it doesn't seems to be coded (yet?) in _set_forecast_now.

siku2 commented 1 year ago

yes! I looked quickly into the code, saw the class extension as per above but I can't relay how extra data gets into entity while it doesn't seems to be coded (yet?) in _set_forecast_now.

The trick is this part here: https://github.com/siku2/hass-srf-weather/blob/93bc29f3981837accfa9ccd3f3c8a41acebcf8e1/custom_components/srf_weather/weather.py#L147-L152

all the keys that are part of the "srf extra" set are copied over to the extra state attributes. But let me just create a new release, that way you can check it out directly.

kds69 commented 1 year ago

wow! Very class..y ;) Too advanced for me!

kds69 commented 1 year ago

Still a little issue: 3-hours forecasts are loaded as hourly. Why nit but it is then quite annoying when templating sensors with get_forecast service. Can’t exclude them. Not sure they can be tagged freely though, like 3_hours instead of hourly. Or do what I did previously, skip hourly records after 48hrs as this is science fiction to rely on hourly anything, and let 3 hours one kick in from there.

Also realizing scan interval is too short: 15mn = api used 96x a day > 50x for freemium… may this be every 30mn, which is more than decent obviously

siku2 commented 1 year ago

Not sure they can be tagged freely though, like 3_hours instead of hourly.

No, hass only supports hourly, twice-daily, and daily forecasts. There is still a bug in there though, the forecasts shouldn't skip back in time when the 3 hour ones start appearing. Instead, 1 hour forecasts should be used until there aren't any more of them and then it should use the 3 hour ones from that point onward.

Additionally, I can add an additional key to the forecasts indicating whether it's a 1 or 3 hour one.

Also realizing scan interval is too short: 15mn = api used 96x a day > 50x for freemium… may this be every 30mn, which is more than decent obviously

That's not what's happening. The integration is updating its internal state every 15 minutes, yes, but the actual API requests are performed based on the rate limits reported by the API. The logic for this is here:

https://github.com/siku2/hass-srf-weather/blob/8d96dbe303216e5b35df26d06ff23df3a7f16aba/custom_components/srf_weather/coordinator.py#L30-L48

kds69 commented 1 year ago

ok