MycroftAI / skill-homeassistant

Mycroft Skill/Integration for Homeassistant
GNU Lesser General Public License v3.0
115 stars 62 forks source link

remove optional `thermostat` keyword #78

Closed pfefferle closed 2 years ago

pfefferle commented 2 years ago

Description

Remove optional thermostat keyword from sensor.intent to:

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]

pfefferle commented 2 years ago

@krisgesling can you check this PR please.

krisgesling commented 2 years ago

Hmmm, I don't use thermostats at all - @stratus-ss do you use this?

"thermostat" seems like a very North American thing - we just have our tropical geographic temp set to 32 degrees all year round :laughing:

At the very least it seems like a strange way to phrase it. I would say:

What is the value of the kitchen thermostat What is the value of the thermostat in the kitchen

Not:

What is the value of the thermostat kitchen

pfefferle commented 2 years ago

I agree with your examples, but the phrase seems to be as generic as possible, so that you can ask for the status of lights, thermostats, binary_sensors, covers, ... in the same way.

I would vote for keeping it like it is for now (and remove thermostat to be consistant) and maybe work on more specific phrases for the different sensors in a separate PR.

stratus-ss commented 2 years ago

I don't use this specific function, however thermostat is a common thing in these parts.

I don't have any objection to making this more generic.

FWIW, I am more likely to say "what is the temperature set to" or "what is the temperature in the house".

I might ask what the state of the thermostat is if I wanted to know if it was off, set to air conditioning or heating

pfefferle commented 2 years ago

FWIW, I am more likely to say "what is the temperature set to" or "what is the temperature in the house".

I agree, but this is a whole new feature/usecase, for this we need to check groups and areas (kitchen or first floor instead of {name of thermostat}) (#76) and not specific thermostats and/or other sensors.

pfefferle commented 2 years ago

what about removing the thermostat, but adding some more question possibilities like:

what is the temperature of {Entity}

pfefferle commented 2 years ago

@krisgesling @stratus-ss I added some attributes as alternative to state, what do you think?

stratus-ss commented 2 years ago

What is the position key word for?

pfefferle commented 2 years ago

good point, it is for trackers, but there is no support yet, so I removed it.