[androidtv] Adds CLI Interface

[morph166955]

Adds a CLI interface for the androidtv binding.

Resolves #15143

[morph166955]

Successful test:

2023-06-27 13:59:36.090 [TRACE] [al.console.AndroidTVCommandExtension] - Received CLI Command: androidtv:googletv:theater keypress |||KEY_POWER|||
2023-06-27 13:59:36.090 [DEBUG] [al.console.AndroidTVCommandExtension] - Sending CLI Command to Handler: androidtv:googletv:theater androidtv:googletv:theater:keypress |||KEY_POWER|||
2023-06-27 13:59:36.091 [TRACE] [.androidtv.internal.AndroidTVHandler] - theater - Command received at handler: keypress KEY_POWER
2023-06-27 13:59:36.091 [DEBUG] [l.googletv.GoogleTVConnectionManager] - theater - Command received: keypress

[wborn]

This may cause issues with some Markdown renderers as indicated by SAT: https://ci.openhab.org/job/PR-openHAB-Addons/16042/Static_20Code_20Analysis_20Report/

[jlaur]

Thanks. I have one doubt/concern about this approach: This generic way to update any channels kind of short-circuits the principle to integrate through items. For items there is already a console command that can be used with any binding: openhab:update. Also, there is now a dependency to the PIN code channel for this process.

[jlaur]

You could consider implementing ConsoleCommandCompleter for tab completion.

[morph166955]

Absolutely agreed and it's on my list. I wanted to get the i18n bits sorted first to reduce duplicate work. That PR is pending.

[morph166955]

I don't disagree that it does create a short cut, but I don't think people are interacting with OH at the console level where it would be a problem. The problem is that the way the binding is structured, with the single Handler and the individual protocol ConnectionManagers, the PIN logic would have to be significantly altered. Also, as I add additional protocols (both Philips and Sony are on my radar right now), it would increase complexity. This keeps the logic in one place and just allows a different entry point.

Also, this makes it slightly easier to troubleshoot and test the binding.

[morph166955]

@jlaur I pushed a commit to update the typographical changes as requested.

[morph166955]

@wborn @jlaur is there anything else we need to do on this before merge? I'd like to make sure this gets into 4.0.0 if we can.

[jlaur]

I don't disagree that it does create a short cut

The problem I see with this, is that you have implemented a generic way to interact with channels through the console.

If this is accepted, it would better to implement in core, so all bindings would have this possibility in a consistent way. Otherwise any binding could implement this in its own way, and this binding would have created precedence for that.

The problem is that the way the binding is structured, with the single Handler and the individual protocol ConnectionManagers, the PIN logic would have to be significantly altered.

I haven't checked the code yet, so can't comment on that, but it seems like you want to introduce the channel approach as a work-around for the current binding design?

To add some additional context, see also #15134 (comment)

I will let @wborn decide as he already reviewed and approved. I have expressed my point of view.

[morph166955]

No disagreement, this would be better done at the core level for every binding. Perhaps that's something to tag as a feature for 4.1.0. To play devils advocate, OH has never been a platform to force users to do things in a particular way. When I wrote this PR it made logical sense to me to enable the users to interact with the binding how they saw fit. Either via the item, through the API, or the cli, whichever they found easier or more appropriate for their use case. And I wouldn't call it a work around either. I chose to implement in this manner. That's not to discredit my previous statement that the binding would require restructuring, but that's not due to any design failing. The original design executes pin through an item, hence the logic being in the handleCommand side. This enhancement is providing access to that from another mechanism. I've personally found access to some of the other channels helpful during initial setup as well, so I don't want to create an artificial limitation either.

EDIT: I'll give a slightly different example. AndroidTVPKI shouldn't exist either. Those kind of PKI functions absolutely should exist in the core. Unfortunately, they just don't yet. I have a PR started to merge them into core for 4.1.0. When that happens, ill adapt this binding to use that instead.

[morph166955]

@wborn thoughts? Feature freeze is ~72 hours out and I'd really like to get this in for the release.

[wborn]

I have a PR started to merge them into core for 4.1.0. When that happens, ill adapt this binding to use that instead.

I'm OK with merging this to get the functionality into OH 4.0 and then adapt it to the generic approach in OH 4.1! 🙂

[morph166955]

Thank you!

[morph166955]

openhab/openhab-core#3705 has been opened to track moving this into the core.