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

[radiothermostat] Skip shutdown actions if thing offline #16677

Merged
merged 3 commits into from
Apr 25, 2024

Conversation

mlobstein
Copy link
Contributor

Improvement to #15492. Skip shutdown actions if thing is offline to prevent slowness when the binding is disposed.

Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

Thanks! I have added a comment about the framework's expectations towards the dispose method.

}
if (ThingStatus.ONLINE.equals(this.getThing().getStatus())) {
if (isLinked(REMOTE_TEMP)) {
connector.sendCommand("rem_mode", "0", REMOTE_TEMP_RESOURCE);
Copy link
Contributor

@jlaur jlaur Apr 23, 2024

Choose a reason for hiding this comment

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

If there is no need to do this when the thing is not online, that helps.

But the real issue is probably this blocking call. dispose is expected to be non-blocking and to return quickly - see https://www.openhab.org/docs/developer/bindings/#shutdown. So you should probably schedule a job for sending these commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, I have adjusted the code to run the calls within a job.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm now wondering if there is a risk that isLinked will not work after dispose returns. I didn't check the implementation (yet), but in such a case, introducing local variables before scheduling the job might help.

Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

LGTM

@jlaur jlaur merged commit 7eb5916 into openhab:main Apr 25, 2024
5 checks passed
@jlaur jlaur added this to the 4.2 milestone Apr 25, 2024
@mlobstein mlobstein deleted the radiothermostat_shutdown2 branch April 25, 2024 13:41
lo92fr pushed a commit to lo92fr/openhab-addons that referenced this pull request Apr 30, 2024
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Jun 15, 2024
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Signed-off-by: Paul Smedley <paul@smedley.id.au>
pgfeller pushed a commit to pgfeller/openhab-addons that referenced this pull request Sep 29, 2024
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
cipianpascu pushed a commit to cipianpascu/openhab-addons that referenced this pull request Jan 2, 2025
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants