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

[pushover] Minor improvements for handling sounds, Fixed SAT findings #10840

Merged
merged 1 commit into from
Jun 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions bundles/org.openhab.binding.pushover/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -24,6 +27,17 @@
*/
@NonNullByDefault
public class PushoverAccountConfiguration {
public static final List<Sound> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,10 @@ public class PushoverConfigOptionProvider implements ConfigOptionProvider, Thing
@Override
public @Nullable Collection<ParameterOption> 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<Sound> sounds = accountHandler.getSounds();
List<Sound> sounds = localAccountHandler.getSounds();
if (!sounds.isEmpty()) {
return sounds.stream().map(Sound::getAsParameterOption)
.sorted(Comparator.comparing(ParameterOption::getLabel))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -103,9 +102,8 @@ public List<Sound> 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<String, String> requestParams) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -100,7 +100,14 @@ public Collection<Class<? extends ThingHandlerService>> getServices() {
* @return a list of {@link Sound}s
*/
public List<Sound> 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;
}

/**
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
</parameter>
<parameter name="sound" type="text">
<label>Notification Sound</label>
<description>The default notification sound on target device.</description>
<description>The notification sound on target device.</description>
<default>default</default>
</parameter>
<parameter name="retry" type="integer" min="30" unit="s">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down