From 9f98c8a1b9e71d55f10bbe34e3fb7251c63d8bf9 Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Sun, 30 May 2021 10:35:47 +0200 Subject: [PATCH] Minor improvements, SAT findings Signed-off-by: Christoph Weitkamp --- .../org.openhab.binding.pushover/README.md | 20 +++++++++---------- .../internal/actions/PushoverActions.java | 3 ++- .../config/PushoverAccountConfiguration.java | 14 +++++++++++++ .../config/PushoverConfigOptionProvider.java | 5 +++-- .../connection/PushoverAPIConnection.java | 6 ++---- .../connection/PushoverMessageBuilder.java | 6 ++++-- .../handler/PushoverAccountHandler.java | 13 +++++++++--- .../main/resources/OH-INF/config/config.xml | 2 +- .../OH-INF/i18n/pushover_de.properties | 4 +++- 9 files changed, 49 insertions(+), 24 deletions(-) diff --git a/bundles/org.openhab.binding.pushover/README.md b/bundles/org.openhab.binding.pushover/README.md index 314c846c386a4..7ae9fd4543654 100644 --- a/bundles/org.openhab.binding.pushover/README.md +++ b/bundles/org.openhab.binding.pushover/README.md @@ -12,16 +12,16 @@ You are able to create multiple instances of this Thing to broadcast to differen ## Thing Configuration -| Configuration Parameter | Type | Description | -|-------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------| -| `apikey` | text | Your API token / key (APP_TOKEN) to access the Pushover Message API. **mandatory** | -| `user` | text | Your user key or group key (USER_KEY) to which you want to push notifications. **mandatory** | -| `title` | text | The default title of a message (default: `openHAB`). | -| `format` | text | The default format (`none`, `html` or `monospace`) of a message (default: `none`). | -| `sound` | text | The default notification sound on target device (default: `default`) (see [supported notification sounds](https://pushover.net/api#sounds)). | -| `retry` | integer | The retry parameter specifies how often (in seconds) the Pushover servers will send the same notification to the user (default: `300`). **advanced** | -| `expire` | integer | The expire parameter specifies how long (in seconds) your notification will continue to be retried (default: `3600`). **advanced** | -| `timeout` | integer | The timeout parameter specifies maximum number of seconds a request to Pushover can take. **advanced** | +| Configuration Parameter | Type | Description | +|-------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `apikey` | text | Your API token / key (APP_TOKEN) to access the Pushover Message API. **mandatory** | +| `user` | text | Your user key or group key (USER_KEY) to which you want to push notifications. **mandatory** | +| `title` | text | The default title of a message (default: `openHAB`). | +| `format` | text | The default format (`none`, `html` or `monospace`) of a message (default: `none`). | +| `sound` | text | The notification sound on target device (default: `default`) (see [supported notification sounds](https://pushover.net/api#sounds)). This list will be populated dynamically during runtime with 21 different sounds plus user-defined [custom sounds](https://blog.pushover.net/posts/2021/3/custom-sounds). | +| `retry` | integer | The retry parameter specifies how often (in seconds) the Pushover servers will send the same notification to the user (default: `300`). **advanced** | +| `expire` | integer | The expire parameter specifies how long (in seconds) your notification will continue to be retried (default: `3600`). **advanced** | +| `timeout` | integer | The timeout parameter specifies maximum number of seconds a request to Pushover can take. **advanced** | The `retry` and `expire` parameters are only used for emergency-priority notifications. diff --git a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/actions/PushoverActions.java b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/actions/PushoverActions.java index ebef8cdc058d5..b09431d459782 100644 --- a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/actions/PushoverActions.java +++ b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/actions/PushoverActions.java @@ -59,7 +59,8 @@ public class PushoverActions implements ThingActions { message, title, sound, url, urlTitle, attachment, contentType, priority, device); PushoverMessageBuilder builder = getDefaultPushoverMessageBuilder(message); - if (sound != null) { + // add sound, if defined + if (sound != null && !DEFAULT_SOUND.equals(sound)) { builder.withSound(sound); } if (url != null) { diff --git a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/config/PushoverAccountConfiguration.java b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/config/PushoverAccountConfiguration.java index 4a39c0022f9ed..1b7a7211e9adb 100644 --- a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/config/PushoverAccountConfiguration.java +++ b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/config/PushoverAccountConfiguration.java @@ -14,8 +14,11 @@ import static org.openhab.binding.pushover.internal.PushoverBindingConstants.*; +import java.util.List; + import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.binding.pushover.internal.dto.Sound; /** * The {@link PushoverAccountConfiguration} class contains fields mapping thing configuration parameters. @@ -24,6 +27,17 @@ */ @NonNullByDefault public class PushoverAccountConfiguration { + public static final List DEFAULT_SOUNDS = List.of(new Sound("alien", "Alien Alarm (long)"), + new Sound("bike", "Bike"), new Sound("bugle", "Bugle"), new Sound("cashregister", "Cash Register"), + new Sound("classical", "Classical"), new Sound("climb", "Climb (long)"), new Sound("cosmic", "Cosmic"), + new Sound("falling", "Falling"), new Sound("gamelan", "Gamelan"), new Sound("incoming", "Incoming"), + new Sound("intermission", "Intermission"), new Sound("magic", "Magic"), + new Sound("mechanical", "Mechanical"), new Sound("none", "None (silent)"), + new Sound("persistent", "Persistent (long)"), new Sound("pianobar", "Piano Bar"), + new Sound("pushover", "Pushover (default)"), new Sound("echo", "Pushover Echo (long)"), + new Sound("siren", "Siren"), new Sound("spacealarm", "Space Alarm"), new Sound("tugboat", "Tug Boat"), + new Sound("updown", "Up Down (long)"), new Sound("vibrate", "Vibrate Only")); + public @Nullable String apikey; public @Nullable String user; public String title = DEFAULT_TITLE; diff --git a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/config/PushoverConfigOptionProvider.java b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/config/PushoverConfigOptionProvider.java index b9b0a77469b78..c3bce422be517 100644 --- a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/config/PushoverConfigOptionProvider.java +++ b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/config/PushoverConfigOptionProvider.java @@ -45,9 +45,10 @@ public class PushoverConfigOptionProvider implements ConfigOptionProvider, Thing @Override public @Nullable Collection getParameterOptions(URI uri, String param, @Nullable String context, @Nullable Locale locale) { - if (accountHandler != null && PUSHOVER_ACCOUNT.getAsString().equals(uri.getSchemeSpecificPart()) + PushoverAccountHandler localAccountHandler = accountHandler; + if (localAccountHandler != null && PUSHOVER_ACCOUNT.getAsString().equals(uri.getSchemeSpecificPart()) && CONFIG_SOUND.equals(param)) { - List sounds = accountHandler.getSounds(); + List sounds = localAccountHandler.getSounds(); if (!sounds.isEmpty()) { return sounds.stream().map(Sound::getAsParameterOption) .sorted(Comparator.comparing(ParameterOption::getLabel)) diff --git a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverAPIConnection.java b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverAPIConnection.java index 8f84241d44ba4..f7c5f2cbcbc41 100644 --- a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverAPIConnection.java +++ b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverAPIConnection.java @@ -14,7 +14,6 @@ import java.net.URLEncoder; import java.nio.charset.StandardCharsets; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -103,9 +102,8 @@ public List getSounds() throws PushoverCommunicationException, PushoverCo final JsonObject sounds = json == null || !json.has("sounds") ? null : json.get("sounds").getAsJsonObject(); return sounds == null ? List.of() - : Collections.unmodifiableList(sounds.entrySet().stream() - .map(entry -> new Sound(entry.getKey(), entry.getValue().getAsString())) - .collect(Collectors.toList())); + : sounds.entrySet().stream().map(entry -> new Sound(entry.getKey(), entry.getValue().getAsString())) + .collect(Collectors.toUnmodifiableList()); } private String buildURL(String url, Map requestParams) { diff --git a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverMessageBuilder.java b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverMessageBuilder.java index 7418a0c48389f..29424cc40d107 100644 --- a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverMessageBuilder.java +++ b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/connection/PushoverMessageBuilder.java @@ -163,7 +163,7 @@ public PushoverMessageBuilder withMonospaceFormatting() { return this; } - public ContentProvider build() { + public ContentProvider build() throws PushoverCommunicationException { if (message != null) { if (message.length() > MAX_MESSAGE_LENGTH) { throw new IllegalArgumentException(String.format( @@ -248,7 +248,9 @@ public ContentProvider build() { body.addFilePart(MESSAGE_KEY_ATTACHMENT, file.getName(), new PathContentProvider(contentType, file.toPath()), null); } catch (IOException e) { - throw new IllegalArgumentException(String.format("Skip sending the message: %s", e.getMessage())); + logger.debug("IOException occurred - skip sending message: {}", e.getLocalizedMessage(), e); + throw new PushoverCommunicationException( + String.format("Skip sending the message: %s", e.getLocalizedMessage()), e); } } diff --git a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/handler/PushoverAccountHandler.java b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/handler/PushoverAccountHandler.java index 9840bc23ae8c6..b0064aa200f13 100644 --- a/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/handler/PushoverAccountHandler.java +++ b/bundles/org.openhab.binding.pushover/src/main/java/org/openhab/binding/pushover/internal/handler/PushoverAccountHandler.java @@ -50,7 +50,7 @@ public class PushoverAccountHandler extends BaseThingHandler { private final HttpClient httpClient; - private @NonNullByDefault({}) PushoverAccountConfiguration config; + private PushoverAccountConfiguration config = new PushoverAccountConfiguration(); private @Nullable PushoverAPIConnection connection; public PushoverAccountHandler(Thing thing, HttpClient httpClient) { @@ -100,7 +100,14 @@ public Collection> getServices() { * @return a list of {@link Sound}s */ public List getSounds() { - return connection != null ? connection.getSounds() : List.of(); + try { + return connection != null ? connection.getSounds() : PushoverAccountConfiguration.DEFAULT_SOUNDS; + } catch (PushoverCommunicationException e) { + // do nothing, causing exception is already logged + } catch (PushoverConfigurationException e) { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, e.getMessage()); + } + return PushoverAccountConfiguration.DEFAULT_SOUNDS; } /** @@ -125,7 +132,7 @@ public PushoverMessageBuilder getDefaultPushoverMessageBuilder(String message) { default: break; } - // add sound if defined + // add sound, if defined if (!DEFAULT_SOUND.equals(config.sound)) { builder.withSound(config.sound); } diff --git a/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/config/config.xml b/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/config/config.xml index 09a61f1622773..9f4be6e413423 100644 --- a/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/config/config.xml +++ b/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/config/config.xml @@ -32,7 +32,7 @@ - The default notification sound on target device. + The notification sound on target device. default diff --git a/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover_de.properties b/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover_de.properties index 9f094e31e8f77..2ef1579529c77 100644 --- a/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover_de.properties +++ b/bundles/org.openhab.binding.pushover/src/main/resources/OH-INF/i18n/pushover_de.properties @@ -15,11 +15,13 @@ thing-type.config.pushover.pushover-account.title.description = Standardtitel de thing-type.config.pushover.pushover-account.format.label = Format thing-type.config.pushover.pushover-account.format.description = Standardformat der Nachricht. thing-type.config.pushover.pushover-account.sound.label = Benachrichtigungston -thing-type.config.pushover.pushover-account.sound.description = Standardbenachrichtigungston auf dem Endger�t. +thing-type.config.pushover.pushover-account.sound.description = Benachrichtigungston auf dem Endger�t. thing-type.config.pushover.pushover-account.retry.label = Wiederholungen thing-type.config.pushover.pushover-account.retry.description = Dieser Parameter gibt an, in welchen Abst�nden eine Priorit�tsnachricht wiederholt an den Benutzer gesendet werden soll. thing-type.config.pushover.pushover-account.expire.label = Verfall thing-type.config.pushover.pushover-account.expire.description = Dieser Parameter gibt an, wie lange eine Priorit�tsnachricht wiederholt wird. +thing-type.config.pushover.pushover-account.timeout.label = Timeout +thing-type.config.pushover.pushover-account.timeout.description = Dieser Parameter gibt an, wie lange eine Anfrage an die Pushover Message API maximal dauern darf. # user defined messages offline.conf-error-missing-apikey = Der Parameter 'apikey' muss konfiguriert werden.