openhab-core icon indicating copy to clipboard operation
openhab-core copied to clipboard

[profiles] Add Incoming and Outgoing profiles.

Open Jamstah opened this issue 4 years ago • 17 comments

Specific use case that triggered this is that I have a wall switch that controls two lights over RF (no openhab involvement). I'd like to link the switch to the lights in openhab so that when the switch is pressed, the status is updated correctly. But if I then turn on one of the lights individually, the link posts the wall switch command to the channel, and rfxcom transmits the message, so the other light also turns on.

The Incoming profile allows me to use the switch to keep track of the state of the lights without linking them together, without having to resort to writing rules.

The Outgoing profile... was for the yin and yang of it really. But I tested it and it works.

Jamstah avatar Jun 23 '21 10:06 Jamstah

Thanks, I was trying to acknowledge the copy and paste, but happy to just switch them.

Done and rebased.

Jamstah avatar Jul 11 '21 16:07 Jamstah

if I then turn on one of the lights individually, the link posts the wall switch command to the channel

I'd just like to understand your real problem that you are trying to solve: How do you "turn on the light individually" and to what item is the "link" attached? It's the wall switch item, I assume? But why is there any command sent, if you switch a light?

kaikreuzer avatar Jul 14 '21 10:07 kaikreuzer

OK, lets go detailed :)

I use 433MHz devices and I configure them so that the house "degrades gracefully" when openhab is down.

For simplicity, lets say I have a 2 lights, and 3 switches.

Switch A transmits code "SA" Switch B transmits code "SB" Switch C transmits code "SC"

Light 1 responds to SA and SC without openhab Light 2 responds to SB and SC without openhab

So, I have a button for light 1, and button for light 2, and a button for both.

Enter openhab: Thing SwitchA (COMMAND channel receives code SA and sends code SA) Thing SwitchB (COMMAND channel receives code SB and sends code SC) Thing SwitchC (COMMAND channel receives code SC and sends code SC)

Item Light1 (channels SwitchA:COMMAND and SwitchC:COMMAND) Item Light2 (channels SwitchB:COMMAND and SwitchC:COMMAND)

If SwitchA is pressed, openhab records that Light1 is turned on. Because Light1 is turned on, openhab also sees the linked channel and posts an ON command to switch C, which also turns on Light2.

If in openhab I set Light1 to ON, it sends a command to both SwitchA and SwitchC, and in the real world, both lights turn on, which is not what I wanted.

If I unlink SwitchC to stop that behaviour, then if I press Switch C in the real world, openhab doesn't know that Light1 and Light2 are turned on, so doesn't update their state to reflect the real world.

I could (I think) write a rule to update the state of the lights without sending a command when SwitchC is pressed, then I wouldn't need to link the lights to that channel. But it would be easier if I could just add SwitchC as a read-only channel so that the lights reflect the commands.

(In this example I have a lot of switches - and its true for some of my lights. For others, SwitchA and SwitchB are virtual switches that exist only in openhab for sending commands to individual lights, and my physical wall switches just control one or more lights. I can go into a room a turn on our "normal" 3 lamps by pressing the wall switch. If I want to customise exactly, I get the openhab app out and select individual lights. That's been working fine, except that openhab doesn't know the state of the lights, so some cases where I want to "toggle" end up being weird.)

Jamstah avatar Jul 14 '21 13:07 Jamstah

Thanks for the details! I think the "bug" in your setup is that Light1 and Light2 are linked to SwitchC, which is not correct as these items do not represent that switch. The issue is that you have a uni-directional system, which cannot report the state of devices PLUS you have to deal with an external grouping here (SC actually translating into "simultaneously SA+SB") - this clearly makes it difficult to model it nicely in openHAB.

I haven't tried it, but wouldn't it work, if you link the SwitchC with the "follow" profile to the two items? Follow means, it exchanges commands and states, i.e. if you receive SC from rfxcom, your channel will only cause a state update on Light1 and Light2 and not a command. As a sideeffect, this could now cause SwitchC to again send out SC, I'm not sure how the rfxcom binding handles this and whether that's a problem.

The cleaner approach would be to actually change the rfxcom binding: When it receives the codes SA,SB or SC, the handler should actually post a state update and not a command, since the "device" that is controlled here lives in the external world. Hence the binding only additionally captures this code and optimistically concludes that the external device has received it as well and will hopefully have changed its state. I'd therefore consider that currently a bug in the rfxcom binding. Would you agree?

kaikreuzer avatar Jul 14 '21 14:07 kaikreuzer

I haven't tried it, but wouldn't it work, if you link the SwitchC with the "follow" profile to the two items?

Follow really confuses me. It seems to still send commands (so wouldn't help in my case - if I turn on a light in openhab it would send the SC command and turn on both lights in the real world), and actually to take state updates and convert them into commands: https://github.com/openhab/openhab-core/blob/79edf2b9e643ad802c31b2e5f54edb93534277fd/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/profiles/SystemFollowProfile.java#L64

In case my understanding is wrong, I'll test it tomorrow, but don't have time right now.

The cleaner approach would be to actually change the rfxcom binding: When it receives the codes SA,SB or SC, the handler should actually post a state update and not a command

You seem to be right about what the rfxcom binding is doing (https://github.com/openhab/openhab-addons/blob/03d9281c0b4634e5844fc28f907110a2be154e44/bundles/org.openhab.binding.rfxcom/src/main/java/org/openhab/binding/rfxcom/internal/handler/RFXComHandler.java#L229) - it calls postCommand, but my understanding of state vs commands and what it should do is weak, so I couldn't tell you if I agree that it should be sending an update. What if I'm trying to use rfxcom to pick up an RF switch and use that to tell a zigbee light to turn on or something, wouldn't it need to come in as a command that gets passed through to the zigbee binding?

Also, if it was a state update, that would help me use Switch A to turn on Light 1, have it reflected in openhab, and not turn on Light 2, but then if I tried to turn on Light 1 using openhab, as both channels were still linked, wouldn't it still turn on both lights?

You're the expert here, feel free to tell me that I'm approaching this all wrong. For me, adding a read only "I just want to track the world here" seemed simple, but I can see that there may be other ways to model things that could do the job. I'm just not sure what they are :)

Jamstah avatar Jul 14 '21 23:07 Jamstah

I didn't test follow in the end, because the doc says this:

The FollowProfile takes state updates on an Item and sends them as a command onto the channel. In the direction from the ThingHandler towards the Item, the FollowProfile ignores state updates.

If anything, that makes my problem worse :)

Jamstah avatar Jul 15 '21 12:07 Jamstah

I went back and re-read it, and went and tested the follow profile.

With follow: -> UPDATE to ITEM -> Command sent to channel -> COMMAND to ITEM -> Ignored -> UPDATE from CHANNEL -> Ignored -> COMMAND from CHANNEL -> Command sent to item

So for triggering items from openhab, that does work correctly if I set Switch C to follow:

-> openhab:send Light1 ON -> Ignored by SwitchC channel (profile set to follow)

And for using the physical switches, that also works:

-> Pressing Switch A -> Command sent to Light 1 -> Command then ignored by Switch C channel -> Pressing Switch C -> Command sent to Light 1 and Light 2 -> Openhab knows both lights are on

If I go into the openhab console and openhab:update Light1 ON, then a command gets sent to both, so both lights come on. Not ideal, but then, that's not in my workflow so its fine for me.

I'm still not sure this is the intended use of the follow profile, it sounds like its designed so that a channel will follow the state of an item, not so that an item will exclusively follow the state of a channel, as evidenced by the commands sent when the item itself is updated.

I think your description was wrong:

Follow means, it exchanges commands and states, i.e. if you receive SC from rfxcom, your channel will only cause a state update on Light1 and Light2 and not a command.

Follow means it ignores commands to items, converts updates to items to commands, performs the default behaviour on commands from the thing, and ignores state updates from the thing. So if I receive the SC command from rfxcom, the item receives the SC command as normal.

Jamstah avatar Jul 15 '21 12:07 Jamstah

If we were to update rfxcom to post updates instead of commands (while using follow): -> Pressing Switch A -> UPDATE sent to Light 1 -> Turns on Light 2

When you mentioned rfxcom should send updates, are you saying that we should be adding configuration options to rfxcom things that allow the user to decide if a given thing should be posting update or commands?

If I were to do that I could identify my physical switches as things that update instead of post, and I wouldn't need to change any profiles.

Or were you saying it should always send updates instead of commands? I'm guessing no.

Jamstah avatar Jul 15 '21 12:07 Jamstah

It took me a minute or two, but you're right. Not about follow, but about rfxcom. Rfxcom should be sending updates, for everything.

What if I'm trying to use rfxcom to pick up an RF switch and use that to tell a zigbee light to turn on or something, wouldn't it need to come in as a command that gets passed through to the zigbee binding?

This ^ is what follow is for, if rfxcom send updates, you can link it to another output by adding it with the follow profile to the same item.

So, what to do?

Do you not like the idea of incoming and outgoing profiles? If not I'll just close this.

For rfxcom, just switching it is probably bad for backwards compatibility. How about making it a flag on the bridge, default to sending commands. Then put out a depreciation warning if it's not set saying the default will change to updates in the next release (that hits 3.3). Then change to default to updates in the next release? (3.4)

Jamstah avatar Jul 15 '21 20:07 Jamstah

Nope. I thought it had clicked for me but it hadn't.

I think updating rfxcom to send updates is still the right thing to do, but it doesn't fully resolve my problem. It stops light2 turning on when I press switch A, but it doesn't stop light 2 turning on when I turn on light 1 in openhab.

So I think it's a case of updating rfxcom, because it would be more correct, but I still need a way to stop the outgoing command. I feel like the profile layer is the cleanest place for that, putting it in the rfxcom binding would feel weird I think.

Gosh I talk a lot :)

Jamstah avatar Jul 17 '21 00:07 Jamstah

The cleaner approach would be to actually change the rfxcom binding: When it receives the codes SA,SB or SC, the handler should actually post a state update and not a command, since the "device" that is controlled here lives in the external world. Hence the binding only additionally captures this code and optimistically concludes that the external device has received it as well and will hopefully have changed its state. I'd therefore consider that currently a bug in the rfxcom binding. Would you agree?

Rfxcom binding was changed to send commands rather than state updates couple of years ago (I was agains the change).

https://github.com/openhab/openhab-addons/pull/2103

paulianttila avatar Jul 19 '21 21:07 paulianttila

@mjagdis makes some good points in there about commands making things "just work" more easily than using updates with follow profiles, but conceptually I agree with you that going for an update and extending that with follow and expire is more "correct" behaviour.

I'm out of my depth from an experience pov here though, so I'll add @martinvw as the other rfxcom binding maintainer.

Jamstah avatar Jul 19 '21 22:07 Jamstah

Its all a little off topic for this PR though, maybe I should close it and open a clean one for the profiles ;)

Jamstah avatar Jul 19 '21 22:07 Jamstah

@kaikreuzer a little prod - the discussion on this PR went off on talking about RFXCOM and if it should send commands or updates. Either way, I think there is still a need for these profiles.

  • If it sends commands, then the follow profile would help, because commands would be ignored by "following" items, but it would still do "unexpected" things when items receive updates.

  • If it sends updates, then follow wouldn't help either way.

The "follow" profile is for allowing a thing to follow an item, not for implementing a "read only" mode for a thing.

Happy to talk about alternative names for the profiles. ReadOnly and WriteOnly are an option, but reading and writing to what - the thing or the item? Same applies for Incoming and Outgoing I guess, but I feel it's a little more "obvious". We could be explicit and call them ThingToItemOnly and ItemToThingOnly but that feels ugly.

Jamstah avatar Aug 01 '21 17:08 Jamstah

Hey @Jamstah, sorry for the long silence, it is tough to always follow up on all discussions in a timely manner...

the discussion on this PR went off on talking about RFXCOM

Yes, mainly because I questioned if such profiles are useful at all (which I still doubt, but I am willing to be convinced ;-)).

So the RFXCom use case is the only situation, which was brought up as an example for the necessity of these profiles. My point is that it from a framework pov, all situations should be covered by having default+follow profiles. Any specific handling of commands/updates should be done within the handler of the specific binding, since the way how to deal with the events can highly depend on the devices at hand and how they allow to communicate. That's why I'd like to analyze, how your issue could be addressed within the RFXcom binding itself.

If it sends updates, then follow wouldn't help either way.

If it sent updates, the follow profile would not be needed, but the default profile should just be fine.

If you only look at your Light 1+2 with Switches A+B, everything should work as expected, do you agree? So the complexity comes with Switch C, which essentially represents a group. Linking it to any of the 2 items is hence incorrect from a modelling pov. In the KNX binding, we actually have a similar situation: Any "single" address (MainGA) can update its state by any number of additional "listening addresses" (listeningGA). We allow those to be added as configuration on the channels, see https://www.openhab.org/addons/bindings/knx/#group-address-notation.

Would that maybe also make sense for RFXCom? So instead of defining Thing SwitchC, merely say by configuration that SwitchA and SwitchB both also react on the SC command? Btw, the things should rather be called Light1 and Light2, since this is what they represent - you then only say that you determine their state by listening for the radio commands SA+SC, resp. SB+SC.

kaikreuzer avatar Aug 02 '21 13:08 kaikreuzer

Btw, the things should rather be called Light1 and Light2, since this is what they represent

I disagree.

We use Items to model state. Things are really just groups of channels through which events can flow. Interpreting how the events affect the model (and how changes to the model generate events) is the job of links and rules. The incoming events in the channel for Switch A indicate that a switch has been pressed, I could use that event in openhab to do anything. The link to Light1 is to interpret that pressing Switch A turns on Light 1.

As an extended example, I have a wall switch that in the physical world (without openhab) turns on 3 lights. Using a rule to record how many times it was pressed, if I press it twice, openhab will then turn on another 2 lights, and if I press it a third time, openhab will turn on a further 2 lights. I can't name that Thing (the wall switch) after any individual light or group of lights - it's a switch and its effect on the model is determined by links and rules.

If you only look at your Light 1+2 with Switches A+B, everything should work as expected, do you agree?

Yes, the simple case is fine already, although I believe with commands it might be echoing rf signals for no reason.

So the complexity comes with Switch C, which essentially represents a group. Linking it to any of the 2 items is hence incorrect from a modelling pov.

I disagree, based on the above. The interpretation of the events is via links and rules. A one-way link is a perfectly good way to model that interaction. The other option is to write a rule, but I believe that profiles are there to help us not have to write rules for simple cases.

In the KNX binding, we actually have a similar situation: Any "single" address (MainGA) can update its state by any number of additional "listening addresses" (listeningGA). We allow those to be added as configuration on the channels.

To me, the fact that we have a similar need in another binding indicates that it is a common need. If rfxcom implements a similar system, then we're building the same abstraction in two places. I would argue that knx should use these new profiles to support one way linking between things and items.

Jamstah avatar Aug 02 '21 14:08 Jamstah

Things are really just groups of channels through which events can flow.

No, that's where we have a different understanding. A Thing is what is needed to add technology/device specific configuration to items. If you recall openHAB 1.x item file syntax: The stuff between curly brackets was refactored out from the items file to become a separate things file, with channel ids being the reference between the two.

The correct way to model a wall switch as a Thing is to use trigger channels. The "only send events" and do not transport any state. And they are then linked to items, which represent a light and which is already linked to some proper state channel, which it represents.

In your case, the two Things clearly have to represent the physical lights in your room. You want to show toggles in the UI, which make the lights (and not the wall switches) change their state. The configuration in the Thing is thus what is required to make openHAB send out a radio telegram to make this light state change happen.

Yes, the simple case is fine already, although I believe with commands it might be echoing rf signals for no reason.

Right, I was already thinking about the "proper" way that RFXCom sends state updates instead.

To me, the fact that we have a similar need in another binding indicates that it is a common need.

No, there is no similar need. KNX has an inherent concept, which is nicely mapped to the existing features of openHAB through the binding. Read-only profiles would be of no help for a KNX configuration. If the explained possibility of configuring the channels would be removed, KNX modelling would become much much harder, because many additional channels would need to be defined and proper profiles being selected for them.

kaikreuzer avatar Aug 02 '21 19:08 kaikreuzer

Since there has not been any activity for a year now, I'm closing this PR.

J-N-K avatar May 14 '23 12:05 J-N-K