Why are linkeStates (or readAfterStates) or "read-after-write" necessary?

[modmax]

This is more a question than an issue.
Also in opposite to issue #135.

I've asked myself, why the linkedStates or readAfterStates are necessary?
Until now they are only necessary to set or retrieve the correct value for "state" (on or off) if the brightness is changed. But does this make sense?

My assumption is, that the request will be fullfilled by a device, if the write reqeuest returns with an ACK; otherwise we cannot determine if the request was done or not. Also it's not very good, that a state change in iobroker is marked as "acknowledged", even if an error occured during the request.

So my decision was to just synchronize states within ioBroker, instead of triggering read requests.
Also the acknowledgments of state changes are done within "publishFromState"; in case of an error the state in iobroker stays as not acknowledged.

So I introduces the leightweight synchronization (to set state "state" to on or off, of brightness write request was ACKed) in my repository. So many READ requests will become obsolete.
For possible "readAfterWrite" needs, like mentioned in issue #135 I added the "readAfterWriteStates" where a read request is triggered after write request, as introduced by @allofmex.

Perhaps i'm completly wrong with my assumption ... i'm not involved in each internal of zcn and shepherd and so on. So this is more a question or discussion than an issue; but for my system it works better ...
but of course I've not tested each router device which may be possible .. and perhaps some devices does not allow a "moveToColorWithOnOff", where the internal state of the lam is also changed on brighness changes.

Best regards
Markus

[allofmex]

I agree that it may not be a good idea to read-back all published states

But the readAfterWrite may be useful for "device config states" that do not have reporting configured (like occupancy timeout on a motion sensor). Reported states will update themself, so there states may show wrong value only for a short time. No need for readAfter.
But those not-reported states would never show up device's real value (if write failed or device changed value by itself (e.g. to default in case of out-of-scope,...)). Here readAfter should be used.

So what I would like to see is something like readAfterStates: readAfter.readself for such no-reporting-states.

Correct me if I'm wrong, but the current readAfter/'get' implementation is just triggering the read but does not handle the response? The device response gets handled by the normal "incoming event" routine (through shepherd->fromZigbee)?
My suggest is to use the callback of this readAfter publish call. Else it would be needed to add all toZigbee states also in fromZigbee! Would heavily blowup the converter (there are many currently unused states on already implemented devices like defaultPowerOn state, notificationLedSettings,...)

[modmax]

Hi allofmex,
i've dropped the readAfterStates and replaces them with syncStates, which are just handled inside ioBroker adapter and set correct states in ioBroker. Right now only for brightness; then also the state will be set according to brightness (on or off).
For your "not-reporting device config states" i've added the readAfterWriteStates, which implements your featured. I've added this state to the Philips SML, that one with the occupancyPirUtoOdelay. In this case the value will be read after write; just like it was before.
See here:
https://github.com/modmax/ioBroker.zigbee/blob/00a4d89409da77960fbf8ba25b3600819f268b4b/main.js#L895

The advantage now is, that combining of brightness and state is handled inside ioBroker an no additional read requests are needed; for non-reporting state changes the readAfterWriteStates can be used.

My intention was to reduce the amount of requests to the device, so that only relevant requests should be sent; relevant are of course the write requests to the router; and some states which are not reported back per event on changes.

But your right: The correct implementation would be to parse the response from shepherd and write them back to ioBroker. But as you mentioned, that needs some codings to parse the response back; therefore the same converter that fpr set could be used.

[allofmex]

Great. Looks like a clean solution. Thank you very much.
(We really need a wiki page that explains all the possible devstate properties. Someone working on it already? Else I may do it...)

[modmax]

I would also here some comments from @kirovily, @Apollon77 and @arteck before I create a pull request into the dev branch.
Also I will do that after the groups has been published to the master and the 0.9.0 is alive.

But even if readAfterWrite is triggered; the response will not be parsed, but hopefully an event will come with the correct value from the device.

I think for parsing the response much more effort must be done. Right now it's all something like fire-and-forgert-and-wait-for-the-event.

[allofmex]

As far as I understand, the readAfterWrite response should not be difficult to handle. Callback of zbControl.publish() should report all we need.
In case of the occupancyPirUtoOdelay, the device response to read request looks like this:
{"msg":[{"attrId":16,"status":0,"dataType":33,"attrData":112}]}
A Osram plug (onOff status):
{"msg":[{"attrId":0,"status":0,"dataType":16,"attrData":1}]}

Should be enough to publish attrData from callback to state (maybe check attrId == writtenId)...

[modmax]

Very confusing responses.
For the OSRAM plug you have to set "on" or "off" but then read a 0 or 1 ... :-)

The problem ist that zigbee-shpeherd-converters have the converters to send (toZigbee) and also the converters for event (fromZigbee).

The reading of values directly from devices are not supported by the zigbee-shepherd-connverters, cause the data structure between "read" response and an "event" is completly different.
So a "parseResponseMessage" must be implemented for reading the values from read response and transform them into ioBroker module. Could be an idea ... but takes time ...

[allofmex]

Yes, thats correct. Setting in this case is functional command-type and the command is 'on' or 'off'. For reading we use foundation type, here the command is 'read' and it response with the (raw) attribute value.

For values set by a foundation call, we use 'write' command (like the pirUtoOdelay). A 'read' will have same format as write. We just read and write a value. No conversion needed.

I did not thought about this before. So only foundation writes can be read and published without conversion. But maybe this is enough already. The attributes we want to readAfter may be mostly/all? foundations.

[kirovilya]

@modmax Initially, reading attributes was conceived exactly as you described like fire-and-forgert-and-wait-for-the-event.
If there is a possibility to check, then you can send the answer from reading to processing here https://github.com/ioBroker/ioBroker.zigbee/blob/dev/lib/zigbeecontroller.js#L225 . The necessary event should be generated (but I'm not sure that the answer will correspond to the events used in the converters).
I think you can make a PR in dev, if it helps to improve the stability of the adapter.

[modmax]

@allofmex, @kirovilya
the reading of events is completly different then the reponse of a read request.
The get-Requesst (reading values) are always foundation calls and the response looks like this; i.e. for the brightness:
read response: [{"attrId":0,"status":0,"dataType":32,"attrData":25}]
It's always an array of objects, which contains a status and value (attrData). For parsing this response the attrId and dataType can be ignored right now.

So I added a method to parse values from the read response; convert them in the value needed by ioBroker and set this value as acknowleged in ioBroker; always depending if it should be a boolean or a number.
See: https://github.com/modmax/ioBroker.zigbee/blob/fabdc4338f222038a9315f46d644ca33eee84096/main.js#L998

For the read requests a slight offset (10ms are enough) between write and read must be implemented; had some issues with an OSRAM plug.
See:
https://github.com/modmax/ioBroker.zigbee/blob/fabdc4338f222038a9315f46d644ca33eee84096/main.js#L946

But also some special response parsing had to be done; i.e. for brightness, cause there the attrData is 25; and this must be converted via a readResponse call of the state
See: https://github.com/modmax/ioBroker.zigbee/blob/fabdc4338f222038a9315f46d644ca33eee84096/lib/devstates.js#L660

The conclusion now is, that readAfterWrite is just needed,for states which must be read after they have been written, cause the "write" does not trigger an event from the device to the zigbee network.
Right now this seems just the case for the occupance_pirUtOdelay for philipps motion sensor. But also it may be that some other foundation calls does not lead to an evenbt from the device. But I don't have such devices where I can test it.

At the moment there is just one state with the new "readAfterWriteStates"; the readAfter-States are dropped, cause that lead to heavy load.
Therefore for brightness the syncStates are introduced, where the assumption is, that an ACKed write request is fullfilled in the lamp and we just can set the state "state" within ioBroker objects to the correct state true (on) or false (off).

OSRAM devices are sometimes a little bit slow I think. A write request is done (i.e. brightness to 50%) is done; you can see it in the lamp but the request is not ACKed by the lamp.
This leads to incorrect state in ioBroker objects; so the next step could be to use readAfterWrite states in error case to have the correct brightness and state ... but thats another part.

With the PR I will create right now the Zigbee adapter should emit less read requests than now and become more stable.