Skip to content
This repository was archived by the owner on Oct 1, 2021. It is now read-only.

Pass the device name without mapping to the component#184

Merged
balloob merged 6 commits intohome-assistant-libs:masterfrom
syssi:feature/yeelight-ceiling4
Apr 8, 2018
Merged

Pass the device name without mapping to the component#184
balloob merged 6 commits intohome-assistant-libs:masterfrom
syssi:feature/yeelight-ceiling4

Conversation

@syssi
Copy link
Copy Markdown
Contributor

@syssi syssi commented Apr 4, 2018

No description provided.

@rytilahti
Copy link
Copy Markdown
Contributor

Is there a need to be so specific when matching for type? I think I would simply merge all those different versions into one, if there's no specific reason for that.

Or more broadly, is it even necessary to have this type information available at all?

@syssi
Copy link
Copy Markdown
Contributor Author

syssi commented Apr 4, 2018

The type will be important some day because every ceiling lamp has different features for example. But you are right... we shouldn't map the device type here. The full name should be passed to the component.

@syssi syssi changed the title mDNS prefix of the Yeelight Ceiling Light 4 (Jiaoyue 650) added Pass the device name without mapping to the component Apr 5, 2018
@syssi
Copy link
Copy Markdown
Contributor Author

syssi commented Apr 5, 2018

This PR will change the entity id of discovered yeelights:

https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/light/yeelight.py#L126-L128

We could introduce some legacy handling. Suggestions?

Comment thread netdisco/discoverables/yeelight.py Outdated
else:
logging.warning("Unknown miio device found: %s", entry)
device_type = \
entry.name.replace(DEVICE_NAME_PREFIX, '').rsplit('_', 1)[0]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why wouldn't we always do this? It seems unnecessary for us to make up our own device types instead of using the ones provided by Yeelink.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The device_type is part of the entity id if the device is auto-discovered. The old mapping provides stable entity ids.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing the previous mapping would be a breaking change for consistency.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It will only be a breaking change if not resolved in Home Assistant. I think that it is weird that a discovery library is introducing their own names for types. We should follow the vendor types.

@syssi
Copy link
Copy Markdown
Contributor Author

syssi commented Apr 8, 2018

I've removed the mapping and will introduce it at Home Assistant.

@balloob balloob merged commit 5b4ff1e into home-assistant-libs:master Apr 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants