Neato config flow#26579
Conversation
|
I'm not entirely sure what I'm doing, so please give me hints if I'm obviously doing something wrong 🙈 |
|
ha we were both working on this? lol were you looking at the changes I had in my branch? |
|
@dshokouhi Oh no. I wasn't aware that you were working on it. I don't know your branch either. How far are you? Does it make sense that we work together? |
|
@Santobert I got ti to work on importing config and setting up the flow. The tests I am not sure of and I think there may be somethings missing. Feel free to snoop around here :) https://github.com/dshokouhi/hass-neato-custom-component/tree/config_flow |
|
@dshokouhi Great, Thank you. May I ask why do you work on a custom component instead of the official integration? |
|
I always do custom components first to test with end users before I update to HA. I did this for testing neato fixes in the past so just keeping up the habit :) |
dshokouhi
left a comment
dshokouhi
left a comment
There was a problem hiding this comment.
Left some comments on a few things to address. One thing I am not sure about is if the components need to be updated for async_setup_platform and async_setup_entry. The below would replace line 14-21. Again not sure if its needed as the code works when I test it.
Here is an example from my camera.py file:
def async_setup_platform(hass, config, add_entities, discovery_info=None):
"""Set up the Neato Camera."""
pass
async def async_setup_entry(hass, entry, async_add_entities):
"""Setup the Neato camera using config entry."""
dev = []
for robot in hass.data[NEATO_ROBOTS]:
if 'maps' in robot.traits:
dev.append(NeatoCleaningMap(hass, robot))
_LOGGER.debug("Adding robots for cleaning maps %s", dev)
async_add_entities(dev, True)
dshokouhi
left a comment
dshokouhi
left a comment
There was a problem hiding this comment.
Looking a lot better now the form loads without the translation error too :) Couple more comments and I think it should be ready for the real review once CI passes and it needs tests for the config flow and a docs PR
dshokouhi
left a comment
dshokouhi
left a comment
There was a problem hiding this comment.
Just need to remove some of the changes as its out of scope for the config flow PR, we should fix these things in a subsequent PR. Its best to break things down because we already have a lot of changes.
|
@frenck I've updated the documentation here: I think we can remove that label now. |
dshokouhi
left a comment
dshokouhi
left a comment
There was a problem hiding this comment.
Looks good to me now, I think its ready for review from a member with merge rights now :)
|
@balloob @MartinHjelmare I did some research and found that the life360 integration also deletes and recreates the outdated entries. Personally, I think it's perfectly fine to do this. If a user changes things in configuration.yaml, he should expect that previous changes from the UI will be lost. |
MartinHjelmare
added
the
second-opinion-wanted
|
Not only will you lose changes from the UI, you also lose your entity ID customizations. This means that all your automations break, scripts break, scenes break. I can guarantee you that peope will be pissed ;) |
MartinHjelmare
removed
the
second-opinion-wanted
|
Can be merged when build passes. |
MartinHjelmare
left a comment
MartinHjelmare
left a comment
There was a problem hiding this comment.
Sorry, noticed the coverage wasn't 100%.
|
🎉 |
7 participants
Breaking Change:
Nothing's breaking. Neato can still be configured via configuration.yaml.
Changes in configuration.yaml will be noticed and the resulting configuration entry will be updated.
Description:
This PR adds config_flow to the neato integration.
Pending tasks:
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#10356
Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest.requirements_all.txtby runningpython3 -m script.gen_requirements_all..coveragerc.If the code does not interact with devices: