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

[MyQ] Initial commit of the MyQ binding for OH3 #9347

Merged
merged 6 commits into from
Feb 26, 2021
Merged

Conversation

digitaldan
Copy link
Contributor

@digitaldan digitaldan commented Dec 12, 2020

Signed-off-by: Dan Cunningham dan@digitaldan.com

This introduces a new MyQ binding for openHAB to replace the 1.x version. Feedback welcome.

This will replace the 1.x myq binding referenced in #6179

@digitaldan digitaldan requested a review from a team as a code owner December 12, 2020 17:20
@morph166955
Copy link
Contributor

morph166955 commented Dec 13, 2020

I've tested the binary you provided. I'm getting errors in the log showing info below. The redacted JSON had all my info in it so I'm assuming it did in fact find my account.

2020-12-13 13:35:05.518 [INFO ] [me.event.ThingStatusInfoChangedEvent] - Thing 'myq:account:home' changed from UNKNOWN to ONLINE
2020-12-13 13:35:05.615 [INFO ] [me.event.ThingStatusInfoChangedEvent] - Thing 'myq:account:home' changed from ONLINE to OFFLINE (COMMUNICATION_ERROR): Invalid Response Code 404

2020-12-13 13:49:51.725 [TRACE] [q.internal.handler.MyQAccountHandler] - Account Response - status: 200 content: {"SecurityToken":"REDACTED"}
2020-12-13 13:49:51.775 [TRACE] [q.internal.handler.MyQAccountHandler] - Account Response - status: 200 content: {"Users":{REDACTED}
2020-12-13 13:49:51.826 [TRACE] [q.internal.handler.MyQAccountHandler] - Account Response - status: 404 content: {"code":"404.401","message":"Not Found","description":"Account not found."}

@digitaldan
Copy link
Contributor Author

Are you configuring this with things files? Auto discovery ? Can you describe the steps you took to configure this? Also it seems the jar you have might be a few revisions behind (logging looks outdated). Can you download again from https://github.com/digitaldan/openhab-addons/releases/tag/myq-wip-1 ? I update that with the last build time and a new jar instead of creating new revisions. Any other logging or info would be helpful. Thanks!

@digitaldan
Copy link
Contributor Author

Actually the logging does not look outdated, my guess is you do have a updated version, so info on how you configured this would be helpful, thanks!

@morph166955
Copy link
Contributor

morph166955 commented Dec 13, 2020

Configured as:

Bridge myq:account:home "MyQ Account" [ username="my@mail.com",password="shhhhhh",refreshInterval=60 ] {
        Thing garagedoor ABCDE "Double Garage Door" [ serialNumber="ABCDE" ]
        Thing garagedoor FGHIJ "Single Garage Door" [ serialNumber="FGHIJ" ]
}

I will say the JSON from the original posting looked "OK". It had my email, name, address, etc. I would assume it was a valid reply.

@fwolter fwolter added new binding If someone has started to work on a binding. For a new binding PR. oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2 work in progress A PR that is not yet ready to be merged labels Dec 13, 2020
@digitaldan
Copy link
Contributor Author

can you download org.openhab.binding.myq-3.0.0-SNAPSHOT.jar and try again? For some reason i don't think it liked the UserId i was grabbing for the account (although it works for me) , so i 'm grabbing a different one .

@digitaldan
Copy link
Contributor Author

@morph166955
Copy link
Contributor

I'd say closer but still missing something. The JSON that starts with Users is now about twice as long as it was with the last version. I still get the 404.401 error however after that.

@digitaldan
Copy link
Contributor Author

Try it again ? I missed a spot where i needed to use that new ID , thanks.

@morph166955
Copy link
Contributor

There we go! I'll start to test/monitor actual usage now.

@digitaldan
Copy link
Contributor Author

Thanks for testing, appreciate it !

@digitaldan digitaldan changed the title [WIP] MyQ Initial commit of the MyQ binding for OH3 [MyQ] Initial commit of the MyQ binding for OH3 Dec 20, 2020
@digitaldan digitaldan removed the work in progress A PR that is not yet ready to be merged label Dec 20, 2020
@digitaldan
Copy link
Contributor Author

This is now ready for review, thanks!

@morph166955
Copy link
Contributor

Do you have a final JAR you want me to test out to confirm it's all solid here?

@digitaldan
Copy link
Contributor Author

@digitaldan
Copy link
Contributor Author

@computergeek1507 thanks for helping with the light. I'm not sure how you made those changes as i don't see a branch they were committed from or a sign off message. In fact , some how you modified my personal branch on my repo. If we change that UID to lamp, we also need to modify the thing definition to match, i will update that myself, and make sure things compile and work as much as i can..

@digitaldan
Copy link
Contributor Author

@computergeek1507 is it possible to remove your changes? I'm not sure now how to get my branch in sync with those changes.

@digitaldan
Copy link
Contributor Author

@computergeek1507 I updated the binding to use "lamp" vs "light" throughout the binding so there is consistency with the MyQ naming conventions. I updated the jar at https://github.com/digitaldan/openhab-addons/releases/download/myq-wip-1/org.openhab.binding.myq-3.0.0-SNAPSHOT.jar with these changes. If you previously added a lamp/light, you will need to remove it and let the binding find the lamp again. Thanks for the pointer. If you have a json dump of a lamp that would be helpful.

@fwolter i will update it to the latest milestone shortly, thanks.

@computergeek1507
Copy link
Contributor

@digitaldan I sent the JSON to you over on the community forum

@digitaldan
Copy link
Contributor Author

I totally missed that , thank you! Looking at the json and my recent changes, it seems like it should work. Let me know if thats the case or not.

@digitaldan
Copy link
Contributor Author

@fwolter, done!

@digitaldan
Copy link
Contributor Author

I Just rebased to main and updated license headers to get the build passing

@digitaldan
Copy link
Contributor Author

👀

Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

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

Looks way better with the ThingHandlerService!

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
@digitaldan
Copy link
Contributor Author

Thanks, changes done.

@Hilbrand Hilbrand requested a review from fwolter January 26, 2021 09:52
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
@digitaldan
Copy link
Contributor Author

Unless there are any major issues, i would love to see this merged soon. Thanks.

Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

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

LGTM

@fwolter fwolter added the cre Coordinated Review Effort label Jan 30, 2021
@digitaldan
Copy link
Contributor Author

👀

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.

Only a few small suggestions.

Signed-off-by: Dan <dan@MacBook-Pro.digitaldan.com>
Signed-off-by: Dan Cunningham <dan@digitaldan.com>
@digitaldan
Copy link
Contributor Author

Just realized my last commit was missing a sign off. Should be ready now @cpmeister

@digitaldan
Copy link
Contributor Author

👀

@digitaldan
Copy link
Contributor Author

👀 😢

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

@cpmeister cpmeister merged commit 42edf53 into openhab:main Feb 26, 2021
@cpmeister cpmeister added this to the 3.1 milestone Feb 26, 2021
@morph166955
Copy link
Contributor

Updated to S2225 which includes this. Everything looks good. Thank you @digitaldan !!!!!

@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/newbie-question-myq-binding/118091/2

themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this pull request May 10, 2021
* Rebase with main, update license headers
* Small PR cleanups
* One last small PR cleanup
* Syntactical sugar
* Updated error handling
* Spelling mistake

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Signed-off-by: John Marshall <john.marshall.au@gmail.com>
computergeek1507 pushed a commit to computergeek1507/openhab-addons that referenced this pull request Jul 13, 2021
* Rebase with main, update license headers
* Small PR cleanups
* One last small PR cleanup
* Syntactical sugar
* Updated error handling
* Spelling mistake

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
* Rebase with main, update license headers
* Small PR cleanups
* One last small PR cleanup
* Syntactical sugar
* Updated error handling
* Spelling mistake

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
* Rebase with main, update license headers
* Small PR cleanups
* One last small PR cleanup
* Syntactical sugar
* Updated error handling
* Spelling mistake

Signed-off-by: Dan Cunningham <dan@digitaldan.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cre Coordinated Review Effort new binding If someone has started to work on a binding. For a new binding PR. oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants