Basic cover support

[pfefferle]

Description

Basic implementation of cover_open, cover_close and cover_stop

Type of PR

If your PR fits more than one category, there is a high chance you should submit more than one PR. Please consider this carefully before opening the PR.
Either delete those that do not apply, or add an x between the square brackets like so: - [x]

• Bugfix
• Feature implementation
• Refactor of code (without functional changes)
• Documentation improvements
• Test improvements

Testing

Say "Can you close {Entity} please?" or "Can you open {Entity} please?"

[Tony763]

Will send you a PR with Czech translations, tonight.
Also, cover entity state could by read with sensor intent when domain is added, but here should be also intent to get position of cover (and tilt if supported) and set it. 'current_cover_position', 'current_cover_tilt_position'
https://developers.home-assistant.io/docs/core/entity/cover/

[Tony763]

Also we should think about a way to check if cover was really closed/opened as safety can be involed (Garage doors). That fuction should be universal, so we can use ot with other entties like locks.

[pfefferle]

Also we should think about a way to check if cover was really closed/opened as safety can be involed (Garage doors). That fuction should be universal, so we can use ot with other entties like locks.

hmmm... I am not sure about that, because none of my covers has a valid way to check if it really is open/closed...

[Tony763]

Each entity that is somehow activated return its state. For example switch. You can turn it on/off but also You can retrieve it's state:

skill-homeassistant/__init__.py

Line 458 in 2f8b7a4

ha_entity = self._find_entity(entity, ['sensor', 'switch'])

Now we just execute turn on/off action and say it's done if no error occur. My option: For covers and locks I would rater execute service, inform user we executed command and spawn some type of timeout which after done would read state of entity and confirm that command was un/successfully done.

This way it would be only up to HA component, how they handle security manner.

[pfefferle]

@Tony763 @krisgesling With the last commit, you can check the state of a cover. Is it possible to translate the open/close returned by home assistant?

[Tony763]

It would be fine to report open/opening/close/closing in selected language. I think I have seen a function for this somewhere. I will try to find it.

[Tony763]

Didn't find, I think, ti should be done as voc_match, but in reverse.

We would create 4 voc files with name of status and put translates states into them. Then when state is returned, we try to check if returned state match name of voc file and if yes, take random word from it (in case multiple words in voc). What do you think?

[krisgesling]

I think the preferred way would be to have an open.dialog and close.dialog then you could call:

self.translate(action)

docs for that method here

---

Alternatively you could have a comma separated listed of actions in an actions.value file then create a map of those values:

action_vocab = self.translate_namedvalues('actions')
close = action_vocab['close']

But yeah, the first option I think is better.

[pfefferle]

thanks @krisgesling !

what if there is no matching file? does it return the text param? I ask, because there are sensors, that return for example a string, that should not be translated (for example thermostats).

[Tony763]

You could limit it by entity domain.

[pfefferle]

@Tony763 makes sense!

[pfefferle]

@Tony763 @krisgesling I decided to use different dialogues for each state, because the sentence syntax differs between the states (at least in german), so the response is much nicer.

[Tony763]

Yes, that's usual. Dialogues in Czech are often different as direct 1:1 translation makes no sense.

[pfefferle]

@stratus-ss done!

[stratus-ss]

LGTM, pending testing

[pfefferle]

@Tony763 can you help? what do I have to do to fix the Resource not accessible by integration issues?

[Tony763]

Hi @pfefferle, sure, I have to go to work, now so I will check it tonight.

[Tony763]

@pfefferle create empty branch gh-pages and enable github pages.

[Tony763]

also change line 556 in __init__.py from self.speak_dialog('homeassistant.sensor.cover.%s' % sensor_state, data={ to self.speak_dialog('homeassistant.sensor.cover.{sensor_state}', data={ to fix pylint.

@pfefferle Faster than me :D I edited fixed line as there was whitespace before , and not after. With that, run in your fork will be ok, as all checks will pass. Tonight I will check why it is not working on PR as it should. If not, we can disable them on PRs and just check outcome on fork from where PR is.

[pfefferle]

;)

I fixed all lint errors in the cover and binary_sensor PRs. I will rework the GUI PR if/when the first two are merged.

[pfefferle]

@Tony763 on my "local" fork, it also works on branches. I think it breaks if you send PRs from remote branches. Maybe a security thingy.

[stratus-ss]

@krisgesling do you have any insight into the check that is failing?

[Tony763]

Hi @stratus-ss, please check PR #87. I covered there whats going on and fixed it.

[stratus-ss]

I have a cover defined

cover.double_garage_door

I don't seem to get any output from the skill because the cover is dumb and does not report its' state

Here is an entity test

turn on steve_s_desk_fan                                                                                                          
 >> turned on Steve's Desk Fan.                                                                                                 
 turn off steve desk fan                                                                                                          
 >> Steve's Desk Fan is now off.                                                                                                                     

here is the cover test output

can you close double garage door                                                                                                                    
 can you close double garage door         
 close the double garage door                                                                                                               
 open the double garage door   

The cover does indeed open and close, but there is no output from the skill

I am using a binary_sensor on the cover to determine its' open/close status since the cover itself does not report state

[pfefferle]

@stratus-ss thanks for testing!

You miss an output, like "{Entity} is going to open"?

The missing "real" state is a problem of the cover and/or Home Assistant (config). I mentioned it here: #80 (comment)

Not every cover reports the real state it is in. If I, for example, open my covers using Home Assistant, they simply change the state to open, if they receive the signal or not. So we have to be sure that the cover supports state reportings or not. Otherwise it can cause massive problems, at least if we handle it as a security feature.

[stratus-ss]

@stratus-ss thanks for testing!

You miss an output, like "{Entity} is going to open"?

Yes, some sort of feedback is what I expect. If I wasn't watching my garage door, I wouldn't know if Mycroft heard me, had a problem, was waiting on a slow internet connection, had a bad TTS problem or otherwise.

Consistent interaction is important. As existing commands give some sort of feedback, so should this PR

The missing "real" state is a problem of the cover and/or Home Assistant (config). I mentioned it here: #80 (comment)

Yes, I understand, I was confirming this. I was adding context in case someone reads this in the future

[pfefferle]

Should I keep the output vague? Something like: "trying to open {Entity}?"

[stratus-ss]

Should I keep the output vague? Something like: "trying to open {Entity}?"

There is the ideal and then the practical.

The ideal is to attempt to check the status of the cover. If, like in my case, the status comes back with nothing, is empty or throws an error then the response would be general.
If there is some sort of status then that would cause a different response.

Having said that, if you just want to push this through, the practical or MVP for this PR is just a vague response

[pfefferle]

I would start with the simple and vague solution, to keep the PR simple.

The problem is, that it always return a state and you never can get sure if it is really the state. At least I have not found a valid metric! So the only metric that I can think of is time. If the cover instantaneous changes the state it seems to be a "stateless" cover. Maybe this is something to report on the home assistant repo.

[stratus-ss]

Looks good to me. Mycroft now provides feedback when dealing with cover commands