Rules engine does not cope pretty well with invisible properties

WebThingsIO / gateway

WebThings Gateway - a self-hosted web application for monitoring and controlling a building over the web

http://webthings.io/gateway

Mozilla Public License 2.0

2.61k stars 339 forks source link

Rules engine does not cope pretty well with invisible properties #2467

Closed bewee closed 3 years ago

bewee commented 4 years ago

Currently, the rules engine does not cope pretty well with rules that use properties marked invisible.

But let's say I have a light which has a property "mode" (enum: white, colour) and two more properties "colour" and "brightness" which are shown or hidden according to the selected mode. I can now create two rules, one depending on "brightness" and the other one depending on "colour" (I need to change the "mode" property of the light between creation of the two rules). This will then lead to always one of them getting marked [INVALID] and deactivated. Still both will be executed (although deactivated, a little weird), but when entering edit mode the rule is changed so that the now invalid nodes are deleted irreversibly.

Proposal: The rule engine should always allow all known properties of a device to be used regardless of their visibility state.

mrstegeman commented 4 years ago

We've never had a case (until now) where properties were conditionally marked visible, based on another property.

For this specific state, there is a new feature coming with 0.12: ColorModeProperty

I'd suggest adding this to your devices, and then you can base your rules off of that property value plus the other relevant property value.

bewee commented 4 years ago

Great! I've already seen this one, but why is it marked read-only?

bewee commented 4 years ago

Anyway, I guess there may be other devices doing this as well. At least I did so with this adapter (Radio: frequency is only available in FM mode, whereas skipping songs can only be done during listening Spotify, ...) How were hidden properties usually used so far?

mrstegeman commented 4 years ago

Great! I've already seen this one, but why is it marked read-only?

None of the devices I've worked with have let me directly change mode. With that said, there's nothing that's forcing it to be read-only, and I can change the schema if necessary.

How were hidden properties usually used so far?

Until now, we've only ever used them to hide complexity. For instance, a device may internally have separate hue, saturation, and brightness properties, but we may mark those as invisible and create a "fake" ColorProperty, which will internally map back to the real properties.

bewee commented 4 years ago

None of the devices I've worked with have let me directly change mode. With that said, there's nothing that's forcing it to be read-only, and I can change the schema if necessary.

The tuya lightbulbs I use allow this, even more they require the mode to be changed manually. Would be really great if this could be changed.

Until now, we've only ever used them to hide complexity. For instance, a device may internally have separate hue, saturation, and brightness properties, but we may mark those as invisible and create a "fake" ColorProperty, which will internally map back to the real properties.

But does this really require additional properties? And besides this: Wouldn't it be nice to be able to change the hue, for example, on its own from the rules engine, even if it is hidden in the normal device view?

mrstegeman commented 4 years ago

The tuya lightbulbs I use allow this, even more they require the mode to be changed manually. Would be really great if this could be changed.

Done: https://iot.mozilla.org/schemas/#ColorControl

But does this really require additional properties? And besides this: Wouldn't it be nice to be able to change the hue, for example, on its own from the rules engine, even if it is hidden in the normal device view?

This is done to match the ColorProperty schema, which takes an RGB hex color value. It was just an example. We do similar things with door locks, where the schemas use actions but the devices use a property.

bewee commented 4 years ago

Done: https://iot.mozilla.org/schemas/#ColorControl

Thanks

This is done to match the ColorProperty schema, which takes an RGB hex color value. It was just an example. We do similar things with door locks, where the schemas use actions but the devices use a property.

I don't quite see yet why it should be a problem to allow rules to make use of these hidden attributes. But anyway, could maybe some sort of other visibility flag be offered that is intended to change? For instance it could be called 'shown', set to true if not defined and affect visibility just as the visible flag but be ignored by the rules engine. Or another idea: For a property, one could specify whether its visibility might change, for instance with a flag called 'variableVisiblity' Obviously these are all just ideas.

mrstegeman commented 4 years ago

It seems weird to me to have properties that are visible to rules but not in the normal properties UI. Maybe, instead of changing the visibility, you could mark properties as read-only when they're not allowed to be changed?

bewee commented 4 years ago

I guess I see your point there. But I find it kind of unnecessary to show properties to the user which are completely irrelevant at the moment. Especially for devices with many modes, things may get messy and there may even be so many properties altogether they can no longer be rendered well (but still one may want to use them in rules). And therefore it seems somehow more reasonable to me to fill the available space with relevant properties.

mrstegeman commented 4 years ago

Yeah, that's definitely a fair point. This probably deserves some additional thought about the right approach. This may even be another case where object properties would come in handy.

mrstegeman commented 3 years ago

I'm going to close this. We had some issues with the 1.0.0 release where the visible property was not properly forwarded to the gateway, which led me to the conclusion that we really shouldn't have "invisible" properties. Instead, adapters should start filtering out their own invisible properties if they don't want the gateway to see them.