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

Normalize thread names #17804

Merged
merged 26 commits into from
Dec 8, 2024
Merged

Normalize thread names #17804

merged 26 commits into from
Dec 8, 2024

Conversation

lsiepel
Copy link
Contributor

@lsiepel lsiepel commented Nov 25, 2024 •
edited
Loading

Normalize thread names:
OH-binding-<thingUID>
OH-binding-<thingUID>-<custom>
OH-binding-<bindingID>

Fixes: #8216

@lsiepel
Fix thread names
d63dc62 
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Spotless
f7d58a5 
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Spotless
2524457 
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Build fix
9195772 
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Fix build
8368069 
Signed-off-by: Leo Siepel <[email protected]>
@jlaur jlaur mentioned this pull request Nov 26, 2024
Merged
andrewfg
andrewfg approved these changes Dec 4, 2024
View reviewed changes
Copy link
Contributor

@andrewfg andrewfg left a comment

Choose a reason for hiding this comment

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

LGTM for Hue

@andrewfg
Copy link
Contributor

andrewfg commented Dec 4, 2024

@lsiepel in addition to the bindings that you have so far included, you should perhaps also add bindings with classes that extend the Thread class => search phrase "extends Thread" . Or??

One such example is as follows..

openhab-addons/bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java

Line 165 in 8d1471d

private class StatusReceiver extends Thread {

andrewfg added a commit to andrewfg/openhab-addons that referenced this pull request Dec 4, 2024
@andrewfg
normalize thread name (see openhab#17804)
34d9731 
Signed-off-by: AndrewFG <[email protected]>
@andrewfg andrewfg mentioned this pull request Dec 4, 2024
Draft
@lsiepel
Copy link
Contributor Author

lsiepel commented Dec 4, 2024 •
edited
Loading

@lsiepel in addition to the bindings that you have so far included, you should perhaps also add bindings with classes that extend the Thread class => search phrase "extends Thread" . Or??

One such example is as follows..

Thanks I searched for ‘new Thread’ if you have more ideas let me know, will look into your suggestion.

@lsiepel
Fix some more
02b85ad 
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Copy link
Contributor Author

lsiepel commented Dec 4, 2024 •
edited
Loading

@lsiepel in addition to the bindings that you have so far included, you should perhaps also add bindings with classes that extend the Thread class => search phrase "extends Thread" . Or??

One such example is as follows..

Had a look and there are 57 possible hits. It would take some more time and don;t want to extend the scope for this PR at this moment. Maybe after 4.3.0 i'll create a follow up PR.

@lsiepel
Fix import
7308a98 
Signed-off-by: Leo Siepel <[email protected]>
@andrewfg
Copy link
Contributor

andrewfg commented Dec 5, 2024

Maybe after 4.3.0 i'll create a follow up PR.

If you wish, I can do that.

@lsiepel
Russound import
e992978 
Signed-off-by: Leo Siepel <[email protected]>
@andrewfg andrewfg mentioned this pull request Dec 5, 2024
Merged
@lsiepel
Fix last imports
d5c3090 
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel lsiepel marked this pull request as ready for review December 5, 2024 19:39
@lsiepel lsiepel requested review from claell, steand and a team as code owners December 5, 2024 19:39
andrewfg
andrewfg requested changes Dec 5, 2024
View reviewed changes
Copy link
Contributor

@andrewfg andrewfg left a comment

Choose a reason for hiding this comment

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

Some comments/questions..

...penhab.binding.ekey/src/main/java/org/openhab/binding/ekey/internal/handler/EkeyHandler.java Outdated Show resolved Hide resolved
...enhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/connection/Clip2Bridge.java Outdated Show resolved Hide resolved
...ng.intesis/src/main/java/org/openhab/binding/intesis/internal/handler/IntesisBoxHandler.java Outdated Show resolved Hide resolved
@lsiepel
Fix review comment
22327d6 
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Fix review comment 2
3db75e6 
Signed-off-by: Leo Siepel <[email protected]>
andrewfg
andrewfg approved these changes Dec 6, 2024
View reviewed changes
Copy link
Contributor

@andrewfg andrewfg left a comment

Choose a reason for hiding this comment

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

I have checked them all now. LGTM

@andrewfg andrewfg added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Dec 6, 2024
@andrewfg
Copy link
Contributor

andrewfg commented Dec 6, 2024

@lsiepel the CI build is reporting "The Jenkins Controller is preparing for shutdown. No new builds can be started." .. it is not clear why that is the case .. but apart from that it all looks good to me.

morph166955
morph166955 approved these changes Dec 6, 2024
View reviewed changes
Copy link
Contributor

@morph166955 morph166955 left a comment

Choose a reason for hiding this comment

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

LGTM for AndroidTV

@lsiepel
Copy link
Contributor Author

lsiepel commented Dec 6, 2024

@lsiepel the CI build is reporting "The Jenkins Controller is preparing for shutdown. No new builds can be started." .. it is not clear why that is the case .. but apart from that it all looks good to me.

Jenkins is having trouble for some time, not related to this PR. i just ignore it, as long as the regular java builds succeeds....

hmerk
hmerk approved these changes Dec 6, 2024
View reviewed changes
Copy link
Contributor

@hmerk hmerk left a comment

Choose a reason for hiding this comment

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

LGTM

druciak
druciak approved these changes Dec 6, 2024
View reviewed changes
Copy link
Member

@druciak druciak left a comment

Choose a reason for hiding this comment

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

LGTM for Satel

jlaur
jlaur reviewed Dec 6, 2024
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for making this more consistent, and by that making it easier to monitor threads. I have added a few minor questions/remarks for consideration only.

....binding.ekey/src/main/java/org/openhab/binding/ekey/internal/api/EkeyUdpPacketReceiver.java Outdated Show resolved Hide resolved
...ding.intesis/src/main/java/org/openhab/binding/intesis/internal/api/IntesisBoxSocketApi.java Outdated Show resolved Hide resolved
....binding.milight/src/main/java/org/openhab/binding/milight/internal/protocol/QueuedSend.java Outdated Show resolved Hide resolved
...k/src/main/java/org/openhab/binding/tellstick/internal/core/TelldusCoreDeviceController.java Outdated Show resolved Hide resolved
...rc/main/java/org/openhab/binding/yeelight/internal/lib/device/connection/WifiConnection.java Outdated Show resolved Hide resolved
lsiepel and others added 4 commits December 7, 2024 12:52
@lsiepel @jlaur
Update bundles/org.openhab.binding.milight/src/main/java/org/openhab/…
2fb6ee4 
…binding/milight/internal/protocol/QueuedSend.java

Co-authored-by: Jacob Laursen <[email protected]>
Signed-off-by: lsiepel <[email protected]>
@lsiepel @jlaur
Update bundles/org.openhab.binding.yeelight/src/main/java/org/openhab…
df1aedd 
…/binding/yeelight/internal/lib/device/connection/WifiConnection.java

Co-authored-by: Jacob Laursen <[email protected]>
Signed-off-by: lsiepel <[email protected]>
@lsiepel
Fix review comments
5a592ec 
Signed-off-by: Leo Siepel <[email protected]>
@lsiepel
Merge branch 'various-threadnames' of https://github.com/lsiepel/open…
04b7cd7 
…hab-addons into various-threadnames
jlaur
jlaur approved these changes Dec 7, 2024
View reviewed changes
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, thanks!

....binding.ekey/src/main/java/org/openhab/binding/ekey/internal/api/EkeyUdpPacketReceiver.java Outdated Show resolved Hide resolved
....binding.ekey/src/main/java/org/openhab/binding/ekey/internal/api/EkeyUdpPacketReceiver.java Outdated Show resolved Hide resolved
...ding.intesis/src/main/java/org/openhab/binding/intesis/internal/api/IntesisBoxSocketApi.java Outdated Show resolved Hide resolved
...ding.intesis/src/main/java/org/openhab/binding/intesis/internal/api/IntesisBoxSocketApi.java Outdated Show resolved Hide resolved
@jlaur
Apply suggestions from code review
3e41a10 
Revert unneeded changes

Signed-off-by: Jacob Laursen <[email protected]>
@jlaur jlaur merged commit fb54c2b into openhab:main Dec 8, 2024
5 checks passed
@jlaur jlaur added this to the 4.3 milestone Dec 8, 2024
@lsiepel lsiepel deleted the various-threadnames branch December 8, 2024 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewfg andrewfg andrewfg approved these changes

@morph166955 morph166955 morph166955 approved these changes

@jlaur jlaur jlaur approved these changes

@hmerk hmerk hmerk approved these changes

@druciak druciak druciak approved these changes

@claell claell Awaiting requested review from claell claell is a code owner

@steand steand Awaiting requested review from steand steand is a code owner

@markus7017 markus7017 Awaiting requested review from markus7017 markus7017 is a code owner

@jsjames jsjames Awaiting requested review from jsjames jsjames is a code owner

@marcelrv marcelrv Awaiting requested review from marcelrv marcelrv is a code owner

@bobadair bobadair Awaiting requested review from bobadair bobadair is a code owner

@ppieczul ppieczul Awaiting requested review from ppieczul ppieczul is a code owner

@kgoderis kgoderis Awaiting requested review from kgoderis kgoderis is a code owner

@cweitkamp cweitkamp Awaiting requested review from cweitkamp cweitkamp is a code owner

@mhilbush mhilbush Awaiting requested review from mhilbush mhilbush is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.

Normalize thread naming of threads in add-ons
6 participants
@lsiepel @andrewfg @druciak @hmerk @jlaur @morph166955