New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove resources selection from Nut config flow #59450
Remove resources selection from Nut config flow #59450
Conversation
Hey there @bdraco, mind taking a look at this pull request as it has been labeled with an integration ( |
@@ -48,25 +44,13 @@ def _base_schema(discovery_info): | |||
return vol.Schema(base_schema) | |||
|
|||
|
|||
def _resource_schema_base(available_resources, selected_resources): | |||
"""Resource selection schema.""" | |||
def _check_available_resouces(available_resources): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove this check. It's not the config flow responsibility to determine available sensors. The config flow should just validate the connection with the provided config data and then create the entry if the connection to the device or service is successful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could eventually create integration without entity. is this OK? Before it was not possible because user was forced to select at least 1 resource to complete the config flow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should create a device entry for the UPS explicitly if this isn't already done. Then there will always be at least a device entry. The entities is ok to not be any.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you follow up with a PR to add a device explicitly to the device registry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I lost this point. There is a reference to an integration that do the same, I never did it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's an example:
core/homeassistant/components/bond/__init__.py
Lines 72 to 81 in c03fdd5
device_registry = dr.async_get(hass) | |
device_registry.async_get_or_create( | |
config_entry_id=config_entry_id, | |
identifiers={(DOMAIN, hub.bond_id)}, | |
manufacturer=BRIDGE_MAKE, | |
name=hub_name, | |
model=hub.target, | |
sw_version=hub.fw_ver, | |
suggested_area=hub.location, | |
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we merge this PR, and save it for a follow up, or do you want to do it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please, merge it, I will create follow up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Proposed change
Remove the initial selection of Nut resources from the config flow. Only main resources will be automatically enabled, all the others are created in state disabled. Import of existing configuration is implemented.
Test has been changed to not consider anymore the
CONF_RESOURCES
settings.Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: