ioBroker / ioBroker.javascript

Script engine for JavaScript and Blockly
MIT License
325 stars 120 forks source link

Feature: function setState / setStateAsync only if not equal with oldState (setStateChanged) #1507

Closed Scrounger closed 6 months ago

Scrounger commented 7 months ago

Describe the solution you'd like I would like to have a function where the state is only changed, if the val is diffrent from oldState.

E.g. setStateIfNotEqual / setStateIfNotEqualAsync or an optinal parameter in the existing setState / setStateAsync function.

klein0r commented 7 months ago

This feature is called setStateChanged[Async]

https://github.com/ioBroker/ioBroker.js-controller/blob/51f36ede1998a6d2488218bb896f19be36d9dea4/packages/adapter/src/lib/adapter/adapter.ts#L8150

Must be added to the sandbox to make those functions available.

PeterVoronov commented 6 months ago

This feature is called setStateChanged[Async]

https://github.com/ioBroker/ioBroker.js-controller/blob/51f36ede1998a6d2488218bb896f19be36d9dea4/packages/adapter/src/lib/adapter/adapter.ts#L8150

Must be added to the sandbox to make those functions available.

I made a simple implementation setStateChanged/setStateChangedAsync based on adapter.setForeignStateChanged.

1541

Notise: looks like the adapter.setForeignStateChanged use the state.ts value to compare, as result, it always be changed.

            let differ = false;
            if (!oldState) {
                differ = true;
            } else if (state.val !== oldState.val) {
                differ = true;
            } else if (state.ack !== undefined && state.ack !== oldState.ack) {
                differ = true;
            } else if (state.q !== undefined && state.q !== oldState.q) {
                differ = true;
            } else if (state.ts !== undefined && state.ts !== oldState.ts) {
                differ = true;
            } else if (state.c !== undefined && state.c !== oldState.c) {
                // if comment changed
                differ = true;
            } else if (state.expire !== undefined && state.expire !== oldState.expire) {
                differ = true;
            } else if (state.from !== undefined && state.from !== oldState.from) {
                differ = true;
            } else if (state.user !== undefined && state.user !== oldState.user) {
                differ = true;
            }

            if (differ) {

I tried to clear the state.ts before call adapter.setForeignStateChanged - but in this case it set value without ts. There are two possibility to solve:

PeterVoronov commented 6 months ago

As klein0r already wrote: js-controller already provides this functionality.

#1507 (comment)

Yes. But it is unavailable in scripts. This PR exactly for it - to announce this function in sanbox and make it available inside the user scripts. But there is an issue - via function adapter.setForeignStateChanged from js-controller it always be changed.

I will try to explain additionally: There is a definition in js-controller:

 setForeignStateChanged(
        id: any,
        state: any,
        ack: any,
        options?: any,
        callback?: any
    ):

state is an object:

state = {
  val:
  ack:
  q:
  ts:
  c:
  expire:
  from:
  user:
}

And all properties of new state are compared with similar old state. And, ts property is set in sandbox, and it will be always different from the ts of existing object. So - state will always be changed, and as result - always set.

klein0r commented 6 months ago

Notise: looks like the adapter.setForeignStateChanged use the state.ts value to compare, as result, it always be changed.

.ts will be just checked if provided:

if (state.ts !== undefined && state.ts !== oldState.ts) {

e.g.

setForeignStateChanged('bla.0.test`, { val: 'test', ts: 1712081423 });
PeterVoronov commented 6 months ago

Ok.

Below is explanation, why with simple call setStateChanged('bla.0.test, 'test')the value will be set independently from the current value of state. I.e. write to the state database will be done andts` attribute will be changed to the current.

There is only one possibility, why it not will we written - if user previously will read in script via getState all current state attributes and will use is as an obect - setStateChanged('bla.0.test`, {val: 'test', ts: oldTs, lc: Oldls, ...}) But in this case this function has no sense at all, because if user should previously read current state, then it not needed to call setStateChanged, due to call any setState can be avoided in script ... Moreover - the overhead with reading current state is already made ...

So, let' check it step by step, why setStateChanged via setForeignStateChanged will every time set state value and change state.ts:

And now in the ioBroker.js-controller in the /packages/adapter/src/lib/adapter/adapter.ts we have a call

async setForeignStateChanged(
        id: any,
        state: any,
        ack: any,
        options?: any,
        callback?: any
    )
...
            const res = await this._setStateChangedHelper(id, state);
            return tools.maybeCallbackWithError(callback, null, res.id, res.notChanged);
        } else {
            const res = await this._setStateChangedHelper(id, state);
            return tools.maybeCallbackWithError(callback, null, res.id, res.notChanged);
        }

then next:

 private async _setStateChangedHelper(id: string, state: ioBroker.SettableState): Promise<SetStateChangedResult> {
...
let differ = false;
            if (!oldState) {
                differ = true;
            } else if (state.val !== oldState.val) {
                differ = true;
            } else if (state.ack !== undefined && state.ack !== oldState.ack) {
                differ = true;
            } else if (state.q !== undefined && state.q !== oldState.q) {
                differ = true;
            } else if (state.ts !== undefined && state.ts !== oldState.ts) {
                differ = true;
            } else if (state.c !== undefined && state.c !== oldState.c) {
                // if comment changed
                differ = true;
            } else if (state.expire !== undefined && state.expire !== oldState.expire) {
                differ = true;
            } else if (state.from !== undefined && state.from !== oldState.from) {
                differ = true;
            } else if (state.user !== undefined && state.user !== oldState.user) {
                differ = true;
            }

            if (differ) {
                if (this.performStrictObjectChecks) {
                    // validate that object exists, read-only logic ok, type ok, etc. won't throw now
                    await this._utils.performStrictObjectCheck(id, state);
                }
                this.outputCount++;
                await this.#states!.setState(id, state);
                return { id, notChanged: false };
            } else {
                return { id, notChanged: true };
            }

So, to solve this issue we need to not transfer state.ts to setForeignStateChanged(.

And it will work, but in this case when val will be changed to the new value will not be immediately shown via getState . On my current system it will took about 0,1 second (100 milliseconds).

The appropriate branch is updated, but I'm not sure - is it has sense to implement these functions in sandbox and adapter, due to "caching" issue.

I will recommend use the construction in users javascripts like:

getState(state, (err, result) => {
  if (result && result.val !== newValue) {
     setState(state, newValue)
  }
});

P.S. And yes, it is not an issue of the js-controller ...

PeterVoronov commented 6 months ago

I made last fixes. Now it's works like expected.

PeterVoronov commented 6 months ago

Integrations test of new function is added