Skip to content
This repository has been archived by the owner. It is now read-only.

Improve Sonos notification timeout handling #3808

Merged
merged 1 commit into from Jul 6, 2017
Merged

Improve Sonos notification timeout handling #3808

merged 1 commit into from Jul 6, 2017

Conversation

@ivivanov-bg
Copy link
Contributor

@ivivanov-bg ivivanov-bg commented Jul 6, 2017

  1. Extract the notification timeout as a thing configuration parameter
  2. Increase the default notification timeout to 40 sec. (as 20 s is too short time)
<default>60</default>
</parameter>
</config-description>
</config-description:config-descriptions>
Copy link
Contributor

@hakan42 hakan42 Jul 6, 2017

Please add a linefeed at the end of this file.

Copy link
Contributor Author

@ivivanov-bg ivivanov-bg Jul 6, 2017

done

updateConfiguration(c);
}

this.notificationTimeout = getConfigAs(ZonePlayerConfiguration.class).notificationTimeout;
Copy link
Contributor

@hakan42 hakan42 Jul 6, 2017

Please initiliaze the notification Timeout to the default here and don't use the NPE-safety check in the getter?

Copy link
Contributor Author

@ivivanov-bg ivivanov-bg Jul 6, 2017

done

@@ -2801,4 +2815,8 @@ private long sleepStrTimeToSeconds(String sleepTime) {
int seconds = Integer.parseInt(units[2]);
return 3600 * hours + 60 * minutes + seconds;
}

protected Integer getNotificationTimeout() {
return this.notificationTimeout != null ? this.notificationTimeout : DEFAULT_NOTIFICATION_TIMEOUT;
Copy link
Contributor

@hakan42 hakan42 Jul 6, 2017

This safety check is not necessary (and the getter a standard getter method) if you initialize the default in the initialize() method (see my previous comment)

2. Increase the default notification timeout to 40 sec. (as 20 s is too short time)

Signed-off-by: Ivaylo Ivanov <ivivanov.bg@gmail.com>
@sjsf sjsf merged commit db27358 into eclipse-archived:master Jul 6, 2017
2 checks passed
@sjsf
Copy link
Contributor

@sjsf sjsf commented Jul 6, 2017

Thank you both!

maggu2810 added a commit that referenced this issue Jul 25, 2017
Fix bug introduced by PR #3808

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@ivivanov-bg ivivanov-bg deleted the sonos_notification_duration_param branch Aug 18, 2017
@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017
@kaikreuzer kaikreuzer added the bug label Dec 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants