Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[actions] Don't deprecate ScriptExecution.createTimer #171

Merged
merged 7 commits into from
Nov 12, 2022

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented Nov 12, 2022

Reference https://community.openhab.org/t/js-scripting-why-are-we-deprecated-createtimer/140748.
Refercence openhab/openhab-addons#13695.

Description

This PR reverts the deprecation of createTimer from previous PR #169.

As discussed on the forum https://community.openhab.org/t/js-scripting-why-are-we-deprecated-createtimer/140748, the setTimeout and setInterval methods do not provide the functionality that advanced users need, and to avoid that those users and users coming from Rules DSL leave JS Scripting because of a missing advanced timer creation API, createTimer is reimplemented to be thread-safe in openhab/openhab-addons#13695.

createTimerWithArgument is still deprecated, as there is no use case for it in JS (see openhab/openhab-addons#13695 (comment)).

This PR is backward compatible and does not require the addon to be on the latest add-on version with the thread-safe reimplementation of createTimer, but keep in mind that if not, this can lead to multithread errors.

Testing

Use the following file-based script:

const { actions, time } = require('openhab');

const now = time.toZDT();

// Note that all timers are scheduled to nearly the same time, so without the synchronization mechanism in place, this would lead to a multi-thread exception
actions.ScriptExecution.createTimer(now, () => {
    console.info("Hello world from createTimer without identifier")
});

actions.ScriptExecution.createTimer('createTimer', now, () => {
    console.info("Hello world from createTimer with identifier")
});

let arg1 = 'test';
actions.ScriptExecution.createTimerWithArgument(now, arg1, (newArg) => {
    console.info(`Hello world from createTimerWithArgument without identifier and argument: ${newArg}`);
});

actions.ScriptExecution.createTimerWithArgument('createTimerWithArgument', now, arg1, (newArg) => {
    console.info(`Hello world from createTimerWithArgument with identifier and argument: ${newArg}`);
});

by using the thread-safe createTimer methods from the injected ThreadsafeTimers.

Also remove the deprecation warning for createTimer and createTimerWithArgument.

Signed-off-by: Florian Hotze <[email protected]>
Signed-off-by: Florian Hotze <[email protected]>
Signed-off-by: Florian Hotze <[email protected]>
Signed-off-by: Florian Hotze <[email protected]>
Signed-off-by: Florian Hotze <[email protected]>
@florian-h05
Copy link
Contributor Author

@rkoshak Could you possibly review at least the README changes?

@florian-h05 florian-h05 added the enhancement New feature or request label Nov 12, 2022
@florian-h05 florian-h05 added this to the 2.x.x milestone Nov 12, 2022
Copy link
Contributor

@digitaldan digitaldan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Florian Hotze <[email protected]>
@florian-h05 florian-h05 merged commit 080c9d9 into openhab:main Nov 12, 2022
@florian-h05 florian-h05 deleted the scriptexecution-timers branch November 12, 2022 22:24
florian-h05 added a commit to florian-h05/openhab-webui that referenced this pull request Nov 24, 2022
florian-h05 added a commit to florian-h05/openhab-rules-tools that referenced this pull request Dec 16, 2022
Switch from the deprecated `actions.ScriptExecution.createTimerWithArgument` to ...`createTimer` to the threadsafe version.

Reference openhab/openhab-js#171.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants