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

Improved a number of the thread names #9581

Merged
merged 1 commit into from
Jan 12, 2021

Conversation

Hilbrand
Copy link
Member

Related to #8216

Copy link
Member

@kaikreuzer kaikreuzer 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'd like to suggest a slightly stricter naming convention (that is also followed in core bundles):
"The last segment should be camel case starting with small caps."
i.e. a

"OH-binding-" + uid + "-Caddx Communicator"

would become

"OH-binding-" + uid + "-caddxCommunicator"

Wdyt?

t-8ch added a commit to t-8ch/openhab-addons that referenced this pull request Dec 29, 2020
@t-8ch
Copy link
Member

t-8ch commented Dec 29, 2020

For my linuxinput binding I submitted #9582 to make it match more the expected thread names.

Would it make sense for the core to provide a utility function to generate those names or even spawn threads?

t-8ch added a commit to t-8ch/openhab-addons that referenced this pull request Dec 29, 2020
See openhab#9581

Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
@robnielsen
Copy link
Contributor

LGTM

@kaikreuzer
Copy link
Member

Would it make sense for the core to provide a utility function to generate those names or even spawn threads?

@t-8ch We have that already through the NamedThreadFactory. This should be used for executors through

Executors.newSingleThreadExecutor(new NamedThreadFactory("binding-xyz")

which many already do. Creating own Threads within bindings should rather be avoided as an executor with a continuously running job should work in most cases as well.

@t-8ch
Copy link
Member

t-8ch commented Jan 2, 2021

@t-8ch We have that already through the NamedThreadFactory. This should be used for executors through

Executors.newSingleThreadExecutor(new NamedThreadFactory("binding-xyz")

@kaikreuzer Thanks for the pointer with the Executor and the ThreadFactory.
For my binding for each Thing a thread is spawned. I'd like to have the thing UID as part of the thread name.
Would it make sense to extend NamedThreadFactory to support this usecase or should I roll my own?

@kaikreuzer
Copy link
Member

Yes, including the ThingUID is indeed something you should do. Assembling the string can be done by yourself.
Btw, @Hilbrand summarised it nicely in #8216 (comment).

Copy link
Contributor

@cpmeister cpmeister left a comment

Choose a reason for hiding this comment

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

LGTM

J-N-K pushed a commit that referenced this pull request Jan 3, 2021
See #9581

Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Many thanks @Hilbrand!

@kaikreuzer kaikreuzer merged commit ac3f907 into openhab:main Jan 12, 2021
@kaikreuzer kaikreuzer added the enhancement An enhancement or new feature for an existing add-on label Jan 12, 2021
@kaikreuzer kaikreuzer added this to the 3.1 milestone Jan 12, 2021
@Hilbrand Hilbrand deleted the thread-naming branch January 14, 2021 21:30
nowaterman pushed a commit to nowaterman/openhab-addons that referenced this pull request Jan 19, 2021
nowaterman pushed a commit to nowaterman/openhab-addons that referenced this pull request Jan 19, 2021
themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this pull request May 10, 2021
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
Signed-off-by: John Marshall <john.marshall.au@gmail.com>
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
See openhab#9581

Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
See openhab#9581

Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
Related to openhab#8216

Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants