Skip to content

Commit 522e409

Browse files
EvilPinguEvilPingu
committed
code review changes
Signed-off-by: Christian Kittel <ckittel@gmx.de>
1 parent 5987547 commit 522e409

25 files changed

Lines changed: 524 additions & 189 deletions

‎bundles/org.openhab.binding.ntfy/README.md‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@ This binding provides the following Thing types. The Thing type IDs match the de
1919

2020
### `ntfyConnection` Bridge Configuration
2121

22-
| Name | Type | Description | Default | Required | Advanced |
23-
|-------------------|---------|-----------------------------------------|-----------------|----------|----------|
24-
| hostname | text | Hostname or base URL of the ntfy server | [https://ntfy.sh](https://ntfy.sh) | yes | no |
25-
| username | text | Optional username for basic auth | N/A | no | no |
26-
| password | text | Optional password for basic auth | N/A | no | no |
27-
| connectionTimeout | integer | WebSocket / HTTP connection timeout ms | 60000 | no | yes |
22+
| Name | Type | Description | Default | Required | Advanced |
23+
|-------------------|---------|-----------------------------------------|------------------------------------|----------|----------|
24+
| hostname | text | Base URL of the ntfy server | [https://ntfy.sh](https://ntfy.sh) | yes | no |
25+
| username | text | Optional username for basic auth | N/A | no | no |
26+
| password | text | Optional password for basic auth | N/A | no | no |
27+
| connectionTimeout | integer | WebSocket / HTTP connection timeout ms | 60000 | no | yes |
2828

2929
Configure the `ntfyConnection` as a bridge to hold shared server and authentication settings. Topic Things reference the bridge to reuse these settings.
3030

@@ -38,10 +38,11 @@ Configure the `ntfyConnection` as a bridge to hold shared server and authenticat
3838

3939
### `ntfyTopic` Channels
4040

41-
| Channel | Type | Read/Write | Description |
42-
|-------------|----------|------------|---------------------------------------------|
43-
| lastMessage | String | R | Last received message payload (read-only) |
44-
| messageTime | DateTime | R | Timestamp of the last message (read-only) |
41+
| Channel | Type | Read/Write | Description |
42+
|-----------------|----------|------------|---------------------------------------------|
43+
| lastMessage | String | R | Last received message payload (read-only) |
44+
| lastMessageTime | DateTime | R | Timestamp of the last message (read-only) |
45+
| lastMessageId | String | R | ID of the last message (read-only) |
4546

4647
## Full Example
4748

@@ -92,22 +93,23 @@ Important: Always check that the returned actions object is not null (the Thing
9293

9394
Below is a quick reference of the builder-style methods provided by the binding actions. Use these to construct messages before calling `send()`.
9495

95-
| Method | Parameters | Description | Ntfy docs |
96-
|--------|------------|-------------|----------|
97-
| withMessage | (String message) | Set the main message text. | [publish](https://docs.ntfy.sh/publish/#message-title) |
98-
| withPriority | (int priority) | Set message priority (0-5). | [priority](https://ntfy.sh/docs/publish/#priority) |
99-
| withTag | (String tag) | Add a tag to the message. | [tags](https://ntfy.sh/docs/publish/#tags) |
100-
| withIcon | (String url) | Set an icon URL for the notification. | [icons](https://ntfy.sh/docs/publish/#icons) |
101-
| withAttachment | (String url, String filename) | Attach a file or resource URL with optional filename. | [attachments](https://docs.ntfy.sh/publish/#attach-file-from-a-url) |
102-
| withViewAction | (String label, Boolean clearNotification, String url) | Add a view action (opens URL) with optional clear flag. | [actions](https://docs.ntfy.sh/publish/#open-websiteapp) |
103-
| withCopyAction | (String label, Boolean clearNotification, String value) | Add an action that copies text to clipboard on the client. | [actions](https://docs.ntfy.sh/publish/#copy-to-clipboard) |
104-
| withHttpAction | (String label, Boolean clearNotification, String url, String method, String headers, String body) | Add an HTTP action with optional method, headers and body. | [actions](https://docs.ntfy.sh/publish/#send-http-request) |
105-
| withBroadcastAction | (String label, Boolean clearNotification, String params) | Add a broadcast action to trigger local apps/handlers. | [actions](https://docs.ntfy.sh/publish/#send-android-broadcast) |
106-
| withDelay | (String delay) | Assign a delay. | [delay](https://docs.ntfy.sh/publish/#scheduled-delivery) |
107-
| withSequenceId | (String sequenceId) | Assign a sequence id for later reference (useful for delete/update). | [sequence id](https://docs.ntfy.sh/publish/#updating-deleting-notifications) |
108-
| send | () | Send the built message; returns a message ID string. | [publish](https://ntfy.sh/docs/publish/) |
109-
| send | (String file, String filename, String sequenceId) | Send a file; returns a message ID string. | [publish_local_file](https://docs.ntfy.sh/publish/#attach-local-file) |
110-
| delete | (String sequenceId) | Delete a message previously sent with the given sequence id. | [delete](https://ntfy.sh/docs/publish/#deleting-notifications) |
96+
| Method | Parameters | Description | Ntfy docs |
97+
|---------------------|---------------------------------------------------------------------------------------------------|----------------------------------------------------------------------|------------------------------------------------------------------------------|
98+
| withMessage | (String message) | Set the main message text. | [publish](https://docs.ntfy.sh/publish) |
99+
| withPriority | (int priority) | Set message priority (1-5). | [priority](https://ntfy.sh/docs/publish/#priority) |
100+
| withTitle | (String title) | Set title. | [title](https://docs.ntfy.sh/publish/#message-title) |
101+
| withTag | (String tag) | Add a tag to the message. | [tags](https://ntfy.sh/docs/publish/#tags) |
102+
| withIcon | (String url) | Set an icon URL for the notification. | [icons](https://ntfy.sh/docs/publish/#icons) |
103+
| withAttachment | (String url, String filename) | Attach a file or resource URL with optional filename. | [attachments](https://docs.ntfy.sh/publish/#attach-file-from-a-url) |
104+
| withViewAction | (String label, Boolean clearNotification, String url) | Add a view action (opens URL) with optional clear flag. | [actions](https://docs.ntfy.sh/publish/#open-websiteapp) |
105+
| withCopyAction | (String label, Boolean clearNotification, String value) | Add an action that copies text to clipboard on the client. | [actions](https://docs.ntfy.sh/publish/#copy-to-clipboard) |
106+
| withHttpAction | (String label, Boolean clearNotification, String url, String method, String headers, String body) | Add an HTTP action with optional method, headers and body. | [actions](https://docs.ntfy.sh/publish/#send-http-request) |
107+
| withBroadcastAction | (String label, Boolean clearNotification, String params) | Add a broadcast action to trigger local apps/handlers. | [actions](https://docs.ntfy.sh/publish/#send-android-broadcast) |
108+
| withDelay | (String delay) | Assign a delay. | [delay](https://docs.ntfy.sh/publish/#scheduled-delivery) |
109+
| withSequenceId | (String sequenceId) | Assign a sequence id for later reference (useful for delete/update). | [sequence id](https://docs.ntfy.sh/publish/#updating-deleting-notifications) |
110+
| send | () | Send the built message; returns a message ID string. | [publish](https://ntfy.sh/docs/publish/) |
111+
| send | (String file, String filename, String sequenceId) | Send a file; returns a message ID string. | [publish_local_file](https://docs.ntfy.sh/publish/#attach-local-file) |
112+
| delete | (String sequenceId) | Delete a message previously sent with the given sequence id. | [delete](https://ntfy.sh/docs/publish/#deleting-notifications) |
111113

112114
For more information about ntfy features and the notification format, see the ntfy project documentation: [https://ntfy.sh/docs/](https://ntfy.sh/docs/)
113115

‎bundles/org.openhab.binding.ntfy/src/main/java/org/openhab/binding/ntfy/internal/NtfyBindingConstants.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,14 @@
2424
@NonNullByDefault
2525
public class NtfyBindingConstants {
2626

27-
private static final String BINDING_ID = "ntfy";
27+
public static final String BINDING_ID = "ntfy";
2828

2929
// List of all Thing Type UIDs
30-
public static final ThingTypeUID NTFY_CONNECTION_THING = new ThingTypeUID(BINDING_ID, "ntfyConnection");
31-
public static final ThingTypeUID NTFY_TOPIC_THING = new ThingTypeUID(BINDING_ID, "ntfyTopic");
30+
public static final ThingTypeUID NTFY_CONNECTION_THING = new ThingTypeUID(BINDING_ID, "server");
31+
public static final ThingTypeUID NTFY_TOPIC_THING = new ThingTypeUID(BINDING_ID, "ntfy-topic");
3232

3333
// List of all Channel ids
34-
public static final String CHANNEL_NTFY_LASTMESSAGE = "lastMessage";
35-
public static final String CHANNEL_NTFY_LASTMESSAGETIME = "lastMessageTime";
36-
public static final String CHANNEL_NTFY_LASTMESSAGEID = "lastMessageMessageId";
34+
public static final String CHANNEL_LASTMESSAGE = "last-message";
35+
public static final String CHANNEL_LASTMESSAGETIME = "last-message-time";
36+
public static final String CHANNEL_LASTMESSAGEID = "last-message-id";
3737
}

‎bundles/org.openhab.binding.ntfy/src/main/java/org/openhab/binding/ntfy/internal/NtfyConnectionConfiguration.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
public class NtfyConnectionConfiguration {
2828

2929
/**
30-
* Hostname of the ntfy server, e.g. "ntfy.sh"
30+
* Hostname of the ntfy server, e.g. "https://ntfy.sh"
3131
*/
3232
public String hostname = "";
3333

@@ -54,6 +54,8 @@ public class NtfyConnectionConfiguration {
5454
* {@code false} otherwise
5555
*/
5656
public boolean isAuthHeaderNeeded() {
57+
final @Nullable String username = this.username;
58+
final @Nullable String password = this.password;
5759
return username != null && !username.isBlank() && password != null && !password.isBlank();
5860
}
5961

‎bundles/org.openhab.binding.ntfy/src/main/java/org/openhab/binding/ntfy/internal/NtfyConnectionHandler.java‎

Lines changed: 96 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,20 @@
1212
*/
1313
package org.openhab.binding.ntfy.internal;
1414

15+
import java.io.IOException;
1516
import java.net.URI;
17+
import java.net.URISyntaxException;
1618
import java.util.HashMap;
1719
import java.util.Map;
20+
import java.util.concurrent.ScheduledFuture;
1821
import java.util.concurrent.TimeUnit;
1922

2023
import org.eclipse.jdt.annotation.NonNullByDefault;
24+
import org.eclipse.jdt.annotation.Nullable;
25+
import org.eclipse.jetty.client.HttpClient;
2126
import org.eclipse.jetty.websocket.client.ClientUpgradeRequest;
2227
import org.eclipse.jetty.websocket.client.WebSocketClient;
28+
import org.openhab.binding.ntfy.internal.network.NtfySender;
2329
import org.openhab.binding.ntfy.internal.network.NtfyWebSocket;
2430
import org.openhab.core.io.net.http.WebSocketFactory;
2531
import org.openhab.core.thing.Bridge;
@@ -48,30 +54,63 @@ public class NtfyConnectionHandler extends BaseBridgeHandler {
4854
private NtfyConnectionConfiguration config;
4955
private Map<ThingUID, WebSocketClient> webSocketConnections = new HashMap<>();
5056
private WebSocketFactory webSocketFactory;
57+
private HttpClient httpClient;
58+
private @Nullable ScheduledFuture<?> retryConnectionFuture;
59+
private @Nullable String scheme;
5160

5261
/**
5362
* Creates a new {@link NtfyConnectionHandler} for the given bridge (connection) thing.
5463
*
5564
* @param thing the bridge thing representing the ntfy connection
5665
* @param webSocketFactory factory used to create WebSocketClient instances
5766
*/
58-
public NtfyConnectionHandler(Bridge thing, WebSocketFactory webSocketFactory) {
67+
public NtfyConnectionHandler(Bridge thing, WebSocketFactory webSocketFactory, HttpClient httpClient) {
5968
super(thing);
6069
this.webSocketFactory = webSocketFactory;
70+
this.httpClient = httpClient;
6171
config = getConfigAs(NtfyConnectionConfiguration.class);
6272
}
6373

6474
@Override
6575
public void handleCommand(ChannelUID channelUID, Command command) {
6676
}
6777

78+
public NtfySender CreateSender(String topicName) {
79+
return new NtfySender(topicName, httpClient, this);
80+
}
81+
6882
@Override
6983
public void initialize() {
70-
updateStatus(ThingStatus.UNKNOWN);
84+
cancelRetryFuture();
7185

7286
config = getConfigAs(NtfyConnectionConfiguration.class);
7387

74-
scheduler.execute(() -> updateStatus(ThingStatus.ONLINE));
88+
updateStatus(ThingStatus.UNKNOWN);
89+
90+
String scheme;
91+
try {
92+
scheme = (new URI(config.hostname)).getScheme();
93+
} catch (URISyntaxException e) {
94+
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, e.getLocalizedMessage());
95+
return;
96+
}
97+
if (!"http".equals(scheme) && !"https".equals(scheme)) {
98+
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
99+
"URI scheme is missing in hostname or unsupported URI scheme (only http or https are supported): "
100+
+ config.hostname);
101+
return;
102+
}
103+
104+
this.scheme = scheme;
105+
updateStatus(ThingStatus.ONLINE);
106+
}
107+
108+
private void cancelRetryFuture() {
109+
final @Nullable ScheduledFuture<?> retryConnectionFuture = this.retryConnectionFuture;
110+
this.retryConnectionFuture = null;
111+
if (retryConnectionFuture != null) {
112+
retryConnectionFuture.cancel(true);
113+
}
75114
}
76115

77116
/**
@@ -82,17 +121,18 @@ public void initialize() {
82121
* @param thing the thing for which to create and register the WebSocket client
83122
*/
84123
public synchronized void createAndRegisterWebSocketClient(Thing thing) {
85-
WebSocketClient newClient = webSocketFactory
86-
.createWebSocketClient(ThingWebClientUtil.buildWebClientConsumerName(thing.getUID(), "ntfy"));
124+
WebSocketClient newClient = webSocketFactory.createWebSocketClient(
125+
ThingWebClientUtil.buildWebClientConsumerName(thing.getUID(), NtfyBindingConstants.BINDING_ID));
87126

88127
WebSocketClient client = webSocketConnections.get(thing.getUID());
89128

90129
if (client != null && client.isRunning()) {
91130
try {
92131
client.stop();
93132
} catch (Exception e) {
94-
logger.error("Error stopping WebSocketConnection", e);
133+
logger.warn("Error stopping WebSocketConnection", e);
95134
}
135+
webSocketConnections.remove(thing.getUID());
96136
}
97137

98138
webSocketConnections.put(thing.getUID(), newClient);
@@ -122,28 +162,49 @@ public synchronized boolean startWebSocketConnection(Thing topicThing, NtfyWebSo
122162
String topicName = getTopicNameFromThing(topicThing);
123163

124164
if (client != null) {
125-
try {
126-
if (client.isStarted()) {
165+
if (client.isStarted()) {
166+
try {
127167
client.stop();
168+
} catch (InterruptedException e) {
169+
Thread.currentThread().interrupt();
170+
logger.error("WebSocket connection was interrupted", e);
171+
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getLocalizedMessage());
172+
return false;
173+
} catch (Exception e) {
174+
logger.warn("Error stopping WebSocket connection - ignore it and continue", e);
128175
}
176+
}
177+
try {
129178
client.start();
130-
client.setMaxIdleTimeout(config.connectionTimeout);
179+
} catch (InterruptedException e) {
180+
Thread.currentThread().interrupt();
181+
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getLocalizedMessage());
182+
return false;
183+
} catch (Exception e) {
184+
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getLocalizedMessage());
185+
return false;
186+
}
131187

188+
client.setMaxIdleTimeout(config.connectionTimeout);
189+
String webSocketScheme = "http".equals(this.scheme) ? "ws" : "wss";
190+
191+
try {
132192
client.connect(ntfyWebSocket,
133-
new URI("wss:"
193+
new URI(webSocketScheme + ":"
134194
+ (new URI(config.hostname + "/" + topicName + "/ws")).getRawSchemeSpecificPart()),
135195
setupRequest());
136-
} catch (Exception e) {
137-
logger.error("Error creating WebSocketConnection", e);
138-
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, e.getLocalizedMessage());
196+
} catch (IOException | URISyntaxException e) {
197+
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getLocalizedMessage());
139198
return false;
140199
}
200+
updateStatus(ThingStatus.ONLINE);
201+
141202
}
142203
return true;
143204
}
144205

145206
private String getTopicNameFromThing(Thing topicThing) {
146-
return topicThing.getConfiguration().as(NtfyTopicConfiguration.class).topicname;
207+
return topicThing.getConfiguration().as(NtfyTopicConfiguration.class).topicName;
147208
}
148209

149210
/**
@@ -154,6 +215,26 @@ private String getTopicNameFromThing(Thing topicThing) {
154215
*/
155216
public void connectionError(Throwable cause) {
156217
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, cause.getLocalizedMessage());
157-
scheduler.schedule(() -> updateStatus(ThingStatus.ONLINE), 30, TimeUnit.SECONDS);
218+
retryConnectionFuture = scheduler.schedule(() -> {
219+
retryConnectionFuture = null;
220+
updateStatus(ThingStatus.ONLINE);
221+
}, 30, TimeUnit.SECONDS);
222+
}
223+
224+
@Override
225+
public void dispose() {
226+
super.dispose();
227+
228+
cancelRetryFuture();
229+
230+
webSocketConnections.values().forEach(client -> {
231+
try {
232+
if (client.isRunning()) {
233+
client.stop();
234+
}
235+
} catch (Exception e) {
236+
logger.warn("Error stopping WebSocketConnection", e);
237+
}
238+
});
158239
}
159240
}

0 commit comments

Comments
 (0)