Filter rfxtrx configure devices option flow on existing config entry

[RobBie1221]

Breaking change

Proposed change

In some exceptional cases a device can be present in the device registry but there could be a missing config entry key for that device. In that case, when selecting the device and trying to configure options, the option flow will throw an exception.

This PR will not present devices in the list with devices to configure unless it has a config entry key.

The way to solve it when a device misses an entry key (one of the options will work):

1. Remove it (when not needed anymore)
2. The event code is detected through automatic add, the config entry key is added automatically
3. Manually enter a valid event code for the device

Type of change

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

Example entry for configuration.yaml:

# Example configuration.yaml

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request: Add warning to notify users how to fix missing device in options flow home-assistant.io#15331

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• The code has been formatted using Black (black --fast homeassistant tests)
• 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:

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

The integration reached or maintains the following Integration Quality Scale:

• No score or internal
• 🥈 Silver
• 🥇 Gold
• 🏆 Platinum

To help with the load of incoming pull requests:

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

[probot-home-assistant]

Hey there @Danielhiversen, @elupus, mind taking a look at this pull request as its been labeled with an integration (rfxtrx) you are listed as a codeowner for? Thanks!
_{^{(message by CodeOwnersMention)}}

[elupus]

We probably should have a test for this case. It's easy to miss supporting.

[elupus]

Also, any call to get_rfx_object should really check the return value. It can be Null if the event code was not parsable.

[elupus]

I noticed something else.

Here device_id returned from _get_device_data is assumed to be a string. But else where you assume it's tuple since you join it by "_". Also strange constant use for dispatcher.

new_device_data = self._get_device_data(replace_device)
new_device_id = "_".join(x for x in new_device_data.device_id)

device_data = self._get_device_data(entry_id)
event_code = device_data[CONF_EVENT_CODE]
device_id = device_data[CONF_DEVICE_ID]

self.hass.helpers.dispatcher.async_dispatcher_send(
                        f"{DOMAIN}_{CONF_REMOVE_DEVICE}_{device_id}"
                    )

[RobBie1221]

We probably should have a test for this case. It's easy to miss supporting.

I though about that but it's not straightforward. We need to mock a situation where a device is added but not in config entry, which cannot happen anymore so it needs some creativity.
From test cases I've seen, the schema to present options is not validated. Do you know how to validate that?

Also, any call to get_rfx_object should really check the return value. It can be Null if the event code was not parsable.

I did that initially in the options flow PR, and threw an error when not parsable, and got the comment how that could happen and how the user can prevent that. Based on that I removed it, because it cannot happen except when the user enters an event code. In this case return value is checked. In all other cases, the get_rfx_object already succeeded before because otherwise it can't end up in the config entry data.

[RobBie1221]

I noticed something else.

Here device_id returned from _get_device_data is assumed to be a string. But else where you assume it's tuple since you join it by "_". Also strange constant use for dispatcher.

new_device_data = self._get_device_data(replace_device)
new_device_id = "_".join(x for x in new_device_data.device_id)

device_data = self._get_device_data(entry_id)
event_code = device_data[CONF_EVENT_CODE]
device_id = device_data[CONF_DEVICE_ID]

self.hass.helpers.dispatcher.async_dispatcher_send(
                        f"{DOMAIN}_{CONF_REMOVE_DEVICE}_{device_id}"
                    )

About the device id, I need to recheck.

I think we can get rid of the whole dispatcher thing and refactor this a bit. When replacing the device, we directly remove entities from the entity register, which also calls async_remove on the entity, so I think we should do the same here and perhaps make a function remove_device which does it.

[RobBie1221]

Refactoring the dispatcher should be done in a different PR.

I'm still not sure about adding a test. I can't get a test working which asserts on a Vol.In schema (it's always failing, don't know how to get it working). I think I already spent more time on it than it's worth, it's not a typical use case.

See previous comments about the rfx object check.

[elupus]

Okey. I might have a go, but I don't think we need to block this pull.

[MartinHjelmare]

Shouldn't we clean up devices with incorrect data state automatically?

[RobBie1221]

I thought about that, but I worry it'll be frustrating for users. The devices for which this happens have entities which are RestoreEntity. Potentially a user changed names and entity ids for these devices/entities and we should avoid users having to re-add them and re-name them again.

[RobBie1221]

Do note also that for devices which can receive (data or command) it will be automatically fixed. It will be detected that the event code is not in the ConfigEntry data and it will be added. We also risk if the user launches the options flow too soon that we cleanup devices for which data is only received sporadically and config can still be fixed.

[MartinHjelmare]

Ok.