Fixes and enhancements for the Tahoma platform#11547
Conversation
| return 100 | ||
| return position | ||
| try: | ||
| position = 100 - self.tahoma_device.active_states['core:ClosureState'] |
|
I think stripping off the entity id is a good choice. It's currently incredibly messy since it adds 24+ numbers that are already in 'tahoma_device_id' 👍 |
| eid = eid[:eid.rfind('_rts')] | ||
| elif eid.rfind('_io') > 0: | ||
| eid = eid[:eid.rfind('_io')] | ||
| self.entity_id = eid |
There was a problem hiding this comment.
Do you realy need to set the entity_id and don't let's auto generate from internal core/name ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@pvizeli Currently an entity id look like this: cover.bedroom_middle_io12345678901234567890 . Having cover.bedroom_middle would be more desireable, right?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Answering to my own question: yes. Even better, the patch will be to remove L36 altogether.
|
This should now be all good, the travis CI failed only because of a timeout. |
|
Travis CI is failing because you're importing |
As per suggestions, let HA do the standard initialization and assign an appropriate entity_id, instead of overriding it with the lengthy unique_id coming from the TaHoma devices.
|
Good point, sorry to have overlooked it. Being at it, I completely removed the constructor and extended the commit message to explain why we just want to use the default behavior from HA. |
|
And yes, this time no timeouts and no warnings, so should be good to go. |
ghost
removed
the
6 participants
Description:
Hi there,
Related to PR #11538, here you find a few more fixes and enhancements. They all work on my installation, will follow up for the automated linting/CI tests.
A breaking change is commit 963b34e that changes the entity IDs in HA. For this, I appreciate some feedback from @philklei as the original author and @bakedraccoon as the contributor of the above-mentioned PR.
Related issue (if applicable):
Pull request in home-assistant.github.io with documentation (if applicable):
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests pass