Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support adding and removing entities to devices #191

Merged
merged 2 commits into from
Dec 17, 2020
Merged

Conversation

postlund
Copy link
Collaborator

This is not what I should be working on, but I had most of the code sitting on a branch so I decided to finish it.

New entities can be added via + button under Integrations (like when adding a new device, but select an existing device instead) and entities can be removed from Options.

@ultratoto14
Copy link
Collaborator

Hi @postlund, just tested it, here are my remarks:

  • When adding a new entity to an existing one, the already used DP ids are proposed.
  • At the end the label is not what is expected, "Abandonné" means "canceled"
    Canceled
  • At the same time i have an exception in the logs, but the entity was well created:
2020-11-26 10:53:01 ERROR (MainThread) [homeassistant.config_entries] Error unloading entry Tempo for sensor
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 321, in async_unload
    result = await component.async_unload_entry(hass, self)  # type: ignore
  File "/usr/src/homeassistant/homeassistant/components/sensor/__init__.py", line 72, in async_unload_entry
    return await hass.data[DOMAIN].async_unload_entry(entry)
  File "/usr/src/homeassistant/homeassistant/helpers/entity_component.py", line 172, in async_unload_entry
    raise ValueError("Config entry was never loaded!")
ValueError: Config entry was never loaded!
  • When trying to add a new entity, yaml defined entries are proposed, if i choose one i have an error, nothing in the logs
    Error

@postlund
Copy link
Collaborator Author

Thanks for testing @ultratoto14! Let's see if I can answer...

Hmm, that's strange. Is it always like that or only when you add a new entity to a device that you added during the same "session"? Because I know that the browser sometimes cache values and fill those in for some reason. I at least don't have any code for auto-filling values (that's part of #125, which isn't merged yet).

That's a limitation of the entry flow framework. There's only two ways out: creating a new config entry or aborting (yielding the "canceled" message). Since we already have a config entry, that leaves out the first one (which also would request the user to assign the device to a room again), leaving only the abort method. Fact is that this is the de-facto way of doing things like this, e.g. when reconfiguring credentials for a device. So you will see it in the built in components as well.

The exception I don't know the reason for, will have to look into that.

Right, good catch. I will make sure to filter out devices set up via YAML.

@ultratoto14
Copy link
Collaborator

ultratoto14 commented Nov 26, 2020

Hmm, that's strange. Is it always like that or only when you add a new entity to a device that you added during the same "session"? Because I know that the browser sometimes cache values and fill those in for some reason. I at least don't have any code for auto-filling values (that's part of #125, which isn't merged yet).

Hi @postlund, I remember that the filtered already used DP is only for the ID, it was on a PR already merged. So it's normal then, I set up a light with multiple DP used and expect that these will not be present in the dropdown list, but this applies only to ID, my bad.

@ultratoto14
Copy link
Collaborator

@postlund should I retest this one ?

@postlund
Copy link
Collaborator Author

@ultratoto14 Yes please, forgot to ping you about it.

Copy link
Collaborator

@ultratoto14 ultratoto14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good 👍
Tests are Ok:

  • Adding new device 👍
  • Adding entity to device 👍
  • Removing entity from device 👍
  • YAML devices not listed 👍
  • Already configured and not yet configured are there in list 👍

@postlund
Copy link
Collaborator Author

Great! 👍 Let's merge before I charge my mind.

New entities can be added via + button under Integrations (like when
adding a new device) and entities can be removed from Options.
@postlund postlund force-pushed the add_remove_entities branch from 97009c4 to 86d1887 Compare December 17, 2020 18:34
@postlund postlund requested a review from ultratoto14 December 17, 2020 18:49
@postlund postlund merged commit 654c6cd into master Dec 17, 2020
@postlund postlund deleted the add_remove_entities branch December 17, 2020 20:36
@postlund
Copy link
Collaborator Author

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants