openhab / openhab-alexa

openHAB skill for Amazon Alexa
Eclipse Public License 2.0
152 stars 90 forks source link

V3 Metadata and tags #68

Closed digitaldan closed 5 years ago

digitaldan commented 6 years ago

@jsetton , @fmeies Right now the V3 skill was built with the idea of tags, as of 2.3 this will change to the Metadata on items, although v2 tags will still be supported (as you both already know, just stating the obvious) . Right now we have a metadata function in the code that converts metadata to tags, but I wonder if this is the right approach? Now that I have more eyes on this, I would not mind some feedback about how we are translating items and properties to the internal map that we store on Alexa. Its rather complicated, which bothers me, but I don't know if there's a better approach.

jsetton commented 6 years ago

My initial intention when I implemented the metadata support was to limit the amount of changes around the property map core logic, you had implemented, by keeping it based on tags.

However, I thought about directly using the openHAB metadata object as the property map, that we send in the discovery response, and convert all tags to the metadata object format. This basically comes down to what we want the core logic based off.

I guess if at some point, we would want to retire/limit the tag logic, it would make sense to have a metadata-based core. Another point is that we could have the v3 capabilities and parameters support at the metadata level only while still keeping v2 tags support. That would limit the complexity I think, especially with the logic around configuration management, but the usage of the latest openHAB version will be required to use the new enhancements.

In terms of v2 tags, I am not sure if we are using the right term at this point. I feel "alias" or "metadata" label is more appropriate. My goal in my implementation was to have these labels, similar to the ones we have in v2 skill, so that we don't have to specify each time all the different capabilities, especially for light or switch items.

Just for reference, this is how the openHAB metadata object is currently structured.

metadata: {
  alexa: {
    value: "<capability#1|label#1,...>"
    config: {
      <key#1>: <value#1>,
      ...
  }
}
digitaldan commented 6 years ago

However, I thought about directly using the openHAB metadata object as the property map,

This was my though as well, one issue however is that metadata is linked to a single item, where many items can be linked to a Alexa endpoint, would an endpoint contain an array of metadata? (have not really thought that out yet)

I guess if at some point, we would want to retire/limit the tag logic, it would make sense to have a metadata-based core. Another point is that we could have the v3 capabilities and parameters support at the metadata level only while still keeping v2 tags support.

This is my intent as well, we should not encourage future use of tags for Alexa.

My goal in my implementation was to have these labels, similar to the ones we have in v2 skill, so that we don't have to specify each time all the different capabilities, especially for light or switch items.

Can you give an example of this?

jsetton commented 6 years ago

However, I thought about directly using the openHAB metadata object as the property map,

This was my though as well, one issue however is that metadata is linked to a single item, where many items can be linked to a Alexa endpoint, would an endpoint contain an array of metadata? (have not really thought that out yet)

Yes, it could be an array of metadata object to which we include the item name. Not very different than how the property map we sent out is structured currently.

My goal in my implementation was to have these labels, similar to the ones we have in v2 skill, so that we don't have to specify each time all the different capabilities, especially for light or switch items.

Can you give an example of this?

Items Definition

Color light2 "Color Light 2" {alexa="Light"}

Group gThermostat2 "Thermostat 2" {alexa="Thermostat" [binding="foobar", scale="Fahrenheit"]}
Number currentTemperature2 (gThermostat2) {alexa="CurrentTemperature"}
Number targetTemperature2 (gThermostat2) {alexa="TargetTemperature"}
Number highTargetTemperature2 (gThermostat2) {alexa="UpperTemperature"}
Number lowTargetTemperature2 (gThermostat2) {alexa="LowerTemperature"}
Number thermostatMode2 (gThermostat2) {alexa="homekit:HeatingCoolingMode"}
//Could also be alexa="HeatingCoolingMode" or alexa="ThermostatMode"

Expected Results

digitaldan commented 6 years ago

similar to the ones we have in v2 skill, so that we don't have to specify each time all the different capabilities,

Ah, ok now i understand, you want to keep the connivence of a simple label in the metadata, that would be expanded to the full capability syntax, so "Lighting" in the metadata vale would be expanded internally to "PowerController.powerState, BrightnessController.brightness"

I'm ok with this as long as we keep it very basic, so only support a small set of labels like the ones we already have. I want to avoid creating yet another vocabulary in addition to the Alexa capabilities and openHAB item types. If we feel this would be confusing, using labels in some areas but requiring the full Alexa capability namespace in others, then I would vote for requiring the Alexa namespace as it has the least amount of ambiguity, provides a clear mapping between items and endpoints, and helps future proof the syntax as Amazon adds more capabilities to the API.

jsetton commented 6 years ago

You can see here the labels/tags I have added in my implementation.

My idea is that we can always specify Alexa capabilities directly and these would take precedence over the ones derived from labels. But my concern is for folks who aren't familiar with these and may just want a functional label to setup their items.

Do we know how the homekit and google assistant openHAB projects plan to handle metadata? I know you brought this topic up on the smarthome metadata thread but I haven't seen much response. My understanding is up to now, we were all in sync in terms of tag naming convention with the homekit project.

So based on your comment, I assume you only want to keep the below labels and the rest should just support direct Alexa capability namespaces?

Lighting
Switchable

Lock

Thermostat
CurrentTemperature
TargetTemperature
(homekit:)HeatingCoolingMode
LowerTemperature (new)
UpperTemperature (new)

ColorTemperature (new)

Activity (new)
Scene (new)
digitaldan commented 6 years ago

So based on your comment, I assume you only want to keep the below labels and the rest should just support direct Alexa capability namespaces?

I think thats a good place to start, and if the collective group comes up with other labels then I think its ok to support them, but I want to avoid coming up with new ones that are just for Alexa. In the docs i think it will be good to always state what a label expands to so people are aware ( if they care) what a label is actually doing.

I agree the namespaces are long and complex, and the labels make it much nicer to configure, but I worry about having to support these long term as api's and other things change over the years.

fmeies commented 6 years ago

Looks like need to catch up on this a bit. I'm somewhat confused by the different terms/concepts. Please correct me if I'm mixing things up. As far as I understood, there are the classic openhab tags (i.e., "Lighting") but support for these tags will be dropped soon in openhab (or at least it will be deprecated) in favour of metadata. This metadata comes in two flavours: One that allows for direct mapping between openhab and the alexa smart home skill api capabilities (e.g., metadata: { alexa: {value: "PowerController.powerState, BrightnessController.brightness" } }) and one called "labels" that is somewhat more high level for convenience reasons (e.g., metadata: { alexa: {value: "Light" } }) which then internally translates to the smart home capabilities, right? So what's the advantage of the label thing over the classic tags?

Anyway, I think that a kind of a simplified mechanism whether it is tags or labels is pretty handy since I guess that most users of openhab/alexa really don't care about the details of the smart home skill api.

jsetton commented 6 years ago

One that allows for direct mapping between openhab and the alexa smart home skill api capabilities (e.g., metadata: { alexa: {value: "PowerController.powerState, BrightnessController.brightness" } }) and one called "labels" that is somewhat more high level for convenience reasons (e.g., metadata: { alexa: {value: "Light" } }) which then internally translates to the smart home capabilities, right?

Correct, that is the idea behind the concept of "labels" I mentioned.

So what's the advantage of the label thing over the classic tags?

Metadata labels and v2 tags would translate to the same v3 capabilities they would differ at the customization level, as per the examples below, since the plan is to only support parameters at the metadata level. So, the v2 tags would just use default parameters.

Switch itemTest ["Switchable"] // category => SWITCH
Switch itemTest {alexa="Switchable"} // equivalent to above configuration
Switch itemTest {alexa="Switchable" [category="other"]} // category => OTHER
jsetton commented 6 years ago

@digitaldan I just submitted a new PR with the changes we discussed above. Basically, I changed the core logic to be based on items metadata object and removed tag parameters support since it seemed that we both agreed on these two changes. I ended up keeping the same property map structure. As you predicted, having an object based on items doesn't work well for our usage.

The last change is related to the metadata labels. This is the part which I think we seem to diverge on. As it is, v2 tags and metadata labels are synced since using the same convert function. Tags are only processed if no metadata is set and v3 tags (with no parameters) are technically supported.

Below is the list of labels that I added on top of the initial one I mentioned above, so we can discuss if these labels could be useful. I also updated the documentation with the capabilities each label translates to.

EntertainmentChannel
EntertainmentInput
MediaPlayer
SpeakerMute
SpeakerVolume
marziman commented 6 years ago

@jsetton & @digitaldan:

Sorry for my late reply. I was quite busy with some stuff for openhab cloud & Google Assistant. Your proposal is pretty cool on Google Assistant side. We currently use these tags:

We are also planning to remove the Tags in favour of using Metadata labels. So the PR you did might be base for GA also. I might need to adjust it somehow.

We really must have some basic „Labels“ like e.g. Lighting, Switchable etc. abd try to keep it in openHAB generic and the Backend functions like Lamda Alexa Skill or Google Assistant Function can do their converting of the labels into the proper right actions/traits.

This my biggest wish and best choice to keep things symetric and easy to handle for users. I faced sone trouble by tags from ImperiHome, Alexa etc. which caused some users to have issues with Google Assistant and vice versa.

BR Mehmet

jsetton commented 6 years ago

We really must have some basic „Labels“ like e.g. Lighting, Switchable etc. abd try to keep it in openHAB generic and the Backend functions like Lamda Alexa Skill or Google Assistant Function can do their converting of the labels into the proper right actions/traits.

@marziman I tried to keep label names as functional as possible while still having a label for each Alexa functionalities. Looking at the Google Smart Home Device Traits, I can see that there are currently functionality discrepancies between the two voice assistants. Based on what you mentioned above, it seems that the current Google Assistant integration is derived from device types opposed to device traits (called capabilities in Alexa) whereas our current Alexa integration is the other way round with device types being derived from category parameter. So another option would be to switch our integration to be based on display categories. Each of these categories would have default capabilities configured which can be customized through the metadata type "label" parameters. In this case, I think we should be more specific and change Lighting/Switchable for Light/Switch.

// capabilities based on item type
Switch dimmerLight {alexa="Switch"} // PowerController
Color colorLight {alexa="Light"} // PowerController, BrightnessController, ColorController

// capabilities based on item type & category = OTHER as "Fan" isn't supported yet.
Dimmer dimmerFan {alexa="Fan"} // PowerController, PercentageController

// force capability to ColorTemperature
Dimmer colorTemperature {alexa="Light" [type="ColorTemperature", increment=5]}

// door lock
Switch doorLock {alexa="Lock"}

// thermostat/temperature sensor
String thermostatMode {alexa="Thermostat" [type="HeatingCoolingMode"]}
Number currentTemperature {alexa="TemperatureSensor" [type="CurrentTemperature", scale="Fahrenheit]}
Number targetTemperature {alexa="Thermostat" [type="TargetTemperature", scale="Fahrenheit]}

// scene
Switch scene {alexa="Scene"}

// speaker
Switch speakerMute {alexa="Speaker" [type="Mute"]}
Dimmer speakerVolume {alexa="Speaker" [type="Volume", increment=5]}

// tv
String tvInput {alexa="TV" [type="Input"]}
Number tvChannel {alexa="TV" [type="Channel"]}