search

Pirate-Weather / pirate-weather-ha

Replacement for the default Dark Sky Home Assistant integration using Pirate Weather
https://pirateweather.net/
Apache License 2.0
369 stars 26 forks source link

Does having multiple integrations setup result in multiple calls to the API? #243

Closed cloneofghosts closed 5 months ago

cloneofghosts commented 5 months ago

Yea, that must be it. The entities and forecast both use the same data update coordinator, so share API calls, but I suspect that rogue "8 entities" one is the issue here. Can you try deleting that, as well as (or commenting out) your YAML, and we'll see what that does.

Hi @alexander0042 - could you please clarify this a bit more? I previously had my integration set up similarly to @mermelmadness with one instance for Entities and another for Weather. Honestly, I can't recall why I had done that. But when I ran into an API Limit issue this month, @cloneofghosts suggested (#218) consolidating to a single instance of the integration. My conclusion from that was that the former configuration was causing twice the API calls of the latter. Is that how it works?

Originally posted by @jhemak in https://github.com/Pirate-Weather/pirate-weather-ha/issues/221#issuecomment-2087406325

cloneofghosts commented 5 months ago

Hi @jhemak it looks like your comment got missed in the other thread so I created this issue for you. I'll ping @alexander0042 to confirm whether having multiple integrations setup results in multiple calls to the API.

I also tagged this as documentation as it should be added to the FAQ section in the HA documentation.

jhemak commented 5 months ago

Thanks!

alexander0042 commented 5 months ago

Thanks for opening the issue, it really helps to improve this integration!

The overall goal is that there should only be one "Data Update Coordinator" for a given location, and adding additional instances of the integration for the same location should use the existing one. However, (and I'd forgotten this), a while ago I apparently disabled this check, which was sometimes stopping the integration from updating....

Looking at the code, it seems like it should create a single weather update coordinator for a given location, so I think the issue is that it's not checking if one already exists. I originally removed the check because it wasn't re-linking when options were changed, and didn't matter since daily and hourly weather entities had been merged; however, wasn't thinking about additional sensor entities. I looked around at a few other integrations and none of them that sort of check either, so probably not a good idea to put back in.

Long story short, I think this is one for the docs. We should be clear that every time an additional integration is added, it's another set of hits to the API. However, as long as they're configured in the same integration (even using the configuration options after), it should be ok. I think this is reasonably intuitive, so shouldn't be too hard to clarify.

jhemak commented 5 months ago

Thanks guys!