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.

Sign up for GitHub

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

Jump to bottom

[denonmarantz] Run the Telnet socket in a dedicated thread #9511

Merged
merged 2 commits into from
Dec 26, 2020
Merged

[denonmarantz] Run the Telnet socket in a dedicated thread #9511

merged 2 commits into from
Dec 26, 2020

Conversation

jwveldhuis
Copy link
Contributor

Fixes #9494

@jwveldhuis
Run the Telnet socket in a dedicated thread, not from the shared thre…
96d5110 
…ad pool.

Fixes openhab#9494
Signed-off-by: Jan-Willem Veldhuis <jwveldhuis@gmail.com>
J-N-K
J-N-K requested changes Dec 25, 2020
View reviewed changes
Copy link
Member

@J-N-K J-N-K left a comment

Choose a reason for hiding this comment

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

I would suggest a differet approach: Instead of implementing Runnable in DenonMarantzTelnetClient you could extend Thread. Then in l. 64 while (i!sInterrupted()) and call disconnect after exiting the while-loop. In the connector you can call telnetClient.start() to start it and telnetClient.interrupt() to shut it down.

@J-N-K J-N-K self-assigned this Dec 25, 2020
@J-N-K
Copy link
Member

J-N-K commented Dec 25, 2020

Also please set a name to the thread

@Hilbrand
Copy link
Member

Thank you for fixing this. For thread naming see: #8216

@jwveldhuis
Processed review comments.
fa4bbb5 
Signed-off-by: Jan-Willem Veldhuis <jwveldhuis@gmail.com>
@jwveldhuis
Copy link
Contributor Author

jwveldhuis commented Dec 26, 2020 •
edited
Loading

@Hilbrand thanks for the pointer, it also helped debugging the binding to have a fixed Thread name per Thing.

@J-N-K it's ready for a new review.
BTW: the Jenkins setup, why is it compiling every binding and not just the ones touched in the particular PR?

@J-N-K J-N-K merged commit d014ac6 into openhab:main Dec 26, 2020
@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of an add-on label Dec 26, 2020
@J-N-K J-N-K added this to the 3.1 milestone Dec 26, 2020
@morph166955 morph166955 mentioned this pull request Dec 26, 2020
Closed
@J-N-K J-N-K mentioned this pull request Dec 28, 2020
Closed
seaside1 pushed a commit to seaside1/openhab-addons that referenced this pull request Dec 28, 2020
@jwveldhuis @seaside1
[denonmarantz] Run the Telnet socket in a dedicated thread (openhab#9511
8718b23 
)

* Run the Telnet socket in a dedicated thread, not from the shared thread pool.
Fixes openhab#9494
Signed-off-by: Jan-Willem Veldhuis <jwveldhuis@gmail.com>
seaside1 pushed a commit to seaside1/openhab-addons that referenced this pull request Dec 28, 2020
@jwveldhuis @seaside1
[denonmarantz] Run the Telnet socket in a dedicated thread (openhab#9511
19a9833 
)

* Run the Telnet socket in a dedicated thread, not from the shared thread pool.
Fixes openhab#9494
Signed-off-by: Jan-Willem Veldhuis <jwveldhuis@gmail.com>
Signed-off-by: Joseph Hagberg <joseph@zoidberg.se>
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/denonmarantz-binding-broken-after-update-to-oh3-1/124275/6

thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
@jwveldhuis @thinkingstone
[denonmarantz] Run the Telnet socket in a dedicated thread (openhab#9511
2fd034e 
)

* Run the Telnet socket in a dedicated thread, not from the shared thread pool.
Fixes openhab#9494
Signed-off-by: Jan-Willem Veldhuis <jwveldhuis@gmail.com>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
@jwveldhuis
[denonmarantz] Run the Telnet socket in a dedicated thread (openhab#9511
4e8e0cf 
)

* Run the Telnet socket in a dedicated thread, not from the shared thread pool.
Fixes openhab#9494
Signed-off-by: Jan-Willem Veldhuis <jwveldhuis@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@J-N-K J-N-K J-N-K approved these changes

@kaikreuzer kaikreuzer Awaiting requested review from kaikreuzer

Assignees

@J-N-K J-N-K

Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Milestone
3.1
Development

Successfully merging this pull request may close these issues.

[DenonMarantz] Telnet Thread Issue
4 participants
@jwveldhuis @J-N-K @Hilbrand @openhab-bot