Support Konnected Pro alarm panel, embrace async, leverage latest HA features/architecture

[kit-klein]

Description:

This PR updates the Konnected component to support the soon to be released Konnected Alarm Panel Pro. It also brings the Konnected component up to date with the latest HA architecture and features. Notable changes are...

• config flow and config entries support
• migrated discovery to use ssdp via config flows
• device registry support
• uses updated async konnected pypi module
• extracted the konnected alarm panel interface and state management into a separate module (panel.py).
• tests added for init, config flow, and panel modules

All changes are backward compatible with existing configuration.yaml based configs. The yaml based configs will be imported into a config entry if a matching entry does not exist.

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#11785

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated in home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
• Untested files have been added to .coveragerc.

If the code does not interact with devices:

• Tests have been added to verify that the new code works.

[probot-home-assistant]

This pull request needs to be manually signed off by @balloob before it can get merged.

[probot-home-assistant]

Hey there @heythisisnate, mind taking a look at this pull request as its been labeled with a integration (konnected) you are listed as a codeowner for? Thanks!

[MartinHjelmare]

This PR is too big to be reviewed in one piece. Please break it up into multiple sequential PRs. I suggest this order.

• Add tests, except for config flow.
• Use async interface.
• Refactor to panel.
• Implement config entry and flow support with tests for config flow.
• Implement device registry support.
• Migrate to use ssdp discovery.

[kit-klein]

@MartinHjelmare Understand the request. I'm concerned about the feasibility of breaking this into multiple PR's while maintaining functionality. Several of the changes have circular dependencies on others.

The previous discovery mechanism was integrated into the init module code which was migrated to the panel module. Utilizing ssdp/config flow was the main enabler for that refactor.

The new panel module provides the async support for the updated async konnected pypi module.

All the tests were written for the refactored code (anything touched in this effort received test coverage) - non existed before. The tests will not function with the previous codebase and adding them as an independent PR would require a lot of code that would be immediately discarded.

Is it possible to make an exception on reviewing this PR size knowing this is a one time large refactor that won't be repeated in future iterations of this component?

[MartinHjelmare]

The tests should not be written to be dependent on the implementation. If we use the core interfaces to set up the tests and don't interact with the entity directly, this will work, and we can change the implementation without changing the tests for the most part.

I still think my suggested order is the best. You might have to add intermediate code to make each step work independently. In total it will still be faster to do it this way. I know from experience.

Before doing anything though, we should get a green light from @balloob to work on this integration.

[MartinHjelmare]

Maybe change and do the refactor to panel last. I haven't looked at what that module does, so this part I'm not sure about.

[balloob]

This PR is ok to move on, but Martin is right. Please first upgrade the existing integration to use a config entry, then a followup PR to add support for the Pro panel.

[MartinHjelmare]

I'll do the review after Konnected offered to sponsor me for that particular work.

[MartinHjelmare]

I've started looking at the tests in general. As I said earlier we want to use the core interfaces. Patching should preferably be done in the periphery to make the tests robust for changes in our implementation.

I'll look at the rest now.

[MartinHjelmare]

Ok, I've looked at most of it now.

The main points are:

• Use unique_id api for config entries and flows. This will simplify a lot.
• A config flow should not store data in hass.data. The config flow should be contained besides being able to update config entry data via config entries api.
• All optional config options should be part of a separate options flow. We want the main flow to be minimal.
• Always use core interfaces to set up tests and assert state.

[kit-klein]

Going to address PR feedback via a sequence of commits.
1st: fix the formatting issues (import locations, f-strings, dicts, const)
2nd: Migrate the tests to utilize standard interfaces
3rd: Address issues with config flow and utilize options flow
4th: Replace the hass.data cache with config flow and HA state machine info

[MartinHjelmare]

It's ok to store things in hass.data, but it should not be updated from the config flow. We should eg create a new client in async_setup_entry in the component. The config flow will not be run if we only load existing entries.

[kit-klein]

@MartinHjelmare Commit 2683d28 updates the config flow to utilize the options flow and unique ID, along with addressing the test comments from the last PR. Adding extra tests and migrating sections of code is the culprit for most of the large diff. The main changes were

• moving strings defns from config to options
• moving config flow steps to option flow (and corresponding tests)
• rendering the step schema in each step instead of defining it as a constant (required to render default values based on current option settings)

I'm hitting one experience issue with the options flow. The frontend doesn't support showing titles and descriptions in the options flow like it does in config flows. You can see the differences in the links below.

https://github.com/home-assistant/home-assistant-polymer/blob/dev/src/dialogs/config-flow/show-dialog-options-flow.ts#L47

https://github.com/home-assistant/home-assistant-polymer/blob/dev/src/dialogs/config-flow/show-dialog-config-flow.ts#L53

Do you know if this is intentional? I'd like to confirm whether the options dialog could be enhanced to mimic the config flow dialog before I modify field names to replace the missing title/description.

[MartinHjelmare]

I don't know the frontend very well. @balloob?

[balloob]

Yeah that's a frontend thing. I think my initial thinking was that all options flows are going to be one page, so why even bother asking people to set a title. Nowadays we have a couple of flows with more pages.

PR to fix it: home-assistant/frontend#4598

[MartinHjelmare]

Latest changes look good! I'll do another round when the config validation is updated.

[MartinHjelmare]

Looks great!

[MartinHjelmare]

Are we ready to merge?

[kit-klein]

@MartinHjelmare ok to merge