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

WIP: Support for sections for Fibaro hub integration #18981

Closed
wants to merge 1 commit into from

Conversation

pbalogh77
Copy link
Contributor

Description:

Support for naming devices based on section + room + device name for Fibaro hub integrations. This can be useful for people with large houses.

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>

Example entry for configuration.yaml (if applicable):

[fibaro]
    sections: true

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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

@homeassistant homeassistant added integration: fibaro small-pr PRs with less than 30 lines. labels Dec 3, 2018
@ghost ghost added the in progress label Dec 3, 2018
@@ -54,6 +55,7 @@
vol.Required(CONF_USERNAME): cv.string,
vol.Required(CONF_URL): cv.url,
vol.Optional(CONF_PLUGINS, default=False): cv.boolean,
vol.Optional(CONF_SECTIONS, default=False): cv.boolean,
Copy link
Member

Choose a reason for hiding this comment

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

Why does it need to be configurable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you have a big house, it's nicer to have the section names appended, so you don't end up with two devices with the friendly name being "Bathroom lights". But if you don't have a big house, it's more a drag, because every device will be like "First Floor Bathr..." in most views. Once I added section support, I realized that I don't like it myself, but heard back from people who would appreciate it. So it's a tough call. I don't like too many options either.

Copy link
Member

Choose a reason for hiding this comment

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

After unique id feature, users can quite easily change name and entity_id. Do you still think we need to be able to turn sections off?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would have it off by default, but someone with a big house usually would have 100+ devices, renaming those would be a major pain. This option also affects the unique id, to keep the lovelace ui definition file readable as well. I really can see both camps going to a lot of pain and frustration without this flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once we'll have area support, I'll map the whole section/room info into areas and then the device name can be the device name. For now, I think it's still the best option however.

@pvizeli
Copy link
Member

pvizeli commented Dec 5, 2018

That end in a breaking change after home-assistant/architecture#94 is implemented. Let's invest the time, not in workarounds and try to apply this architecture change. More options also not help to achieve it later to config flow or make it user-friendly.

If you implement the unique_id, you can self-rename device over the UI...

@pbalogh77 pbalogh77 changed the title Support for sections for Fibaro hub integration WIP: Support for sections for Fibaro hub integration Dec 5, 2018
@pbalogh77
Copy link
Contributor Author

@pvizeli I've put it on hold for now. I've implemented unique ID in a new PR. I'll do the config flow implementation first and come back to this after that. Thanks for the feedback from you and @MartinHjelmare

@northhacks

This comment has been minimized.

@balloob
Copy link
Member

balloob commented Feb 6, 2019

Areas have been implemented now, I don't think this PR is needed anymore?

@robbiet480
Copy link
Member

Closing this PR as we now have areas.

@robbiet480 robbiet480 closed this Mar 25, 2019
@ghost ghost removed the in progress label Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration: fibaro small-pr PRs with less than 30 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants