smarthomeNG / smarthome

Device integration platform for your smart home
https://www.smarthomeNG.de
GNU General Public License v3.0
122 stars 91 forks source link

Subscribed item properties should be updated on the websocket #661

Closed wvhn closed 5 days ago

wvhn commented 4 months ago

SmarthomeNG supports subscribing item properties like e.g. myitem.property.last_change by a frontend via the monitor command on the websocket. The actual value of the item property is then sent with all regular items in the first answer. But while shNG later sends updates for the subscribed regular items, it does not send updates for item properties.

shNG should also send updates for the subscribed item properties, i.e. if an item is updated, check whether a property of that item has been subscribed and then update that, too.

msinn commented 4 months ago

That won't happen anytime soon. Properties of an Item are just that. All the methods of an item that enable SmartHomeNG to send updates for the item value do not exist for the properties. To implement updates for the 38 properties of an item would be almost a rewrite of the item class (one of the most complex classes of SmartHomeNG).

wvhn commented 4 months ago

Okay that is your decision. Please mark the issue as a bug since the current behavior is unexpected.

msinn commented 4 months ago

I just do not have the time to do a complete overhaul of one of the core components of SmartHomeNG.

The ability to get the property data was a request from a user (or users) of smartVISU. At that time monitoring the property data was not requested. As a quick fix, I added the property data.

I don't think it is a bug. I would rather remove the properties from the smartvisu protocol than declaring it a non fixable bug.

Btw.: There is a way/workaround. You just have to declare a helper item in SmartHomeNG and write the wanted property value to that helper item (in an on_change Attribute of the original item), Now you cat monitor the helper Item and get updated property information.

msinn commented 4 months ago

An addendum to my previous post:

There are three kinds of item properties.

msinn commented 4 months ago

@bmxp What do you think about it.

I just found out, you added the properties (to the visu_websocket plugin, back then). I know, it was 5 years ago and noone thought about changing properties back then.

bmxp commented 4 months ago

Something to the background of this ancient invention:

I had some Items where I wanted to get the last change, like the datetime the door was locked and I did not saw the need to implement another Item to reflect just this. Maybe it was before the elaborated struct possibilities we have today. That time it was sufficient to just refresh the SmartVisu page to see if there was something new.

SmartHomeNG became more and more documented but also complicated for the non programmer. We need to pay attention that it stays easy for most users.

So this is really not a bug but a feature that could be solved using another item maybe with a struct.

onkelandy commented 4 months ago

Actually I think most of the relevant properties can be written in a helper item by using on_update or on_change. If the respective Item has an eval expression it might be useful to also have an on_trigger option..?

wvhn commented 4 months ago

I think there is a certain demand for the function. Approx. 2 years back I have even been asked to implement a template checker routine for the item properties to be used instead of pure items in smartVISU. This year I've been asked to implement item properties for ioBroker. ioBroker sends a complete Object "mytem.state" where state contains properties like "lc" for last_change and others. I have mapped these to "myItem.property." in order to keep compatibility with shNG.

Morg42 commented 1 month ago

@msinn stated

There are three kinds of item properties.

  • The first kind doesn't change during the runtime of SmartHomeNG -> No problem here
  • The second kind changes when the item value changes -> Can be handled by the workaround described in my previous post
  • The third kind changes even when the item value does not change -> I have no idea how to handle them in regards to sending updates to smartVISU

After checking the core and the visu_websocket plugin, the second type of change (especially last_update, last_updat_by, previous_update, previous_update_by, last change, last_change_by, previous_change, previous_change_by, last_trigger_by..) should trigger a SV update, if the Item value is changed or enforce_updates is set.

@wvhn: can you confirm this or is this one of the cases which do not work? In the last cast, this might actually be a bug and could be fixed.

For the last kind of change regarding the list of @msinn, there is currently no way to register property changes, much less act on a change. This would necessitate a major rewrite of the whole Item class. Honestly I doubt if this much effort (especially keeping interfaces and not breaking anything) is warranted just by a "general" request.

Are there special properties which are wanted/requested to be monitored aside from the aforementioned? I'd guess that last_change and last_change_by (possibly update and trigger, too) were the most requested ones...

wvhn commented 1 month ago

The visu_websocket plugin is a good hint despite the fact that it is deprecated and has been replaced by the websocket module.

I have conducted a short test with the plugin: when monitoring an item property "last_change" by the visu, the backend delivers an update for the property every time the item is changed. This feature somehow did not make it into the implementation of the websocket module which doesn't provide the property update.

How can we handle the three kinds of properties from my point of view?

Unfortunately, there is no "read" command for a single item. Otherwise I could poll the requested properties in the driver on item updates. There is only the monitor command which terminates the active subscriptions and replaces the active list of monitored items. So using he monitor command for polling would require re-issuing the monitor command for the complete list of items and properties on a visu page. From my perspective this is not an option because of the massive increase of websocket traffic.

Morg42 commented 1 month ago

Comparing plugin visu_websocket and module websocket with protocol smartvisu, the relevant (?) code parts are present in the module and should work the same way. I have not yet understood how the websocket module gets to know about changed items, but as the module works, there must be some way I haven't found yet...

The trigger properties will update with item value change, but not necessarily the other way round (e.g. on_update/on_change -> target item is triggered but item value does not change and enforce_updates is not set).

eval only "matters" if it sets the item value, which will also update the item. Or did you want to monitor the eval expression? That should not change until we implementes editable items ;)

msinn commented 1 month ago

@Morg42

I have not yet understood how the websocket module gets to know about changed items,

When starting monitoring, the websocket module adds a trigger to the item (lie the plugins do for update_item to get called):

lines 414-415 in prepare_monitor(self, data, client_addr): in file smartvisu.py:

                       if self.update_visuitem not in item.get_method_triggers():
                            item.add_method_trigger(self.update_visuitem)

@wvhn Implementing a 'read' command would probably be the simplest solution for the websocket module.

I have conducted a short test with the plugin: when monitoring an item property "last_change" by the visu, the backend delivers an update for the property every time the item is changed. This feature somehow did not make it into the implementation of the websocket module which doesn't provide the property update.

The code in the websocket module was copied from the visu_websocket plugin, so it should behave identically.

Morg42 commented 1 month ago

@msinn: Thank you! So it works the other way around - the smartvisu-protocol inserts the trigger method (dynamically) into all items which the SV requests for monitoring...

So there we also have the culprit - if only a property, but not the item is requested, the injection does not happen.

Fixing this requires two answers:

PR #680 assuming my answers is up, please check and test.

I moved the item and acl check outside the item/property if/else branch.

msinn commented 1 month ago

do we need to monitor the item itself if we only want to monitor the property? (my answer: no) No, but the trigger has to be set for the item itself and the callback routine must recognize that a property is requested. For the item being, only the item value is sent:


def update_visuitem(self, item, caller=None, source=None, dest=None):
    item_data = (item.property.path, item(), caller, source)
    if self.janus_queue:
        # if queue has been created from the async side
        self.janus_queue.sync_q.put(['item', item_data])

    return

And it would work only for properties that change when the item value changes. The third kind of properties would stay unhandled.
msinn commented 1 month ago

Since you did not change the self.update_visuitem() method, I would suspect, your PR will not work correctly, since update_visuitem() would send the item value and not the property.

Morg42 commented 1 month ago

Read your comment just now. I'll take another look at it.

-> from reading the code, it should work. Maybe @wvhn can test this, my installation is currently blocked for other changes and tests ;)

wvhn commented 1 month ago

I tested the file smartvisu.py in the PR. Unfortunately no improvement. The Log output shows that the requested property is recognized but not sent. First test: item and item property monitored on a page

2024-10-11  15:20:36 DEBUG    modules.websocket.sv Send update to Client myClient (myIP:50276), smartVISU v3.4.a, Firefox 131 for item og.buero.licht.decke with property last_change
2024-10-11  15:20:36 DEBUG    modules.websocket.sv Send update to Client myClient (myIP:50276), smartVISU v3.4.a, Firefox 131 for item og.buero.licht.decke
2024-10-11  15:20:36 DBGMED   modules.websocket.sv visu >MONIT: '{"cmd": "item", "items": [["og.buero.licht.decke", 2]]}'   -   to myClient (myIP:50276), smartVISU v3.4.a, Firefox 131

Second test: only item property monitored

2024-10-11  15:29:50 DEBUG    modules.websocket.sv Send update to Client myClient (myIP:50399), smartVISU v3.4.a, Firefox 131 for item og.buero.licht.decke with property last_change
wvhn commented 1 month ago

New test with original smartvisu.py (current develop branch): Communication during page init:

2024-10-12  10:24:24 DBGMED   modules.websocket.sv myClient (myIP::50018), smartVISU v3.4.a, Firefox 131 sent '{'cmd': 'monitor', 'items': ['og.buero.licht.decke.property.last_change']}'
2024-10-12  10:24:24 DEBUG    modules.websocket.sv Client myClient (myIP::50018), smartVISU v3.4.a, Firefox 131 requested to monitor item last_change with property og.buero.licht.decke
2024-10-12  10:24:24 DEBUG    modules.websocket.sv json_parse: send to myClient (myIP::50018), smartVISU v3.4.a, Firefox 131: {'cmd': 'item', 'items': [['og.buero.licht.decke.property.last_change', datetime.datetime(2024, 10, 11, 17, 40, 43, 262722, tzinfo=tzfile('/usr/share/zoneinfo/Europe/Berlin'))]]}
2024-10-12  10:24:24 INFO     modules.websocket.sv Client myClient (myIP::50018), smartVISU v3.4.a, Firefox 131 new monitored items are ['og.buero.licht.decke.property.last_change']
2024-10-12  10:24:24 DBGMED   modules.websocket.sv visu >REPLY: '{'cmd': 'item', 'items': [['og.buero.licht.decke.property.last_change', datetime.datetime(2024, 10, 11, 17, 40, 43, 262722, tzinfo=tzfile('/usr/share/zoneinfo/Europe/Berlin'))]]}'   -   to myClient (myIP::50018), smartVISU v3.4.a, Firefox 131

So the function prepare_monitor() already recognizes the requested property and the correct answer is sent as "visu >REPLY:" (!) The message (2nd line) is faulty: item and property are swapped (no functional influence).

Now I switch the item on and off.

2024-10-12  10:24:44 DEBUG    modules.websocket.sv Send update to Client myClient (myIP::50018), smartVISU v3.4.a, Firefox 131 for item og.buero.licht.decke with property last_change
2024-10-12  10:24:46 DEBUG    modules.websocket.sv Send update to Client myClient (myIP::50018), smartVISU v3.4.a, Firefox 131 for item og.buero.licht.decke with property last_change

So the websocket module logs that an update will be sent but the "visu >MONIT:" message on the websocket is missing and there is no message on the websocket.

Morg42 commented 1 month ago

Fixed the log and an item access method. Works here now...

wvhn commented 1 month ago

works here, too. Thanks!

onkelandy commented 1 week ago

das Array der vom Cient abonnierten items scheint gelöscht zu werden. Irgendwie funktioniert das Update zwar beim Laden einer Seite, aber danach werden etliche Updates für einzelne Items verschluckt, bis man io.init() ausführt oder die Seite neu lädt. Siehe auch gitter.

wvhn commented 1 week ago

I think the problems are the else statements in lines 426 and 436. Before the recent commits, the else was part of a try-exept-else sequence. So the following newmonitor.append(…) was executed if no exception occured. Now, IMHO the else is superfluous so the following command is bound to the wrong condition.

Morg42 commented 6 days ago

Hope the fix works, please test.

@wvhn: you are completely right, I bungled up the else-clause... :(

wvhn commented 5 days ago

Works for me. Thanks. @onkelandy can you confirm that this solves the reported problem?

onkelandy commented 5 days ago

Yep, can confirm.