Add ConfigFlow for SPC

[imduffy15]

Breaking change

Not yet but YAML support for SPC should be dropped in the future.

Proposed change

• Add config flow for SPC with automatic migration from YAML configuration
• Add unique_ids to the sensors and alarm panels created by SPC
• Add _attr_code_arm_required = false to match reality and allow arming from alexa
• Upgrade to pyspcwebgw 0.6 release notes at https://github.com/pyspcwebgw/pyspcwebgw/releases

Type of change

• [x] Dependency upgrade
• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New integration (thank you!)
• [x] New feature (which adds functionality to an existing integration)
• [ ] Deprecation (breaking change to happen in the future)
• [ ] Breaking change (fix/feature causing existing functionality to break)
• [x] Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• [x] The code change is tested and works locally.
• [x] Local tests pass. Your PR cannot be merged unless tests pass
• [x] There is no commented out code in this PR.
• [x] I have followed the development checklist
• [x] The code has been formatted using Black (black --fast homeassistant tests)
• [x] Tests have been added to verify that the new code works.

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

• [?] Documentation added/updated for www.home-assistant.io

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

• [x] The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• [x] New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• [x] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• [x] Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• [ ] I have reviewed two other open pull requests in this repository.

[imduffy15]

The test test_update_alarm_device fails with the introduction of configflow, I'm unsure how to fix it and would appreciate input.

[imduffy15]

https://github.com/pyspcwebgw/pyspcwebgw/releases shows the changes for pyspcwebgw, nothing major, bumped it to get the version information to make the device entry look nice.

[home-assistant[bot]]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

[imduffy15]

It seems you are missing to create an issue on the migration of yaml and also async_setup_entry seems missing in atleast alarm_control_panel so it would never load.

Hey @gjohansson-ST Thanks for taking a look really appreciate it, new to the code base.

Can create an issue that is no problem.

I'm confused by the second bit, isn't the following responsible for creating the alarm panel? It appeared when running locally.

'''

# create a separate alarm panel for each area
hass.async_create_task(
    discovery.async_load_platform(
        hass, Platform.ALARM_CONTROL_PANEL, DOMAIN, {}, hass.data[DOMAIN]
    )
)

'''

I'm very confused as to why the pytest fails. If I am following correctly it uses the yaml setup and waits for any async tasks to create. The async task to run the aync_setup_entity should run and create the alarm panel and sensors but this doesn't seem to occur in the tests.

[gjohansson-ST]

I'm confused by the second bit, isn't the following responsible for creating the alarm panel? It appeared when running locally.

Wasn't looking careful enough apparently. With the implementation of a config flow to make it UI based you should import any YAML to a config entry and use it to load the integration from their on. This PR would not be approved based on having both UI and YAML setup.

[imduffy15]

Hi @gjohansson-ST unless im misunderstanding something the code is already in place to do the import:

async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
    """Set up the SPC component."""
    if DOMAIN in config:
        hass.async_create_task(
            hass.config_entries.flow.async_init(
                DOMAIN, context={"source": SOURCE_IMPORT}, data=config[DOMAIN]
            )
        )

    return True

Starts the configflow import process.

Tested this locally by creating yaml and seeing if a config entity was created... it was.

[imduffy15]

[epenet]

Notes:

Add config flow for SPC with automatic migration from YAML configuration Add unique_ids to the sensors and alarm panels created by SPC Add _attr_code_arm_required = false to match reality and allow arming from alexa Upgrade to pyspcwebgw 0.6 release notes at https://github.com/pyspcwebgw/pyspcwebgw/releases

Should these not be four separate PR? Config flow tends to be a big PR with quite a lot of back-and-forth, but the other three could probably be 3 preliminary or follow-up PRs.

[imduffy15]

Notes:

Add config flow for SPC with automatic migration from YAML configuration

Add unique_ids to the sensors and alarm panels created by SPC

Add _attr_code_arm_required = false to match reality and allow arming from alexa

Upgrade to pyspcwebgw 0.6 release notes at https://github.com/pyspcwebgw/pyspcwebgw/releases

Should these not be four separate PR?

Config flow tends to be a big PR with quite a lot of back-and-forth, but the other three could probably be 3 preliminary or follow-up PRs.

Happy to just abandon this. Contributing is becoming too difficult.

[epenet]

Notes:

Add config flow for SPC with automatic migration from YAML configuration

Add unique_ids to the sensors and alarm panels created by SPC

Add _attr_code_arm_required = false to match reality and allow arming from alexa

Upgrade to pyspcwebgw 0.6 release notes at https://github.com/pyspcwebgw/pyspcwebgw/releases

Should these not be four separate PR? Config flow tends to be a big PR with quite a lot of back-and-forth, but the other three could probably be 3 preliminary or follow-up PRs.

Happy to just abandon this. Contributing is becoming too difficult.

config-flow is quite often the most difficult - that's why I'm suggesting that you move the other code changes to small individual PRs that can be reviewed and merged independently.

[imduffy15]

Notes:

Add config flow for SPC with automatic migration from YAML configuration

Add unique_ids to the sensors and alarm panels created by SPC

Add _attr_code_arm_required = false to match reality and allow arming from alexa

Upgrade to pyspcwebgw 0.6 release notes at https://github.com/pyspcwebgw/pyspcwebgw/releases

Should these not be four separate PR?

Config flow tends to be a big PR with quite a lot of back-and-forth, but the other three could probably be 3 preliminary or follow-up PRs.

Happy to just abandon this. Contributing is becoming too difficult.

config-flow is quite often the most difficult - that's why I'm suggesting that you move the other code changes to small individual PRs that can be reviewed and merged independently.

We can accept as is or I can close.

Contributing to home assistant has become far too difficult - and I say that as someone who is a PMC for an Apache foundation project. At this point it is easier to maintain my own fork and rebase on each release.

While I understand the ask here is small we are 15 days into this review with ~40 comments having being shared.

Additionally, when attempting to make small changes to components with configflow in the past they have been rejected. https://github.com/home-assistant/home-assistant.io/pull/15793

Test coverage is there. Review feedback has been implemented. Cla has been signed. All ci is passing. Recording demonstrating the functionality has been provided. This can be accepted as is or I can close the pr.

Let me know how you wish to proceed.

[epenet]

See https://developers.home-assistant.io/docs/review-process#creating-the-perfect-pr

Make your PRs as small as possible. A PR should only refactor one thing, fix one thing, add one feature, or adjust a single subject in the documentation. If you want to change multiple things, please create multiple PRs. Smaller PRs have a smaller scope, need less time to review, conflict less often, and generally need fewer review iterations.

Only change one thing at a time. This is the same as the previous point but a bit more specific. It is tempting to improve those one or two lines you've noticed nearby, but please don't. Put those in a separate PR. Unrelated changes in your PR are distracting and often lead to questions. In contrast, in an independent PR, it would be a quick and simple review and merge.

Anyway I will not interfere more with the review from @gjohansson-ST

[imduffy15]

Closing this as its gone stale and now fails code coverage. Not putting any more time into it.

Thanks for destroying the PR @epenet :/