-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[chromecast] Make use of events #19319
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
|
||
<config-description uri="thing-type:chromecast:device"> | ||
<parameter name="ipAddress" type="text" required="true"> | ||
<parameter name="host" type="text" required="true"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To the reviewer, the hostname can be used, so i think ipAddress is the wrong name, but it is also breaking, so i have some doubts to change it or not. I can also update the docs, that ipAddress
can also contain a hostname, but meh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't it at least be explained in the parameter description? That wouldn't break anything.
List<MediaStatus> statuses = event.getData(MediaStatusResponse.class).getStatuses(); | ||
if (statuses != null && statuses.size() > 1) { | ||
logger.warn("Received multiple media statuses, this is not supported. Statuses: {}", statuses); | ||
} else if (statuses != null && !statuses.isEmpty()) { | ||
statusUpdater.updateMediaStatus(statuses.getFirst()); | ||
} else { | ||
statusUpdater.updateMediaStatus(null); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List<MediaStatus> statuses = event.getData(MediaStatusResponse.class).getStatuses(); | |
if (statuses != null && statuses.size() > 1) { | |
logger.warn("Received multiple media statuses, this is not supported. Statuses: {}", statuses); | |
} else if (statuses != null && !statuses.isEmpty()) { | |
statusUpdater.updateMediaStatus(statuses.getFirst()); | |
} else { | |
statusUpdater.updateMediaStatus(null); | |
} | |
MediaStatusResponse mediaStatusResponse = event.getData(MediaStatusResponse.class); | |
List<MediaStatus> mediaStatuses = mediaStatusResponse == null ? null : mediaStatusResponse.getStatuses(); | |
if (mediaStatuses == null) { | |
statusUpdater.updateMediaStatus(null); | |
} else { | |
for (MediaStatus mediaStatus : mediaStatuses) { | |
statusUpdater.updateMediaStatus(mediaStatus); | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There will be multiple statuses at times. There's no point in "rejecting" that, just apply them in order..?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I remember, certain "actions" involve transitions that produce more than one media status in sequence.
case APPEVENT: | ||
logger.debug("Received an 'APPEVENT' event, ignoring"); | ||
case RECEIVER_STATUS: | ||
statusUpdater.processStatusUpdate(event.getData(ReceiverStatusResponse.class).getStatus()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
statusUpdater.processStatusUpdate(event.getData(ReceiverStatusResponse.class).getStatus()); | |
ReceiverStatusResponse receiverStatusResponse = event.getData(ReceiverStatusResponse.class); | |
statusUpdater.processStatusUpdate(receiverStatusResponse == null ? null : receiverStatusResponse.getStatus()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Despite what Eclipse's nullness logic claims, getData()
very much can return null
.
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
Main purpose was to change the library to a more maintained version from @Nadahar.
This made it also possible to change from data polling to events, making the binding a bit more snappy with updates.
Improved scheduled task disposal.