rdmtc / node-red-contrib-sun-position

This is a ultimate Node-Red Timer, Sun, Moon and Blind flow control.
Apache License 2.0
106 stars 24 forks source link

clock-timer differs from blind-control #186

Closed Jo-Be-Co closed 3 years ago

Jo-Be-Co commented 4 years ago

Hi, the clock-timer as clone of the blind-control does not catch up with the latest updates on blind-control.

Maybe the similar parts should be more equal or even based on a central implementation.

Actually I have spotted some faulty behaviour where I would like to contribute some fixes:

cheers, Jogi

Hypnos3 commented 4 years ago

Hi,

I've already thought about merging the implementation. But in particular there are many small differences. This could also lead to downward compatibility problems (changing the properties in the outgoing message, etc.), so that such a massive change is only possible in a mayor version jump.

It's on my todo, but it's not a high priority.

To your findings:

Jo-Be-Co commented 4 years ago

Hi,

I've already thought about merging the implementation. But in particular there are many small differences. This could also lead to downward compatibility problems (changing the properties in the outgoing message, etc.), so that such a massive change is only possible in a mayor version jump.

It's on my todo, but it's not a high priority.

To your findings:

  • I don't understand what you mean with "the 'trigger' option does not work correct"? Can you please explain a little bit more the problem here?

In the code of the blind-control there is a check for "trigger/noOverwrite/triggerOnly". If this is found there won't be any changes on expiring, importance, overwriteReason or the level:

        if (!onlyTrigger && node.nodeData.overwrite.active && isNaN(newPos)) {
            ...
        } else if (!onlyTrigger && !isNaN(newPos)) {
            ...
        }

clock-timer on the other hand will skip the search for overrideData in case of onlyTrigger. Otherwise overrideData will still be undefined so that the following is executed:

        if (node.nodeData.overwrite.active) {
            node.debug(`overwrite active, check of nImportance=${nImportance} or nExpire=${nExpire}`);
            if (Number.isFinite(nExpire)) {
                node.debug(`set to new expiring time nExpire="${nExpire}"`);
                // set to new expiring time
                setExpiringOverwrite(node, dNow, nExpire, 'set new expiring time by message');
            }
            if (nImportance > 0) {
                // set to new importance
                node.nodeData.overwrite.importance = nImportance;
            }
            // node.debug(`overwrite exit true node.nodeData.overwrite.active=${node.nodeData.overwrite.active}, expire=${nExpire}`);
            return setOverwriteReason(node);
        }

Grüße ;-)

Hypnos3 commented 3 years ago

can be tested in https://github.com/rdmtc/node-red-contrib-sun-position/releases/tag/1.2.0