bremor / bureau_of_meteorology

Custom component for retrieving weather information from the Bureau of Meteorology.
MIT License
190 stars 31 forks source link

Add integration configuration and device registry entries #147

Closed Djelibeybi closed 2 years ago

Djelibeybi commented 2 years ago

Adds the ability to reconfigure the integration via the integrations page and creates a device for each configured base name. Each device will contain the sensors that use that base name. Most sensors are set to diagnostic, so they won't appear on auto-generated dashboards by default, but they can be added by using the "Add to dashboard" option.

Signed-off-by: Avi Miller me@dje.li

Djelibeybi commented 2 years ago

I know this is a large PR and it's certainly larger than I'd like. I was halfway through adding the device stuff when I got tired of creating/removing the integration over and over, so I added the ability to reconfigure it at the same time, forgetting to create a new branch for that.

There's also a bunch of formatting changes made by Black which makes this look like a much larger change than it actually is. I didn't change any of the logic, just how it's stored and/or presented by Home Assistant.

Djelibeybi commented 2 years ago

Here are some screenshots of what it looks like on the Integrations page.

Integration box:

2022-07-17

Weather and forecasts device:

2022-07-17 (2)

Observations device:

2022-07-17 (3)

If you configure the same name for everything, only one device is created. If you configure different names for everything, four devices are created.

Makin-Things commented 2 years ago

Thanks for the effort. I don't have time today to look at it in detail but should find time in the next couple of days. Adding the reconfigure option will certainly be welcomed by a lot of people. I'll do some testing and if everything looks ok I'll create a pre-release and get a few other weather geeks to test it as well.

Djelibeybi commented 2 years ago

That sounds great. I didn't update README.md because I wasn't sure how you'd want to version this. Having a prerelease will certainly be appreciated.

Makin-Things commented 2 years ago

I got busy and forgot about this. I have tested it today and it looks good, except for one minor issue. The sensors should not be created as diagnostic. I think all that is needed to not have this happen is remove the entity_category function in sesnor.py (lines173-176). If you can make that change I will merge and create a pre-release for wider testing.

Djelibeybi commented 2 years ago

The sensors should not be created as diagnostic.

This was done on purpose based off the recommendations from this blog post: https://developers.home-assistant.io/blog/page/3?_highlight=category#entity-categories

By categorising the sensors as diagnostic, they do not appear by default on automatically generated dashboards, while remaining active. The only other option is config but that's for things that can be changed, which sadly, these are not.

If I remove the categorisation, users will either have to hide or disable each sensor to get it off their dashboard. As the use case for these sensors is more likely to be used as part of a template or dashboard card, hiding them by default seemed the better option.

Makin-Things commented 2 years ago

But they are not diagnostic entities. I have checked about 5 of the core weather integrations that have been updated to the service model and none of them use diagnostic sensors. Here is an example of one of them. Set to diagnostic for an entity exposing some configuration parameter or diagnostics of a device but does not allow changing image They will only ever get added to the dashboard if you are still using the auto thing you get at install (probably the first thing users disable after installing HA). Currently the the integration creates them as regular entities.

Djelibeybi commented 2 years ago

Fair enough. I've removed the category and rebased the PR to resolve the conflict.

Makin-Things commented 2 years ago

Thanks. I will merge and update the README and create a pre-release shortly.