Skip to content

Conversation

lsiepel
Copy link
Contributor

@lsiepel lsiepel commented Sep 12, 2025

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.

Signed-off-by: Leo Siepel <leosiepel@gmail.com>
@lsiepel lsiepel requested a review from kaikreuzer as a code owner September 12, 2025 08:30
@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Sep 12, 2025

<config-description uri="thing-type:chromecast:device">
<parameter name="ipAddress" type="text" required="true">
<parameter name="host" type="text" required="true">
Copy link
Contributor Author

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.

Copy link
Contributor

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.

@lsiepel lsiepel requested a review from a team September 12, 2025 09:50
Comment on lines 62 to 69
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);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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);
}
}

Copy link
Contributor

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..?

Copy link
Contributor

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());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
statusUpdater.processStatusUpdate(event.getData(ReceiverStatusResponse.class).getStatus());
ReceiverStatusResponse receiverStatusResponse = event.getData(ReceiverStatusResponse.class);
statusUpdater.processStatusUpdate(receiverStatusResponse == null ? null : receiverStatusResponse.getStatus());

Copy link
Contributor

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.

@lsiepel lsiepel marked this pull request as draft September 13, 2025 19:33
Ravi Nadahar added 2 commits September 15, 2025 19:59
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An enhancement or new feature for an existing add-on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants