search

WebThingsIO / zigbee-adapter

Zigbee adapter add-on for WebThings Gateway
Mozilla Public License 2.0
46 stars 29 forks source link

Make property voice control friendly #219

Closed flatsiedatsie closed 3 years ago

flatsiedatsie commented 4 years ago

This improves voice control ability.

Using "power" solves both issues.

Fixes #192

mrstegeman commented 4 years ago

'Power' is actually used elsewhere for actual power measurements...

benfrancis commented 4 years ago

@flatsiedatsie What is your use case exactly? With the voice controls I have used you say the name of the device, e.g. "turn the lounge lamp off" and the voice agent uses the semantic annotations from the Thing Description to know what an on/off switch is rather than its human-readable title. What is the reason for using the title? Is it for devices that have more than one on/off property?

flatsiedatsie commented 4 years ago

@benfrancis Precisely.

And also to make it sound less strange. Right now Snips says "turning on off of living room to off". Having the voice say something like "turning power of livingroom lights to off" makes much more sense.

The word doesn't have to be "power", it can be anything. As long as it's one word, I'd be very happy. "Energy" (alhough I'd say: call the other energy or electricity, and call this power). Alternatives I like less but could work are "Toggle". "Position". "Enabled". "Active".

benfrancis commented 4 years ago

"Energy" is worse than "power".

I think "power" makes sense as a property name, but this default title is used by most adapters, not just the Zigbee adapter. It would be strange to change one and not all of the others, but on/off switches can be used for lots of purposes, not just turning power on and off.

"turning power of livingroom lights to off" makes much more sense.

Do you actually have a device with multiple on/off switches, or is this just a quirk of how the Snips voice control works? Could it use the device name instead? Saying "turn living room lights off" makes much more sense.

Perhaps this needs to be addressed by making the Snips voice control more sophisticated rather than changing titles at the adapter level. Or possibly making property titles user editable? The voice agent can't control whether web thing makers choose voice friendly names for their properties.

mrstegeman commented 4 years ago

As @benfrancis said, it would be easiest to say something like "turn X off", then search device X for a property with an @type of OnOffProperty.

flatsiedatsie commented 4 years ago

@mrstegeman No, it would not. See below (specifically the bit about "is on off of device X on or off").

Do you actually have a device with multiple on/off switches

Yes I do. Most of them.

Energy is worse

Any reasoning behind this?

To be clear, I meant:

But I'd be happy with any other construction, as long as we may avoid the weird set on off to off and is on/off of device X on or off? sentences.

To turn this around: why would you NOT want to optimise voice control usability and flexibility?

mrstegeman commented 4 years ago

As for all the additional toggles, things can just be boolean properties without needing to be OnOffProperty. But that's not the main issue here, as there are certainly cases where devices have multiple (valid) OnOffProperty properties.

To be clear, I meant:

  • Replace 'power' (measured in Watts) on electrity measurement devices with 'energy' or, perhaps even better 'electricity'.
  • This frees up power to be used to mean "power a device on or off".

By definition, Watt is a unit of power, so "Power" is the most logical name.

Instead of doing all this, maybe you could add a small quirk to your voice control intent parser to recognize properties with the name "On/Off", so that you can just say "turn the light off"?

benfrancis commented 4 years ago

Note that not every instance of the On/Off Cluster in the Zigbee specification turns power on and off. For example section 7.6.1.2 of the Zigbee Home Automation profile for the "Shade" device type says:

For this device, “On” shall mean that the shade is open and “Off” shall mean that the shade is closed (i.e. at the level corresponding to the ClosedLimit attribute of the Shade Configuration cluster).

So in this case calling the property "Power" would be incorrect.

If all current use cases of the On/Off Cluster in the Zigbee classifier turn power on and off, I think there is an argument for renaming the title "On/Off" to "Power" and "Power" to "Power Usage" (not technically accurate but possibly good enough).

If there might be some cases where the On/Off Cluster is used for something else (like the example given above), then there would need to be logic specific to device type to choose a more appropriate title.

In general though I think voice agents are going to have to be a lot more sophisticated than this and not rely on conveniently named labels and contrived sentences like "turn the power property of the lounge lamp to off", which is not a sentence that someone would ever utter in natural speech.

flatsiedatsie commented 4 years ago

Instead of doing all this, maybe you could add a small quirk to your voice control intent parser to recognize properties with the name "On/Off", so that you can just say "turn the light off"?

No that's not possible, as explained above. More importantly, as also mentioned in my explanation, this isn't just about toggling things. This is about queries too.

People shouldn't have to say "is on off of device X on or off" to find out the current state. Try saying that sentence out loud. Then imagine a machine learning algorithm trying to understand what exactly the user means. It leads to unnecessary failure if the property name and the action are the same words. That's why I said that I don't mind what the word becomes, as long as it's not "on/off".

flatsiedatsie commented 4 years ago

For this device, “On” shall mean that the shade is open and “Off” shall mean that the shade is closed (i.e. at the level corresponding to the ClosedLimit attribute of the Shade Configuration cluster).

Is that a recommended name? A recommended internal backend name? I could imagine users expect to say "open the shutters" and not "turn the shutters off". But this is just a side note.

flatsiedatsie commented 4 years ago

"turn the power property of the lounge lamp to off", which is not a sentence that someone would ever utter in natural speech.

Have you tried Voco? With Voco they already don't have to. But sometimes you want the ability to target a property.

If you do have Voco, ask "what is the state of the [lamp]". It will tell you "On off of [lamp] is off".

Of the 6 words in the sentence, 4 of them are on or of(f).

flatsiedatsie commented 4 years ago

By definition, Watt is a unit of power, so "Power" is the most logical name.

I don't find that logical, but perhaps I need to be a native speaker to agree with this. Still, like I said, I'm open to any one-word name. I also made some suggestions: "Toggle". "Position". "Enabled". "Active".

benfrancis commented 4 years ago

Is that a recommended name? A recommended internal backend name? I could imagine users expect to say "open the shutters" and not "turn the shutters off". But this is just a side note.

No, it's the name of the Zigbee cluster which the classifier of the Zigbee adapter maps to an OnOffProperty. As I understand it, your patch would label all properties of that type with the title "Power". This is an example where that wouldn't make sense because the on/off switch is not a power switch. That isn't to say that no Zigbee On/Off property should be given the title "power", just that not all on/off properties are power switches and therefore custom logic may be needed.

I agree with you that users would expect to say something like "open the blind" rather than "turn the blind off". That's my point, the voice agent needs to better understand the context and not rely on giving all on/off properties the title "Power", as in your patch.

@mrstegeman Do you think there are cases where the Zigbee classifier would currently try to map the Zigbee On/Off cluster to an OnOffProperty where that on/off property is not a power switch? If all the on/off switches of currently supported device types are in fact power switches then I think would be reasonable to rename the default title from "On/Off" to "Power" and rename "Power" to "Power Usage". If there are cases when something other than a power switch would get mapped to an OnOffProperty then more complex logic may be needed.

mrstegeman commented 4 years ago

There are 2 separate properties right now that are shown as "On/Off". One is for buttons, so the "On/Off" name is a little strange in that case anyway. The other case is the general power state for switches, lights, etc. However, I can't say for certainty that it will only ever be used for those devices, as I don't know enough about Zigbee (i.e. I'm not @dhylands).

flatsiedatsie commented 4 years ago

As I understand it, your patch would label all properties of that type with the title "Power". This is an example where that wouldn't make sense because the on/off switch is not a power switch.

Ah, I see. That's a good point. Like I said, I'm open to any other name, and I made some suggestions. It's bit weird how my proposals in that direction aren't discussed.

To me it just seems, understandably, that the name "on/off" was chosen way back when voice control wasn't a big thing for the Gateway yet. Now, however, I'd say that pretty much 90% of my day to day interactions with the Gateway are through voice, and setting some guidance for how properties can be named in a voice-friendly manner might be warranted?

By the way, the current icon for the on-off capability is... a power icon. And it's called on_off_switch.svg.

mrstegeman commented 4 years ago

"Switch" seems ok to me. Do you still think the "Power" property needs to be changed? If so, maybe "Power Consumption" would be better than "Power usage" (also note the case of the words).

@benfrancis what do you think?

flatsiedatsie commented 4 years ago

Well, going from a voice-first approach, then I'd say:

"power of the heater is on" makes more sense than "switch of the heater is on". But only where a switch is actually toggling a device being on or off. But "Switch" is a good compromise if it's not known beforehand what a property really is.

From that perspective, I think "power consumption" is great. The sentence "Power consumption of the smart socket is 1100 watts" works. It's more expressive than 'power' alone. (Someting to think about is how you would separate watts and kilowatt hours. You could say "current consumption" is the 'live' Watt level, and "total consumption" is a Kwh level that grows over time. But then "current consumption" has a fun double meaning with Amperage, so that might give a future complication there. Because then what would you use with an Amperage sensor?)

With MySensors, I just name the properties according to what makes sense, and what works well with voice (as shown in the pictures above). I have the luxury that the device transmits its desired property name.

That's for example why "data transmission" is called that, and not something generic like "data switch": it works well in a sentence. E.g. "Data transmission of the carbon sensor is on". It's also why the CO2 sensor is called the 'carbon sensor'. Voice recognition has a hard timing understanding something is an abbrevation, and would understand CO2 as "see oh two". Similarly, I also try to avoid abbrevations in prefixes like "W", and just write it out to "Watts", as long as it doesn't get too long. But I digress.

mrstegeman commented 4 years ago

I don't know that we've actually had support for any total power consumption properties (e.g. KWh), but that is definitely a possibility. "Current Power Consumption" would get rid of the ambiguity.

benfrancis commented 4 years ago

The bikeshedding continues... :)

I agree that "Power" makes more sense than "Switch" if you know that the on/off switch turns power on and off. The problem with "Switch" is that it describes the mechanism to change the property rather than the physical property that is being changed, like renaming "Level" to "Dimmer Switch".

I also agree that "Current Power Consumption" is less ambiguous than "Power Consumption" for a property measured in watts (but also more verbose).

Fundamentally the name OnOffProperty and default title "On/Off" were inspired by the name of the corresponding Zigbee cluster, and I think you can only really change it to something more specific like "Power" if you know what the on/off switch is being used for with that particular device.

My recommendation is still that if the adapter knows it's a power switch then "Power" is fine, otherwise "On/Off" is probably still the most accurate description available.

dhylands commented 4 years ago

The classifier should be able to tell based on the device id, although with some of the more generic devices like a switch (especially the battery powered ones), even the notion of on/off can't be determined by the classifier. For example, I use one of these as a mute button for my Sonos speaker.

Ultimately, I think that the user should be able to control what the user interface says and be able to influence how it reacts to voice commands. I think that this ties into internationalization as well.

mrstegeman commented 4 years ago

This is probably the right way to fix things, rather than arguing about generic property names here: https://github.com/mozilla-iot/gateway/issues/1898

benfrancis commented 4 years ago

Yep, I agree that property and action titles could be user-editable like the device title already is. Could be an expansion of the device settings view.

Also agree that internationalization (sic) is an issue here. See https://github.com/mozilla-iot/wot/issues/127

mrstegeman commented 4 years ago

internationalization (sic)

😆

tim-hellhake commented 4 years ago

No that's not possible, as explained above.

@flatsiedatsie Why not? Just add a thin translation layer that optimizes property labels for voice control. That would also allow you to optimize labels from other adapters without changing every single one of them.

flatsiedatsie commented 4 years ago

No that's not possible, as explained above.

@flatsiedatsie Why not? Just add a thin translation layer that optimizes property labels for voice control. That would also allow you to optimize labels from other adapters without changing every single one of them.

Because it only 'solves' the output case. It won't solve the issue of inputting a command. Users would still see "on/off" in the UI, and could try to say turn of the on off of the [device] if they wanted to target that property. Creating a translation layer for input would have two problems: (a) discrepancy between what the UI says and what users need to say (e.g. 'power'). And how would they learn that 'power' would work instead of 'on/off'. It's messy as heck. And (b) some devices actually have a property called 'power', so that could confuse things further. What if a device has an 'on/off' and a 'power' property. Would the rule then be "if the user says power, toggle the on/off property but only if no power property exists". You're asking your users to learn a quirk there, and it's bound to create confusion.

Even creating a translation layer for the output is messy, since it creates the same incongruency, and can lead into confusion about which speech commands are possible. Imagine you're in a hotel room with this system and you hear the bellboy give a query which leads to "the power of the [device] is off" instead of "the on off of the [device] is off". Then a new/temporary/voice only user would expect that they can also toggle the power property.

So the TLDR answer is: it's too messy.

I think ideally all Gateway property names should be selected with voice input and output in mind. Understandably that wasn't the case here. Luckily, all other properties I've come across are actually fine with voice control. It's only this one that is (a) two words with (b) a slash and that (c) uses the same words as higly common states of properties (on and off), which confuses the heck out of STT and intent recognition ML. So that's why just fixing this one will make the current system fully voice friendly. It's so close!

When it's such a small change on the Mozilla side, it doesn't make sense to need to implement translation layers in addons. Aside from all the reasons I've explained why translation layers are a very messy 'solution'.

Allowing property names to be changed would be a great solution, for sure. But it feels a ltitle like a cop out as well, since it effectively shelves the issue. More importantly, it means that the default value will remain "on/off". Not all users will change the property names, and they won't have at first, which could again lead to confusion and poor use cases.

Now I don't mean to be bike shedding (I had to look that up). I just wanted to point out a small issue that I thought would be easy to fix if we think together. Instead there is just so much push back.

benfrancis commented 4 years ago

@flatsiedatsie I understand what you're saying, but unfortunately none of the proposed alternative titles, in the English language, apply as well to all possible uses of the Zigbee On/Off cluster.

I don't see any problem with changing property titles from "On/Off" to "Power" in specific instances where the Zigbee classifier knows that the property does in fact toggle power on the device (and update other titles as necessary to reduce confusion). But that will not apply to all On/Off switches. This is just one adapter of many and native web things define their own titles. The blanket change you've proposed will only solve your very specific problem with a specific set of devices, but could make others more confusing.

More generally, voice user agents probably need to be more flexible and use title as only one input, in addition to semantic annotations and other contextual information.

I'm sure Mike or Dave would be happy to review a pull request which provides clearer default titles for specific things in the Zigbee classifier.

mrstegeman commented 3 years ago

I don't see a great reason to merge this at this time.

flatsiedatsie commented 3 years ago

Perhaps interesting: the Zigbee2MQTT project exposes the zigbee on/off variables with the name state.

I think that's a pretty good name.

            {
                "access": 7,
                "description": "On\/off state of this light",
                "name": "state",
                "property": "state",
                "type": "binary",
                "value_off": "OFF",
                "value_on": "ON",
                "value_toggle": "TOGGLE"
            },
benfrancis commented 3 years ago

I think that's a pretty good name.

I'm afraid I disagree. "State" could literally describe any property of any device. The brightness and colour of a bulb are also part of its "state" for example.

flatsiedatsie commented 3 years ago

I'd argue those properties you mention are the values of thing, while a state is something binary.

"set the value of the brightness to 34" "switch the state of the device to off"

Voco's intent recognition also uses those names to distinguish binary properties from other types.

But doesn't it all depend on how we propose to use it? So it's not really about whether it's true in the real world, it's about whether we decide to make this true in the context of webthings.

benfrancis commented 3 years ago

Well now you're just arguing over the definition of a word in the English language to try and fit your argument :)

Again, the solution to this problem is not to artificially modify the titles of properties to fit the limitations of an intent parser, but to improve the intent parser to understand the properties of a device based on its semantic metadata.

As @benfrancis said, it would be easiest to say something like "turn X off", then search device X for a property with an @type of OnOffProperty.

flatsiedatsie commented 3 years ago

You misunderstand, that stuff about the intent parser was just a fun detail about the names of functions in the Voco code. As mentioned before, Voco already works the way you describe.

However, when a user says "turn off the bedroom light", Voco might be nice enough to say which actual property has been switched. Which is where you'd get the dreaded on off of bedroom light has been switched to off.

Well now you're just arguing over the definition of a word in the English language to try and fit your argument :)

My whole point is that I don't want to do that. I want to define it within the context of the gateway only, and say that when it comes to controlling a smart home, the word state might refer to a binary toggle, while the word value could refer to non-binary properties.

benfrancis commented 3 years ago

My whole point is that I don't want to do that. I want to define it within the context of the gateway only, and say that when it comes to controlling a smart home, the word state might refer to a binary toggle, while the word value could refer to non-binary properties.

Reminder: Web thing developers can set the titles of the properties of their devices to anything they like, which makes trying to define any such constraints completely futile.

flatsiedatsie commented 3 years ago

Web thing developers can set the titles of the properties of their devices to anything they like, which makes trying to define any such constraints completely futile.

? I'm not trying to influence other developers, I'm trying to appeal to just the one who created this Zigbee addon.