Skip to content

Comments

[lgthinq] Initial contribution#12149

Merged
lsiepel merged 197 commits intoopenhab:mainfrom
nemerdaud:feature/lgthinq-binding
Mar 29, 2025
Merged

[lgthinq] Initial contribution#12149
lsiepel merged 197 commits intoopenhab:mainfrom
nemerdaud:feature/lgthinq-binding

Conversation

@nemerdaud
Copy link
Contributor

@nemerdaud nemerdaud commented Jan 28, 2022 •
edited
Loading

Binding to integrate OpenHab to LG Thinq API to control Thinq compatible devices through OpenHab.
See README.MD documentation for supported devices.

Signed-off-by: Nemer Daud nemer.daud@gmail.com

@nemerdaud nemerdaud requested a review from a team as a code owner January 28, 2022 22:37
@openhab-bot
Copy link
Collaborator

openhab-bot commented Jan 28, 2022

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

https://community.openhab.org/t/contribution-lg-thinq-air-conditioner-addon/132444/3

@lolodomo lolodomo added the new binding If someone has started to work on a binding. For a new binding PR. label Jan 29, 2022
hmerk
hmerk requested changes Jan 29, 2022
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.

Thanks for your contribution.
Please check for a constant naming scheme.
Some first comments made with required changes.

@nemerdaud nemerdaud requested a review from hmerk January 29, 2022 18:34
hmerk
hmerk requested changes Jan 30, 2022
View reviewed changes
@tobof
Copy link

tobof commented Feb 3, 2022

@nemerdaud Gave your binding a first shot and it just worked. Amazing! 😄

@tobof
Copy link

tobof commented Feb 5, 2022

Changing the op_mode does not seem to work, it results in an error after a few seconds.

14:19:13.543 [INFO ] [openhab.event.ItemStateChangedEvent  ] - Item 'corridorACMode' changed from 3 to 2
14:19:16.040 [ERROR] [ervices.LGThinqApiV2ClientServiceImpl] - Error returned by LG Server API. The reason is:{"resultCode":"9006","result":{"data":""}}
14:19:16.041 [ERROR] [internal.LGThinqAirConditionerHandler] - Error executing Command 3 to the channel op_mode. Thing goes offline until retry
org.openhab.binding.lgthinq.internal.errors.LGThinqApiException: Error returned by LG Server API. The reason is:{"resultCode":"9006","result":{"data":""}}
        at org.openhab.binding.lgthinq.lgservices.LGThinqApiV2ClientServiceImpl.handleV2GenericErrorResult(LGThinqApiV2ClientServiceImpl.java:231) ~[bundleFile:?]
        at org.openhab.binding.lgthinq.lgservices.LGThinqApiV2ClientServiceImpl.changeOperationMode(LGThinqApiV2ClientServiceImpl.java:128) ~[bundleFile:?]
        at org.openhab.binding.lgthinq.internal.LGThinqAirConditionerHandler$1.run(LGThinqAirConditionerHandler.java:443) [bundleFile:?]
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
        at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
        at java.lang.Thread.run(Thread.java:829) [?:?]
14:19:16.043 [INFO ] [hab.event.ThingStatusInfoChangedEvent] - Thing 'lgthinq:401:2a9a539f15:569c7573-8e85-12aa-86e5-44cb8b809993' changed from ONLINE to OFFLINE (COMMUNICATION_ERROR): Error returned by LG Server API. The reason is:{"resultCode":"9006","result":{"data":""}}

@nemerdaud
Copy link
Contributor Author

nemerdaud commented Feb 7, 2022

@tobof , please provide me $OPENHAB_USERDATA/thinq/thinq-569c7573-8e85-12aa-86e5-44cb8b809993-cap.json file.

Changing the op_mode does not seem to work, it results in an error after a few seconds.

14:19:13.543 [INFO ] [openhab.event.ItemStateChangedEvent  ] - Item 'corridorACMode' changed from 3 to 2
14:19:16.040 [ERROR] [ervices.LGThinqApiV2ClientServiceImpl] - Error returned by LG Server API. The reason is:{"resultCode":"9006","result":{"data":""}}
14:19:16.041 [ERROR] [internal.LGThinqAirConditionerHandler] - Error executing Command 3 to the channel op_mode. Thing goes offline until retry
org.openhab.binding.lgthinq.internal.errors.LGThinqApiException: Error returned by LG Server API. The reason is:{"resultCode":"9006","result":{"data":""}}
        at org.openhab.binding.lgthinq.lgservices.LGThinqApiV2ClientServiceImpl.handleV2GenericErrorResult(LGThinqApiV2ClientServiceImpl.java:231) ~[bundleFile:?]
        at org.openhab.binding.lgthinq.lgservices.LGThinqApiV2ClientServiceImpl.changeOperationMode(LGThinqApiV2ClientServiceImpl.java:128) ~[bundleFile:?]
        at org.openhab.binding.lgthinq.internal.LGThinqAirConditionerHandler$1.run(LGThinqAirConditionerHandler.java:443) [bundleFile:?]
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
        at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
        at java.lang.Thread.run(Thread.java:829) [?:?]
14:19:16.043 [INFO ] [hab.event.ThingStatusInfoChangedEvent] - Thing 'lgthinq:401:2a9a539f15:569c7573-8e85-12aa-86e5-44cb8b809993' changed from ONLINE to OFFLINE (COMMUNICATION_ERROR): Error returned by LG Server API. The reason is:{"resultCode":"9006","result":{"data":""}}

@nemerdaud nemerdaud force-pushed the feature/lgthinq-binding branch 2 times, most recently from aea5f82 to ef8745b Compare February 8, 2022 20:43
@hmerk hmerk changed the title [lgthinq] New LG Thinq Binding [lgthinq] [WIP] New LG Thinq Binding Feb 12, 2022
@hmerk hmerk added the work in progress A PR that is not yet ready to be merged label Feb 12, 2022
@nemerdaud nemerdaud force-pushed the feature/lgthinq-binding branch from 356b6b6 to 5a1eb61 Compare May 4, 2022 11:52
@nemer-smartsw
[lgthinq][review] code reformating
96b9c37 
Signed-off-by: Nemer Daud <nemer@smartsw.com.br>
@nemer-smartsw
Merge remote-tracking branch 'origin/feature/lgthinq-binding' into fe…
24b28f1 
…ature/lgthinq-binding
@nemer-smartsw
[lgthinq][review] max/min labels reformating
322f514 
Signed-off-by: Nemer Daud <nemer@smartsw.com.br>
@nemer-smartsw
[lgthinq][fix] tests
3122dde 
Signed-off-by: Nemer Daud <nemer@smartsw.com.br>
@lsiepel
Copy link
Contributor

lsiepel commented Mar 15, 2025

Not sure f you missed them, but all issues except the ones behind the ‘load more’ button are fixed.

@nemerdaud
Copy link
Contributor Author

nemerdaud commented Mar 15, 2025

Wait a little bit, @lsiepel . I will review all of them. Maybe i lost myself in the reviews..

@lsiepel
Copy link
Contributor

lsiepel commented Mar 16, 2025

Wait a little bit, @lsiepel . I will review all of them. Maybe i lost myself in the reviews..

Sorry don’t want to push. Just noticed this for the last review round. All others are done.

@lsiepel
Copy link
Contributor

lsiepel commented Mar 22, 2025

Checked all comments (we did a lot here ;-) ) and only the 21 hidden ones are left:
image

nemer-smartsw and others added 4 commits March 24, 2025 07:38
@nemer-smartsw
[lgthinq][review] Fix Reviews
53b4008 
Signed-off-by: Nemer Daud <nemer@smartsw.com.br>
@nemerdaud @lsiepel
Update bundles/org.openhab.binding.lgthinq/src/main/java/org/openhab/…
0136c36 
…binding/lgthinq/internal/handler/LGThinQBridgeHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Nemer Daud <37001239+nemerdaud@users.noreply.github.com>
@nemer-smartsw
[lgthinq][review] Fix Reviews
39f90f8 
Signed-off-by: Nemer Daud <nemer@smartsw.com.br>
@nemer-smartsw
Merge remote-tracking branch 'origin/feature/lgthinq-binding' into fe…
61bfc94 
…ature/lgthinq-binding
@nemerdaud
Copy link
Contributor Author

nemerdaud commented Mar 24, 2025

I think all is fixed for now, @lsiepel . I'm focus right now reviewing the Handler initialization as you asked me.

lsiepel
lsiepel requested changes Mar 25, 2025
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

These are my final comments. We expect a 5.0 milestone next weekend. We could possibly merge this and make it part of that milestone. Do you need additional testing time?

@nemerdaud
Copy link
Contributor Author

nemerdaud commented Mar 27, 2025

Hi, @lsiepel . I think until next sunday I can close your last reviews and my final test as well.

@lsiepel
Copy link
Contributor

lsiepel commented Mar 27, 2025

Do you also need additional testing time?

nemerdaud and others added 4 commits March 27, 2025 14:13
@nemerdaud @lsiepel
Update bundles/org.openhab.binding.lgthinq/src/main/java/org/openhab/…
f7da3f1 
…binding/lgthinq/internal/handler/LGThinQBridgeHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Nemer Daud <37001239+nemerdaud@users.noreply.github.com>
@nemerdaud @lsiepel
Update bundles/org.openhab.binding.lgthinq/src/main/java/org/openhab/…
e8ffe17 
…binding/lgthinq/internal/handler/LGThinQAbstractDeviceHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Nemer Daud <37001239+nemerdaud@users.noreply.github.com>
@nemerdaud @lsiepel
Update bundles/org.openhab.binding.lgthinq/src/main/java/org/openhab/…
aec326e 
…binding/lgthinq/lgservices/LGThinQAbstractApiClientService.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Nemer Daud <37001239+nemerdaud@users.noreply.github.com>
@nemerdaud @lsiepel
Update bundles/org.openhab.binding.lgthinq/src/main/java/org/openhab/…
4500cd5 
…binding/lgthinq/internal/handler/LGThinQAirConditionerHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Nemer Daud <37001239+nemerdaud@users.noreply.github.com>
@nemerdaud
Copy link
Contributor Author

nemerdaud commented Mar 27, 2025 •
edited
Loading

I need one more day to confirm that all the features are OK after the reviews. For all devices I have mocks that simulate the behavior, but I need all day tomorrow to go through all of them and check if there is anything that is not working as expected. So, by Saturday I believe I will have finished my tests. Is that okay for you?

@lsiepel
Copy link
Contributor

lsiepel commented Mar 28, 2025

I need one more day to confirm that all the features are OK after the reviews. For all devices I have mocks that simulate the behavior, but I need all day tomorrow to go through all of them and check if there is anything that is not working as expected. So, by Saturday I believe I will have finished my tests. Is that okay for you?

Yes, no rush, we can also postpone this binding to next month’s milestone 3. We made great progress, so a merge will be close.

@nemer-smartsw
[lgthinq][fix] Fixed problems related to finishing command/monitoring…
0935bb2 
… thread polling. Handle bridge removal (NPE)

Signed-off-by: Nemer Daud <nemer@smartsw.com.br>
lsiepel
lsiepel approved these changes Mar 29, 2025
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.

Let me know when you have finished your tests and i'll merge this.

Now, you could add the binding's logo to the openHAB website. See https://www.openhab.org/docs/developer/addons/#add-your-add-on-s-logo-to-the-openhab-website

@nemerdaud
Copy link
Contributor Author

nemerdaud commented Mar 29, 2025

@lsiepel, I've finished my testing and fixed some issues related to the Thing lifecycle. Feel free to push to M2 or push to M3, whichever you consider more appropriate.

@lsiepel lsiepel merged commit 0cebd32 into openhab:main Mar 29, 2025
2 checks passed
@lsiepel lsiepel added this to the 5.0 milestone Mar 29, 2025
@nemerdaud
Copy link
Contributor Author

nemerdaud commented Mar 30, 2025

@lsiepel, thank you very much for all your support and understanding of my time constraints. You have helped me a lot on the path to making this binding viable. I am also very grateful to @hmerk for believing in my binding from the beginning. This contribution of mine comes largely from my desire to return to the open source community everything it has given me so far. I don't know what comes next, but you can count on me to maintain this binding.

phenix1990 pushed a commit to phenix1990/openhab-addons that referenced this pull request Jul 31, 2025
@nemerdaud @jgesser
@nemer-smartsw @phenix1990
[lgthinq] Initial contribution (openhab#12149)
3febb7e 
* [lgthinq][feat] Initial contribution

Signed-off-by: Nemer Daud <nemer.daud@gmail.com>
Co-authored-by: Julio Vilmar Gesser <jgesser@gmail.com>
Co-authored-by: Nemer_Daud <nemer@smartsw.com.br>
@weymann weymann mentioned this pull request Feb 1, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wborn wborn wborn left review comments

@jlaur jlaur jlaur left review comments

@lsiepel lsiepel lsiepel approved these changes

@hmerk hmerk Awaiting requested review from hmerk

Assignees

No one assigned

Labels

new binding If someone has started to work on a binding. For a new binding PR.

Projects

None yet

Milestone

5.0

Development

Successfully merging this pull request may close these issues.