search

ioBroker / ioBroker.node-red

Instantiate the server with node-red
Apache License 2.0
52 stars 30 forks source link

"ioBroker In node" mode "block unless value change" after host reboot or Node-Red deploy. #234

Closed vvfrwl closed 2 years ago

vvfrwl commented 2 years ago

Dear Sirs

The logic "ioBroker in node" with "Mode: block unless value change" is wrong after host reboot or Node-Red deploy. Messages with the same value are not blocked. Node-Red does not take the real value of the state to compare the first time. Please find the example attached.

Thank you, Best Regards

Node-Red flow.txt Node-Red

Platform: linux operating system: linux Architecture: arm64 CPUs: 6 Speed: 500 MHz Model: Khadas VIM3 PRO RAM: 3.7 GB System uptime: 16 d. 10:03:30 Node.js: v12.22.6 NPM: 6.14.15 Disk size: 28.2 GB Disk free: 24.9 GB adapters count: 363 Uptime: 14 d. 23:29:57 Active instances: 17 Path: /opt/iobroker/

Node-Red 2.4.0 Host 3.3.18

Apollon77 commented 2 years ago

Hm ... I expect after reboot the first value is always sent through, correct?

When adapter was stopped ... how he would know which the last value from before was? He would need to persist that somewhere ... this is not yet implemented

vvfrwl commented 2 years ago

"Hm ... I expect after reboot the first value is always sent through, correct?"

I have "send no message at start". A reboot does not generate messages itself. But if the state value is updated without value change Node-Red sends the last("new") value.

Maybe I`ve not caught your question.

Situation (the same): The user state was created before. It had value true or false. It doesn`t matter. It means the state value exists when host\adapter reboots or Node-Red deploys. If we update the state with the same value the first time The "ioBroker in node" with "Mode: block unless value change" always sends the message like the value has been changed. By logic, It shouldn't do that. The value is not changed. Node-Red does not take the real value of the state from ioBroker to compare the first time. It works OK for the next comparison.

Logically expected behaviour like

on({id: '*****', change: "any"}, function (obj) {
let NewVal = obj.state.val;
let OldVal = obj.oldState.val
if (NewVal != OldVal){.....}
//...
}

The "host\adapter reboot" is equal "Node-Red deploy" for this case. The Node-Red does reinitialize.

"When adapter was stopped ... how he would know which the last value from before was?" The adapter doesn`t generate "wrong" message at start. It generates the wrong message at the first time when manages "Subscription on updates". I guess It may get the correct oldval of the object for the first comparison. It exists in ioBroker database.

This case is not critical. It is possible to make additional checks in Node-Red "code" to avoid this situation. (Manually makes checks equal "block unless value change") The problem is unlogical and not described behavior of the system. This behavior is not expected. It`s a notification for developers only ;-)

mickym2 commented 2 years ago

Hm ... I expect after reboot the first value is always sent through, correct?

When adapter was stopped ... how he would know which the last value from before was? He would need to persist that somewhere ... this is not yet implemented

The rbe or filter nodes offers as addition to ignore the initial value. So either this is an option or a rbe node has to be used in addition as workaround.

Apollon77 commented 2 years ago

Ok, I think I found it ... when "fireonstart" is not used then the initial value is not initialized, so i can change that - would mean: when not fireonstart thenread current value and use as "last value".

mickym2 commented 2 years ago

when not fireonstart thenread current value and use as "last value".

I think this not a good solution without additional options - at least this reading on start should be sent - otherwise a flow can not be correctly initialized. As already mentioned and to compare with the filter/rbe node.

With fire on start - there should be an option if the initial value can be ignored - means will be passed in each case - or the block becomes effective on start.

Apollon77 commented 2 years ago

Ok, "fire on start" currently ALWAYS fires the value - independent if rbe is used or not

So the change is just about "how to handle it if "fire on start" is false" 8and then the user decided to NOT fire the value)... then currently nothing would happen and the first value change will be triggered always. My proposed change would just affect this case and initialize the persisted "rbe" value with the current value.

So if someone wants to use external nodes to filter simply use "fire on start" - I think you mean that or?

Or what do I miss?

Apollon77 commented 2 years ago

Ps:alternatives would be to introduce a rbe-init as function that acts like eve but is always initialized on start with value - but means that „fire on start“ have no effect or fire on start ignores this option. Very intransparent.

Other Alternative is „close as won’t fix“ and User must User other nodes for that.

I need opinions here.

mickym2 commented 2 years ago

Ok, "fire on start" currently ALWAYS fires the value - independent if rbe is used or not

Yes and I think we should not loose this option. So this is what the current filter node offers as one option - to ignore the initial value and send it - in each case.

With the current behavior - it is possible to use in the iobroker in node - that only changed values will be passed, but the first value is always sent. And this is good - to initialize a flow.

The thread creator want to have an additional option to read the value and send nothing with the option fire at start. This can be an additional option but should in my opinion not replace the current behavior.

As I understand the node: Case 1: No message at start - does not read anything when NR starts Case 2: Fire at start - with block unless value changes - is currently sending always a message at start even if value hasn't changed. This is why the thread creator opens this issue. BUT I think it is useful one, but it is not correct. Case 3: Fire at start - with block unless value changes - the current value is read and updates are blocked until value changed from beginning.

I understand that case 3 is what the thread creator wants to be implemented - with loosing case 2. (works as it is). And my opinion is only to have case 3 as an additional option to the current behaviour.

Here is how the current rbe/filter node offers both options:

image

Case 2: Ignore initial value - is the current behavior of the node - to send it always when NR starts and then the next value will be checked for changes. So ignoring the initial value result that this value will be always sent and only the second value will be check against the first value for change. Case 3: Reads the initial value and send only a change from start as request by the thread creator.

mickym2 commented 2 years ago

And last comment from my side - there are more important issues - as in my opinion the benefit to have functionality of a filter node included in the iobroker-in node is minimal.

Much more useful in the iobroker IN node would be if values cannot only filtered when ACK=true (or ignoring the ACK-Flag) also to be able to filter ACK=false values. Currently we always have to read the objects and the filter the values afterwards.

https://github.com/ioBroker/ioBroker.node-red/issues/240

Apollon77 commented 2 years ago

@mickym2 I currently go through all issues and if I feel able to do them I will put that into the next (major) release we having in any case ... sooo lets say it that way ... Feel free to have a look at the issues that are there and support me in posting proposals how it should work or what to consider. I'm not an node-red expert and also do not use it persinally at all, so any advice helps me and any proposals help me to do it faster - I have roughly 20 adapters more on my list, so I also do a "cost effort check" for me ... :-) Any support - also with that is helpful. And thanmk yiu for your detailed infoas above, now I understand better (I hope so at least) ;-))

mickym2 commented 2 years ago

... Feel free to have a look at the issues that are there and support me in posting proposals how it should work or what to consider. I'm not an node-red expert and also do not use it persinally at all, so any advice helps me and any proposals help me to do it faster - I have roughly 20 adapters more on my list, so I also do a "cost effort check" for me ... :-) Any support - also with that is helpful. And thanmk yiu for your detailed infoas above, now I understand better (I hope so at least) ;-))

I closed one issue which was already implemented in 2.4.1. Some other issues I added a comment. The other open issues - i think are valid in my opinion (most of them are issued by myself). I opened 2 additional one's - as I think, that it could help to improve the NR adapter.

Apollon77 commented 2 years ago

Thank you for your support ...

Apollon77 commented 2 years ago

Ok to finish here this thing makes me headaches ;-)

@mickym2 @vvfrwl I propose to serate the tow features:

Makes that sense?

mickym2 commented 2 years ago

I would not mix those options - and keep the change as simple as possible.

fireOnStart should be completely independent of the rbe. It is just the same behaviour as today - is the state read after the adapter starts or not. So I would not mix this with the rbe and let not ALWAYS firing a value.

So fireOnStart does only influence when an optional following rbe will be initialized - either after adapter start or an update of the state.

So it is not so complex - but it should not be mixed the the fire on start.

So we have all combinations:

  1. send no message on start => send all events: all states will be sent when an update of a state occurs
  2. send no message on start => rbe-ignoreinitial: current behaviour - sending out the first value when state is updated and check further updates if state is changed.
  3. send no message on start => rbe (as current but different behavior): current state is read and rbe is initialized. Updated state will only be sent out when changed.
  4. fire on start => send all events: all states will be sent when adapter has started
  5. fire on start => rbe-ignoreinitial: current behaviour - sending out the first value when adapter has been started and check further updates if state is changed. (Only change description).
  6. fire on start => rbe current state is read and rbe is initialized. Updated state will only be sent out when changed.

As 3 and 6 behaves the same - it does not play a role.

So the only changes are:

  1. Change the description for existing rbe behavior to rbe-ignoreinitial
  2. Add a second rbe option - with the current description and read the existing value at adapter start to initialize rbe function.

This makes it not complicated and is easy to understand.

So fire on start means, that message is sent after adapter start

In the last case (only rbe) it is just initialised - but does not cause a different behavior.

Apollon77 commented 2 years ago

@mickym2: Thank you for your thougths. Yes ake sense. So the current stay "rbe" (backward compat but I add the "ignore inbitial value" to the selection") and I add a new rbe-preinitvalue

ANd in fact for your case nr 6 above the "fireonstart" will never send out a value, right? (Because the current value is pre-initialized and then send will find unchanged value)

mickym2 commented 2 years ago

No the current state (with backward compatibility) should be renamed to "initial value will be ignored" as the first value will be sent in any case - as it is currently implemented In addition there is the rbe option which reads the values in advance, but does not send this value and only initialize the rbe function.

No case 6 and 3 are identical for a user - but differs only when the rbe function is initialized. In both cases (3 & 6) a changed value will be sent.

In case 3 - no message at start - the rbe is initialized when state is updated. In case 6 - fire at start - the rbe is initialized at adapter start.

In both cases the first value will not be sent out and only used to initialize the rbe. In both cases a changed value will be sent out - also in case 6. Fire at start does in case 6 means that the value is read and rbe is initialized but nothing sent at the beginning.

Apollon77 commented 2 years ago

Yes all fine ... we exactly tell the same :-)

Apollon77 commented 2 years ago

GitHub updated

mickym2 commented 2 years ago

So issue should be reopened to have a consistent description of the rbe functionality and the rbe functionality of the filter node. So we have to change - what means initial value is ignored and vice versa. screen

So the description should only be changed - that when initial value is ignored - no value will be sent and a value is sent when initial value is not ignored.