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

Draft: TV Casting APIs to recall previously connected TVs/Content Apps and target user/client selected ones #40

Closed
wants to merge 2 commits into from

Conversation

sharadb-amazon
Copy link
Owner

These APIs address the following issues:

  1. tv-casting-app: KVS storage management improvements project-chip/connectedhomeip#20075
  2. tv-casting-app: Add APIs to list all TV endpoints and sending commands to a specific endpoint project-chip/connectedhomeip#22779

Note: This is a draft PR to get feedback on the shape of the APIs themselves. The APIs are going to be very similar for iOS, except for mapping Java types to Objective C ones. There will an updated PR later with the actual implementation and this draft PR will NOT be merged in project-chip/master as such.

* @return Read all VideoPlayers the TvCastingApp knows about, from previous connections (may not
* be actively connected to some/any of them)
*/
public native List<VideoPlayer> readAllVideoPlayers();
Copy link

Choose a reason for hiding this comment

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

could we follow same verb for the method above and this one. So either get for both or read for both?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I used the verb "read" here deliberately to tell the client that we are reading from the app's cache in this API, rather than "getting" the list of video players over the network.

* @param discoveryFailureCallback
* @return true, if the discovery request was sent successfully, false otherwise
*/
public boolean discoverCommissioners(
Copy link

Choose a reason for hiding this comment

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

Will this discover any commissioner or only video players? If only video players does it make sense to rename this to discoverVideoPlayers?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sure, I'll update the name to discoverVideoPlayers

@@ -0,0 +1,13 @@
/** a.k.a content app like the PV app on a TV */
public class ContentApp {
Copy link

Choose a reason for hiding this comment

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

For reference : https://github.com/project-chip/connectedhomeip/blob/master/examples/tv-app/android/App/platform-app/src/main/java/com/matter/tv/server/model/ContentApp.java
Looks like all the properties are present. Maybe have a Set for cluster IDs to make clear that they cant be duplicate.

Copy link
Owner Author

@sharadb-amazon sharadb-amazon Oct 14, 2022

Choose a reason for hiding this comment

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

I am actually using the other option we discussed: that of letting the PV app fetch the attributes from the ApplicationBasic cluster directly, rather than packaging them in to this one class. That seems to be a simpler option, and the PV app can decide what information it needs to fetch. This ContentApp class will just contain the endpointId and the list of clusterIds. See project-chip#23181

private byte fabricIndex;

private boolean isConnected;
// Empty if we don't have a connection to the TargetVideoPlayer yet (for e.g. when the casting app
Copy link

Choose a reason for hiding this comment

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

Or if this is not a casting video player.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Currently, provided we're connected to the video player, we do return a list of all the endpointIds with all their clusterIds. How is a "casting video player" defined anyway - are we supposed to check for specific clusters on the endpoints it hosts?

Copy link

Choose a reason for hiding this comment

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

device type of the video player will be different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants