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

[shieldtv] Initial Contribution - NVIDIA ShieldTV #13934

Closed
wants to merge 46 commits into from

Conversation

morph166955
Copy link
Contributor

@morph166955 morph166955 commented Dec 12, 2022

This is the initial contribution of the NVIDIA ShieldTV binding. The intention is to mirror the NVIDIA ShieldTV App (Android version) protocol to enable IP based control over the ShieldTV. Initial version will be limited to basic X.509 key exchange, auto discovery via mDNS on-screen PIN request process, basic key press functionality, and app control. Future versions will include anything else we are able to uncover as we explore the (otherwise undocumented) protocol.

@lolodomo lolodomo added the new binding If someone has started to work on a binding. For a new binding PR. label Dec 12, 2022
@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/nvidia-shield-binding-successfully-decoded-protocol-looking-for-binding-coding-help/141610/13

Signed-off-by: morph166955 <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: morph166955 <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Added encoder/decoder
Added raw channel

Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
RAW.

Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Updated stream reader.
Added hostname decoder.

Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Adjusted message parser for hostname.

Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Added RAWMSG channel to test decoder
Updated read for new message types
Added message decoders.

Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
working.

Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
morph166955 and others added 5 commits December 17, 2022 16:09
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
parser.  Updated callbacks.

Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
@morph166955 morph166955 marked this pull request as ready for review December 18, 2022 02:31
@morph166955 morph166955 requested a review from a team as a code owner December 18, 2022 02:31
@morph166955
Copy link
Contributor Author

morph166955 commented Dec 18, 2022

Binding tests and works on all of my shields.
Keypress works no issue.
Pin registration works no issue.
App control and status works with caveat that status only seems to report after the shield receives one command (of any kind).

Auto device discovery is not implemented yet. I may work on that as a follow-up or may commit to here depending on time.

Big thanks to @bobadair for giving permission to reuse the leap parts of the lutron binding. It made this significantly easier to get knocked out.

JAR available at: https://github.com/morph166955/openhab-addons/releases/tag/shieldtv-beta-2

Published to the community marketplace at: https://community.openhab.org/t/nvidia-shieldtv-binding/142203

@morph166955 morph166955 changed the title [shieldtv][WIP] Initial Contribution - NVIDIA ShieldTV [shieldtv] Initial Contribution - NVIDIA ShieldTV Dec 18, 2022
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
@morph166955
Copy link
Contributor Author

morph166955 commented Dec 28, 2022

This is ready for review IMHO. I'm working on mdns discovery, and if I can get it all figured out prior to review I'll add it, but otherwise I don't have anything on the (immediate) roadmap to contribute at this time (other than fixes from reviews).

Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
@morph166955
Copy link
Contributor Author

And if someone can tell me what I'm doing wrong on mdns discovery I'd greatly appreciate it. From what I can tell it's not finding the binding as a participant.

https://github.com/morph166955/openhab-addons/blob/shieldtv-mdns/bundles/org.openhab.binding.shieldtv/src/main/java/org/openhab/binding/shieldtv/internal/discovery/ShieldTVDiscoveryParticipant.java

@mlobstein
Copy link
Contributor

And if someone can tell me what I'm doing wrong on mdns discovery I'd greatly appreciate it. From what I can tell it's not finding the binding as a participant.

Your createResult() method always returns null. It needs to return a DiscoveryResult. Take a look at the vizio binding for an example.

Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
@morph166955
Copy link
Contributor Author

And if someone can tell me what I'm doing wrong on mdns discovery I'd greatly appreciate it. From what I can tell it's not finding the binding as a participant.

Your createResult() method always returns null. It needs to return a DiscoveryResult. Take a look at the vizio binding for an example.

That was a byproduct of my debugging. I removed a large chunk of the createResult() code to make sure it wasn't causing the issue. I added it back in just now and retested. Nada.

I'm 99% sure I'm not even getting close to createResult() firing. From what I can tell, addMDNSDiscoveryParticipant() isn't firing at all. It looks like the mdns discovery side of openhab-core isn't seeing my MDNSDiscoveryParticipant.

Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
@mlobstein
Copy link
Contributor

I'm 99% sure I'm not even getting close to createResult() firing. From what I can tell, addMDNSDiscoveryParticipant() isn't firing at all. It looks like the mdns discovery side of openhab-core isn't seeing my MDNSDiscoveryParticipant.

I think you need to add this to feature.xml:
<feature>openhab-transport-mdns</feature>

@morph166955
Copy link
Contributor Author

I'm 99% sure I'm not even getting close to createResult() firing. From what I can tell, addMDNSDiscoveryParticipant() isn't firing at all. It looks like the mdns discovery side of openhab-core isn't seeing my MDNSDiscoveryParticipant.

I think you need to add this to feature.xml: <feature>openhab-transport-mdns</feature>

Problem was something stuck in my build environment, code was fine. I'm building cleanly now. Thanks to @mhilbush for the sanity check! I'll be sorting through the mdns today and hopefully commit the code later on. Thanks again!

morph166955 and others added 2 commits December 29, 2022 10:31
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
@morph166955
Copy link
Contributor Author

morph166955 commented Dec 29, 2022

Discovery tested and committed. Jar available at https://github.com/morph166955/openhab-addons/releases/download/shieldtv-beta-3/org.openhab.binding.shieldtv-4.0.0-SNAPSHOT-abfcf0e.jar

EDIT: 3.4.0 jar also now available for those who want to test it out: https://github.com/morph166955/openhab-addons/releases/download/shieldtv-beta-3/org.openhab.binding.shieldtv-3.4.0-SNAPSHOT-abfcf0e.jar

I have nothing else to submit to this minus any changes from reviews at this time.

@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/nvidia-shieldtv-binding/142203/6

@morph166955
Copy link
Contributor Author

morph166955 commented Dec 29, 2022

EDIT: Fixed this issue when updating copyright

@morph166955
Copy link
Contributor Author

Rebased to include 2023 copyright and re-added dispose() back to the handler.

morph166955 and others added 3 commits January 8, 2023 16:22
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
Signed-off-by: Ben Rosenblum <rosenblumb@gmail.com>
@morph166955
Copy link
Contributor Author

As this is still pending review, I've pulled the app support upgrades in from my beta branch so they can be included.

Copy link
Contributor

@clinique clinique left a comment

Choose a reason for hiding this comment

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

Minor observations

@morph166955
Copy link
Contributor Author

I appreciate all of the comments/review/feedback. In an effort to have this binding support a larger part of the community I've decided to re-spin this as an AndroidTV binding and not just a ShieldTV binding. Part of this will be the incorporation of additional protocols (like the GoogleTV v2 protocol). As such, I'm making some significant changes to the layout, naming conventions, etc. which will make this very complicated to try to merge into this PR. I'll be closing this PR and opening a new one in short order for an AndroidTV binding so that it's merged cleaner. I will make sure to make the changes from the reviews here as well, they all seem relevant to the code still. Thank you!

@morph166955
Copy link
Contributor Author

Just for note, all review comments have been fixed and merged into androidtv

@morph166955 morph166955 deleted the shieldtv branch March 14, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new binding If someone has started to work on a binding. For a new binding PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants