[touchwand] Remove Thing state update on unit discovery causing expire to fail

[roieg]

in the current implementation the periodically task of units discovery also updated the units state invoking "updateState".
This occurs every 1 minute and update the unit state regardless if the state has changed or not (for example a switch from ON to OFF)
I assumed openhab framework will drop same state update and will not consider it as a "real" update.
This is actually the case except when using 'expire'
if the same state update occurs when 'expire' timer is on , it will never expire
Not sure if this is a bug or feature in the expire implementation , but it was an easy fix for me in the bundle .
I

[lolodomo]

Changing a binding behaviour before another feature in openHAB is not working as you expect looks a little strange.
Maybe you should rather open an issue for the "expire" feature ?
In case your change makes sense on binding level, why not.

[lolodomo]

The code that sets the thing to ONLINE and is called based on data retrieved by the discovery code:

    public void onItemStatusUpdate(TouchWandUnitData unitData) {
        if (unitData.getStatus().equals("ALIVE")) {
            updateStatus(ThingStatus.ONLINE);
        } else {
            // updateStatus(ThingStatus.OFFLINE); // comment - OFFLINE status is not accurate at the moment
        }
        updateTouchWandUnitState(unitData);
    }

If you suppress that, are you sure that a thing will come ONLINE after it was OFFLINE ? Needs to be studied more in details.
At least the code could set the status to ONLINE only if not yet ONLINE ? Especially if the discovery stuff is run frequently.

[lolodomo]

Ok, I see now you are the author or maintainer of this binding, so you certainly have the answers to my questions.

[lolodomo]

After a closer look, I see no code except that method that can update the thing status. So setting the status to ONLINE in onItemStatusUpdate is maybe useless.
But this method is also calling updateTouchWandUnitState which is maybe something you don't want to remove when your discovery job is retrieving data ? Or is it enough to rely on the call by your web socket class (TouchWandWebSockets) ?
In all cases, I would suggest to update the status to ONLINE in this method only when the status is not ONLINE. This will avoid having regular call to set it to ONLINE. What was the reason to not set the status to OFFLINE?

[roieg]

Thank you for your feedback , I will try to answer one by one

Touchwand binding invoke 'updateTouchWandUnitState' , the low level state update , on 3 different scenarios

1. When the Thing initialize , BaseThingHandler initialize() method is invoked , this is when Thing is being created
2. When asynchronous update message is received from the hub via WebSocket
3. Discovery process periodically poll the hubs units , update openhab thingDiscovery on new units and since it has all the current units state , invokes indirectly updateTouchWandUnitState which updates current state even if it has not changed.

When I implemented the binding , I thought that although scenario 3 is not mandatory , it will help , if from some reason , there is inconsistency between Touchwandhub hub units states and openhab Things states , it will be "fixed" in UnitDiscovery interval (1 minute).

As I using the binding in home system, I observed that that this is not really needed since this inconsistency rarely happens (if at all).
I did see , very clearly, that 'expires' does not expire , probably because the timer resets each state update regradless what was the previous state

The quick fix (without rocking the boat too much ) is remove the support third scenario as it is not really needed.
I can of course consult with the community regarding expire behavior and see if it need to be fixed.
Please let me know what you think.

Regarding the OFFLINE status
The unit get offline in Touchwand hub when, from some reasons , there is z wave connectivity issues.
Unfortunately this happened too frequently, Touchwand hub is quick to declare offline although it will go online very soon after.
Since it is not my code and I can not control it , I decided to gracefully ignore those status update and not disturb openhab with non accurate ONLINE/OFFLINE status updates.
I check if the unit is status on initialization, and it it is online, I assume it stays online .
If the unit was to ONLINE because it can not be reached , I still give it a chance by setting it to ONLINE if I get online status update for this unit via webscoket (or unit discovery).

[roieg]

I just found this thread
https://community.openhab.org/t/openhab-3-1-expire-command-does-not-always-work/124355
This is when using Z wave directly , somewhat similar problem.
But in my case I let the hub connect with the Z wave , I don't really need those periodic updates

Another solution that I can think of is keep updates in the unit discovery and invoke updateStatus only when status change .
I will need to keep the unit status in the Thing handler or use openhab API (if exist)

[lolodomo]

Ok, let's go with uour change.