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

Group entities by physical device #343

Merged
merged 20 commits into from
Feb 3, 2021
Merged

Group entities by physical device #343

merged 20 commits into from
Feb 3, 2021

Conversation

iMicknl
Copy link
Owner

@iMicknl iMicknl commented Jan 14, 2021

Functionality from #328 by @vlebourl

@iMicknl
Copy link
Owner Author

iMicknl commented Jan 14, 2021

@vlebourl could you give this a try? I don't have any devices that have multiple entities...

@vlebourl
Copy link
Collaborator

@vlebourl could you give this a try? I don't have any devices that have multiple entities...

I'll try that tomorrow! But looks good to me.

@vlebourl
Copy link
Collaborator

that works for me!
image

vlebourl
vlebourl previously approved these changes Jan 15, 2021
@iMicknl
Copy link
Owner Author

iMicknl commented Jan 15, 2021

@vlebourl I see that I need to copy the device info as well, looks like it is now displaying the last one.

@vlebourl
Copy link
Collaborator

@vlebourl I see that I need to copy the device info as well, looks like it is now displaying the last one.

ohyeah... missed that one

@vlebourl
Copy link
Collaborator

@vlebourl I see that I need to copy the device info as well, looks like it is now displaying the last one.

ohyeah... missed that one

That's why somewhere I retrieve the base_url#1 label to get the device info.

@iMicknl
Copy link
Owner Author

iMicknl commented Jan 15, 2021

@vlebourl I see that I need to copy the device info as well, looks like it is now displaying the last one.

ohyeah... missed that one

That's why somewhere I retrieve the base_url#1 label to get the device info.

Did this work in your previous PR?

@vlebourl
Copy link
Collaborator

vlebourl commented Jan 15, 2021

hmm scratch that, that's actually the model part in the return of tahoma_device device_info. We could set it to name as well. Or find something else relevant.

@iMicknl
Copy link
Owner Author

iMicknl commented Jan 15, 2021

@vlebourl we can pull model and firmware from our parent device (#1) as well. I am not able to test it, but what if we just leave it empty for devices not being #1? Could you try that if I add it to this pr?

@vlebourl
Copy link
Collaborator

only #1 info shows up now:
image
that's fine IMO

@vlebourl
Copy link
Collaborator

but firmware is wrong...

@iMicknl
Copy link
Owner Author

iMicknl commented Jan 19, 2021

@vlebourl could you perhaps give the latest version a try? If this doesn't work, I am not sure how to solve it :-). It is hard to fix without having the device present.

@iMicknl
Copy link
Owner Author

iMicknl commented Jan 27, 2021

While removing old branches, I came across #152. Was this simple approach not good enough? :D

@tetienne
Copy link
Collaborator

@iMicknl If I remember well, it was because the name of the device was not correct. But indeed, the amount of modification between this PR and mine are not the same at all :)
@lebourl how dit you solve this: https://github.com/iMicknl/ha-tahoma/pull/152/files#r455789429 ?

@iMicknl
Copy link
Owner Author

iMicknl commented Jan 27, 2021

@iMicknl If I remember well, it was because the name of the device was not correct. But indeed, the amount of modification between this PR and mine are not the same at all :)

Ahh, that is indeed solved in this PR. The amount of code is indeed more, but it is not that much. Most of the code is around retrieving the entity id / name of the original device.

@iMicknl
Copy link
Owner Author

iMicknl commented Jan 31, 2021

@vlebourl could you perhaps give this PR a try? I am not able to test it and would be good to check before merge.

custom_components/tahoma/tahoma_device.py Outdated Show resolved Hide resolved
custom_components/tahoma/tahoma_device.py Outdated Show resolved Hide resolved
@iMicknl iMicknl merged commit 7a72c75 into master Feb 3, 2021
@iMicknl iMicknl deleted the feature/merge_subdevices branch February 3, 2021 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants