Skip to content

Commit ea9baa6

Browse files
committed
Some more code review
Signed-off-by: Christian Kittel <ckittel@gmx.de>
1 parent 613af2d commit ea9baa6

6 files changed

Lines changed: 54 additions & 52 deletions

File tree

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

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,21 @@ The Ntfy binding enables openHAB to publish notifications to Ntfy-compatible ser
44

55
Ntfy is a simple HTTP-based notification service and message broker; see [ntfy.sh](https://ntfy.sh) for details and public servers.
66

7-
It is intended for integrations where openHAB should push alert or informational messages to mobile or desktop clients that support the Ntfy protocol. The binding supports basic text messages as well as common rich features supported by the protocol: message priority, tags, icon URLs, attachments, click actions and simple action buttons.
7+
It is intended for integrations where openHAB should push alert or informational messages to mobile or desktop clients that support the Ntfy protocol.
8+
The binding supports basic text messages as well as common rich features supported by the protocol: message priority, tags, icon URLs, attachments, click actions and simple action buttons
89

910
Typical uses include doorbell alerts, security notifications, system health messages, or any automation that should notify users in real time via an Ntfy-compatible notification channel.
1011

1112
## Supported Things
1213

13-
This binding provides the following Thing types. The Thing type IDs match the definitions in `src/main/resources/OH-INF/thing/thing-types.xml`.
14+
This binding provides the following Thing types:
1415

15-
- `ntfy:ntfyConnection` (bridge) — Represents a connection to an Ntfy server. Configure the bridge with the server hostname (for example `https://ntfy.sh`) and optional credentials (username/password). The bridge holds shared connection settings (hostname, username, password, connectionTimeout) which are used by topic Things.
16-
- `ntfy:ntfyTopic` (Thing) — Represents a topic/channel on a configured Ntfy server. Each topic Thing must be associated with an `ntfyConnection` bridge and requires the `topicname` configuration parameter.
16+
- `server` (bridge) — Represents a connection to an Ntfy server. The bridge holds shared connection settings (hostname, username, password, connectionTimeout) which are used by topic Things.
17+
- `ntfy-Topic` (Thing) — Represents a topic/channel on a configured Ntfy server. Each topic Thing must be associated with a `server` bridge.
1718

1819
## Thing Configuration
1920

20-
### `ntfyConnection` Bridge Configuration
21+
### `server` Bridge Configuration
2122

2223
| Name | Type | Description | Default | Required | Advanced |
2324
|-------------------|---------|------------------------------------------------------------------------------------------|------------------------------------|----------|----------|
@@ -26,45 +27,44 @@ This binding provides the following Thing types. The Thing type IDs match the de
2627
| password | text | Optional password - if username is provided basic auth is used else Bearer token is used | N/A | no | no |
2728
| connectionTimeout | integer | WebSocket / HTTP connection timeout ms | 60000 | no | yes |
2829

29-
Configure the `ntfyConnection` as a bridge to hold shared server and authentication settings. Topic Things reference the bridge to reuse these settings. For authentication with access token only set the password and leave the username empty.
30+
Configure the `server` as a bridge to hold shared server and authentication settings.
31+
For authentication with access token only set the password and leave the username empty.
3032

31-
### `ntfyTopic` Thing Configuration
33+
### `ntfy-topic` Thing Configuration
3234

3335
| Name | Type | Description | Default | Required | Advanced |
3436
|-----------|------|---------------------------|---------|----------|----------|
3537
| topicname | text | Name of the topic/channel | N/A | yes | no |
3638

3739
## Channels
3840

39-
### `ntfyTopic` Channels
41+
### `ntfy-topic` Channels
4042

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) |
43+
| Channel | Type | Read/Write | Description |
44+
|-------------------|----------|------------|---------------------------------------------|
45+
| last-message | String | R | Last received message payload (read-only) |
46+
| last-message-time | DateTime | R | Timestamp of the last message (read-only) |
47+
| last-message-id | String | R | ID of the last message (read-only) |
4648

4749
## Full Example
4850

49-
Below are examples for textual configuration files showing a bridge (`ntfyConnection`), a topic Thing (`ntfyTopic`), Items bound to the Thing's channels, and a simple sitemap to display the last message and its timestamp.
51+
Below are examples for textual configuration files showing a bridge (`server`), a topic Thing (`ntfy-topic`), Items bound to the Thing's channels, and a simple sitemap to display the last message and its timestamp.
5052

5153
### Thing Configuration
5254

53-
```things
54-
Bridge ntfy:ntfyConnection:myConn "Ntfy Server" [ hostname="https://ntfy.sh", connectionTimeout=60000 ]
55+
```java
56+
Bridge ntfy:server:myConn "Ntfy Server" [ hostname="https://ntfy.sh", connectionTimeout=60000, username="ntfyUser", password="MyPaSsWoRd"]
5557

56-
Thing ntfy:ntfyTopic:home "Front Door Notifications" (ntfy:ntfyConnection:myConn) [ topicname="home" ]
58+
Thing ntfy:ntfy-topic:home "Front Door Notifications" (ntfy:server:myConn) [ topicname="home" ]
5759
```
5860

59-
If your server requires authentication, supply `username` and `password` in the bridge configuration. The `topicname` must be provided for each `ntfyTopic` Thing.
60-
6161
### Item Configuration (items file)
6262

6363
Bind Items to the read-only channels exposed by the topic Thing to show the last received message and its timestamp:
6464

65-
```items
66-
String FrontDoorLastMessage "Front Door Message" { channel="ntfy:ntfyTopic:home:lastMessage" }
67-
DateTime FrontDoorMessageTime "Last message received" { channel="ntfy:ntfyTopic:home:messageTime" }
65+
```java
66+
String FrontDoorLastMessage "Front Door Message" { channel="ntfy:ntfy-topic:home:last-message" }
67+
DateTime FrontDoorMessageTime "Last message received" { channel="ntfy:ntfy-topic:home:last-message-time" }
6868
```
6969

7070
Note: Sending notifications from openHAB to ntfy is done via the binding's Actions from rules (the binding exposes Actions to publish/delete messages). The channels above are read-only and show incoming or last-state values.
@@ -73,7 +73,7 @@ Note: Sending notifications from openHAB to ntfy is done via the binding's Actio
7373

7474
Simple sitemap demonstrating how to display the last message and its timestamp:
7575

76-
```sitemap
76+
```perl
7777
sitemap notifications label="Notifications"
7878
{
7979
<Frame label="Front Door">
@@ -85,9 +85,11 @@ Simple sitemap demonstrating how to display the last message and its timestamp:
8585

8686
## Actions (Rules DSL and JavaScript)
8787

88-
This binding exposes Rule Actions to send and manage messages on a configured topic. Actions are available for use from the Rules DSL and the ECMAScript/JavaScript automation scripts. The actions support a builder-style API to configure a message (message text, priority, tags, icon, attachments, actions, sequence id, ...) and then send it.
88+
This binding exposes Rule Actions to send and manage messages on a configured topic.
89+
Actions are available for use from the Rules DSL and the ECMAScript/JavaScript automation scripts.
90+
The actions support a builder-style API to configure a message (message text, priority, tags, icon, attachments, actions, sequence id, ...) and then send it.
8991

90-
Important: Always check that the returned actions object is not null (the Thing must exist and be handled by the binding) before invoking methods.
92+
Important: Always check that the Thing exist before invoking methods.
9193

9294
### Available action methods
9395

@@ -118,7 +120,7 @@ For more information about ntfy features and the notification format, see the nt
118120

119121
```rules
120122
// Obtain the Thing-specific actions and use the builder to send a message
121-
val bindingActions = getActions("ntfy", "ntfy:ntfyTopic:frontdoor")
123+
val bindingActions = getActions("ntfy", "ntfy:ntfy-topic:frontdoor")
122124
if (bindingActions !== null) {
123125
// simple one-liner: send a message
124126
val msgId = bindingActions.withMessage("Someone is at the door").withPriority(4).send()
@@ -144,7 +146,7 @@ In ECMAScript scripts you can obtain the Thing Actions and use the same builder
144146

145147
```javascript
146148
// get the Thing actions for the topic Thing
147-
var bindingActions = actions.get("ntfy", "ntfy:ntfyTopic:frontdoor");
149+
var bindingActions = actions.get("ntfy", "ntfy:ntfy-topic:frontdoor");
148150
if (bindingActions) {
149151
// send a simple message
150152
var id = bindingActions.withMessage("Garage opened").withPriority(3).send();

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

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@ public void initialize() {
8585

8686
config = getConfigAs(NtfyConnectionConfiguration.class);
8787

88-
updateStatus(ThingStatus.UNKNOWN);
89-
9088
String scheme;
9189
try {
9290
scheme = (new URI(config.hostname)).getScheme();
@@ -96,8 +94,7 @@ public void initialize() {
9694
}
9795
if (!"http".equals(scheme) && !"https".equals(scheme)) {
9896
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);
97+
"@text/offline.communication-error.unsupported-schema");
10198
return;
10299
}
103100

@@ -167,11 +164,10 @@ public synchronized boolean startWebSocketConnection(Thing topicThing, NtfyWebSo
167164
client.stop();
168165
} catch (InterruptedException e) {
169166
Thread.currentThread().interrupt();
170-
logger.error("WebSocket connection was interrupted", e);
171167
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getLocalizedMessage());
172168
return false;
173169
} catch (Exception e) {
174-
logger.warn("Error stopping WebSocket connection - ignore it and continue", e);
170+
logger.debug("Error stopping WebSocket connection - ignore it and continue", e);
175171
}
176172
}
177173
try {
@@ -198,7 +194,6 @@ public synchronized boolean startWebSocketConnection(Thing topicThing, NtfyWebSo
198194
return false;
199195
}
200196
updateStatus(ThingStatus.ONLINE);
201-
202197
}
203198
return true;
204199
}
@@ -215,10 +210,7 @@ private String getTopicNameFromThing(Thing topicThing) {
215210
*/
216211
public void connectionError(Throwable cause) {
217212
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, cause.getLocalizedMessage());
218-
retryConnectionFuture = scheduler.schedule(() -> {
219-
retryConnectionFuture = null;
220-
updateStatus(ThingStatus.ONLINE);
221-
}, 30, TimeUnit.SECONDS);
213+
retryConnectionFuture = scheduler.schedule(this::initialize, 30, TimeUnit.SECONDS);
222214
}
223215

224216
@Override

bundles/org.openhab.binding.ntfy/src/main/java/org/openhab/binding/ntfy/internal/NtfyTopicHandler.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.net.URISyntaxException;
1818
import java.util.Collection;
1919
import java.util.List;
20+
import java.util.concurrent.atomic.AtomicBoolean;
2021

2122
import org.eclipse.jdt.annotation.NonNullByDefault;
2223
import org.eclipse.jdt.annotation.Nullable;
@@ -51,7 +52,7 @@ public class NtfyTopicHandler extends BaseThingHandler implements WebSocketConne
5152

5253
private NtfyWebSocket ntfyWebSocket;
5354
private @Nullable NtfySender ntfySender;
54-
private boolean isInitializing;
55+
private AtomicBoolean isInitializing = new AtomicBoolean(false);
5556
private @Nullable MessageEvent lastMessageEvent;
5657

5758
/**
@@ -121,18 +122,17 @@ public void handleCommand(ChannelUID channelUID, Command command) {
121122

122123
@Override
123124
public void initialize() {
124-
if (isInitializing) {
125+
if (!isInitializing.compareAndSet(false, true)) {
125126
return;
126127
}
127-
this.isInitializing = true;
128128

129129
updateStatus(ThingStatus.UNKNOWN);
130130

131131
scheduler.execute(() -> {
132132
try {
133133
start(thing);
134134
} finally {
135-
this.isInitializing = false;
135+
isInitializing.set(false);
136136
}
137137
});
138138
}
@@ -157,7 +157,8 @@ public void connectionEstablished() {
157157
@Override
158158
public void connectionLost(@Nullable String reason) {
159159
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
160-
reason == null || reason.isBlank() ? "WebSocket connection lost" : reason);
160+
reason == null || reason.isBlank() ? "@text/offline.communication-error.websocket-connection-lost"
161+
: reason);
161162
}
162163

163164
@Override

bundles/org.openhab.binding.ntfy/src/main/java/org/openhab/binding/ntfy/internal/network/NtfySender.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,14 +143,14 @@ public NtfySender(String topicName, HttpClient httpClient, NtfyConnectionHandler
143143
return GsonDeserializer.deserialize(response.getContentAsString(), MessageEvent.class);
144144
}
145145
} catch (IOException e) {
146-
logger.error("Failed to read file for upload: {}", e.getMessage());
146+
logger.warn("Failed to read file for upload: {}", e.getMessage());
147147
return null;
148148
} catch (InterruptedException e) {
149149
Thread.currentThread().interrupt();
150-
logger.error("Failed to send message: {}", e.getMessage());
150+
logger.debug("Failed to send message: {}", e.getMessage());
151151
bridgeHandler.connectionError(e);
152152
} catch (TimeoutException | ExecutionException e) {
153-
logger.error("Failed to send file: {}", e.getMessage());
153+
logger.warn("Failed to send file: {}", e.getMessage());
154154
bridgeHandler.connectionError(e);
155155
}
156156
return null;
@@ -180,10 +180,10 @@ public boolean deleteMessage(String sequenceId) throws URISyntaxException {
180180
return HttpStatus.isSuccess(request.send().getStatus());
181181
} catch (InterruptedException e) {
182182
Thread.currentThread().interrupt();
183-
logger.error("Failed to delete message: {}", e.getMessage());
183+
logger.warn("Failed to delete message: {}", e.getMessage());
184184
bridgeHandler.connectionError(e);
185185
} catch (TimeoutException | ExecutionException e) {
186-
logger.error("Failed to delete message: {}", e.getMessage());
186+
logger.warn("Failed to delete message: {}", e.getMessage());
187187
bridgeHandler.connectionError(e);
188188
}
189189
return false;

bundles/org.openhab.binding.ntfy/src/main/resources/OH-INF/i18n/ntfy.properties

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ thing-type.ntfy.server.description = Connection to an ntfy server
1212

1313
# thing types config
1414

15-
thing-type.config.ntfy.ntfy-topic.topicName.label = Topic name
15+
thing-type.config.ntfy.ntfy-topic.topicName.label = Topic Name
1616
thing-type.config.ntfy.ntfy-topic.topicName.description = Name of the topic
1717
thing-type.config.ntfy.server.connectionTimeout.label = Connection timeout
1818
thing-type.config.ntfy.server.connectionTimeout.description = Connection timeout for web requests and web socket connections in milliseconds
@@ -28,3 +28,8 @@ thing-type.config.ntfy.server.username.description = Username to access the serv
2828
channel-type.ntfy.last-message-id.label = Message Id of the last message
2929
channel-type.ntfy.last-message-time.label = Time of the last message
3030
channel-type.ntfy.last-message.label = Last Message
31+
32+
# thing status descriptions
33+
34+
offline.communication-error.unsupported-schema = "URI scheme is missing in hostname or unsupported URI scheme (only http or https are supported): {0}"
35+
offline.communication-error.websocket-connection-lost = "WebSocket connection lost"

bundles/org.openhab.binding.ntfy/src/main/resources/OH-INF/thing/thing-types.xml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
<label>Ntfy Connection</label>
1010
<description>Connection to an ntfy server</description>
11+
<semantic-equipment-tag>WebService</semantic-equipment-tag>
1112
<representation-property>hostname</representation-property>
1213

1314
<config-description>
@@ -27,11 +28,12 @@
2728
<label>Password</label>
2829
<description>Password to access the server</description>
2930
</parameter>
30-
<parameter name="connectionTimeout" type="integer" required="false">
31+
<parameter name="connectionTimeout" type="integer" required="true">
3132
<context>time</context>
3233
<label>Connection timeout</label>
3334
<description>Connection timeout for web requests and web socket connections in milliseconds</description>
3435
<advanced>true</advanced>
36+
<default>60000</default>
3537
</parameter>
3638
</config-description>
3739
</bridge-type>
@@ -42,7 +44,7 @@
4244
</supported-bridge-type-refs>
4345
<label>Ntfy Topic</label>
4446
<description>Topic of a ntfy server</description>
45-
47+
<semantic-equipment-tag>WebService</semantic-equipment-tag>
4648
<channels>
4749
<channel id="last-message" typeId="last-message"/>
4850
<channel id="last-message-time" typeId="last-message-time"/>
@@ -54,7 +56,7 @@
5456
<config-description>
5557
<parameter name="topicName" type="text" required="true">
5658
<context>topicName</context>
57-
<label>Topic name</label>
59+
<label>Topic Name</label>
5860
<description>Name of the topic</description>
5961
</parameter>
6062
</config-description>

0 commit comments

Comments
 (0)