Skip to content

Fixes and enhancements for the Tahoma platform #11547

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.

Sign up for GitHub

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

Merged
pvizeli merged 5 commits into from
Jan 13, 2018
Merged

Fixes and enhancements for the Tahoma platform #11547

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
963b34e
Strip off the RTS/IO ID from the entity ID
glpatcern Jan 6, 2018
ca38711
Ignore exception thrown when the device does not provide an active state
glpatcern Jan 6, 2018
7da2781
Send actions with a label for easier identification in the Tahoma log
glpatcern Jan 6, 2018
5a86b19
Linting
glpatcern Jan 9, 2018
1f0e30a
Strip off the RTS/IO ID from the entity ID, take 2
glpatcern Jan 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions homeassistant/components/cover/tahoma.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,13 @@ class TahomaCover(TahomaDevice, CoverDevice):
def __init__(self, tahoma_device, controller):
"""Initialize the Tahoma device."""
super().__init__(tahoma_device, controller)
self.entity_id = ENTITY_ID_FORMAT.format(self.unique_id)
eid = ENTITY_ID_FORMAT.format(self.unique_id)
# strip off the RTS or the IO ID from the entity ID
if eid.rfind('_rts') > 0:
eid = eid[:eid.rfind('_rts')]
elif eid.rfind('_io') > 0:
eid = eid[:eid.rfind('_io')]
self.entity_id = eid

@pvizeli pvizeli Jan 9, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you realy need to set the entity_id and don't let's auto generate from internal core/name ?

@glpatcern glpatcern Jan 9, 2018

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 point is that - as mentioned above - the current entity_id looks like <custom_name>_rts_<NNN>, where <NNN> is a very long numeric ID internally used by the TaHoma box to reference the shutters. This makes writing automations pretty unpractical (and the produced code unreadable).

IMHO Home Assistant should abstract from such platform-dependent details and expose high-level identifiers, so I believe it is definitely worth to override the auto-generated identifiers here.

@thijsdejong thijsdejong Jan 9, 2018

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.

@pvizeli Currently an entity id look like this: cover.bedroom_middle_io12345678901234567890 . Having cover.bedroom_middle would be more desireable, right?

@pvizeli pvizeli Jan 9, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes. Home-Assistant generate automatic entity_id with name. Remove the entity_id and the core do that automatic. For unique_id, you can only use the NN if they is unique.

@glpatcern glpatcern Jan 9, 2018

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.

So the question is about the pre-existing statement at L36, right? Do I understand correctly that leaving entity_id unassigned will make Home Assistant auto-generate an appropriate name?
There's no question about unique_id including the full identifier.

@glpatcern glpatcern Jan 9, 2018

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.

Answering to my own question: yes. Even better, the patch will be to remove L36 altogether.


def update(self):
"""Update method."""
Expand All @@ -46,12 +52,16 @@ def current_cover_position(self):

0 is closed, 100 is fully open.
"""
position = 100 - self.tahoma_device.active_states['core:ClosureState']
if position <= 5:
return 0
if position >= 95:
return 100
return position
try:
position = 100 - \
self.tahoma_device.active_states['core:ClosureState']
if position <= 5:
return 0
if position >= 95:
return 100
return position
except KeyError:
return None

def set_cover_position(self, position, **kwargs):
"""Move the cover to a specific position."""
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/tahoma.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,4 @@ def apply_action(self, cmd_name, *args):
from tahoma_api import Action
action = Action(self.tahoma_device.url)
action.add_command(cmd_name, *args)
self.controller.apply_actions('', [action])
self.controller.apply_actions('HomeAssistant', [action])