Skip to content

Commit 21cd31f

Browse files
jsettonfabgio
jsetton
authored and
fabgio
committed
[insteon] Fix duplicate scene entry feature listeners (openhab#18275)
Signed-off-by: Jeremy Setton <[email protected]>
1 parent 0f55359 commit 21cd31f

File tree

2 files changed

+55
-95
lines changed

2 files changed

+55
-95
lines changed

‎bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/command/SceneCommand.java

+42-73
Original file line numberDiff line numberDiff line change
@@ -227,92 +227,61 @@ private void addDevice(Console console, String[] args) {
227227
InsteonDevice device = getInsteonDevice(args[2]);
228228
if (device == null) {
229229
console.println("The device " + args[2] + " is not configured or enabled!");
230-
return;
231-
}
232-
DeviceFeature feature = device.getFeature(args[3]);
233-
if (feature == null) {
234-
console.println("The device " + args[2] + " feature " + args[3] + " is not configured!");
235-
return;
236-
}
237-
if (!feature.isResponderFeature()) {
238-
console.println("The device " + args[2] + " feature " + args[3] + " is not a responder feature.");
239-
return;
240-
}
241-
if (!device.getLinkDB().isComplete()) {
230+
} else if (!device.getLinkDB().isComplete()) {
242231
console.println("The link database for device " + args[2] + " is not loaded yet.");
243-
return;
244-
}
245-
if (!device.getLinkDB().getChanges().isEmpty()) {
246-
console.println("The link database for device " + args[2] + " has pending changes.");
247-
return;
248-
}
249-
if (!getModem().getDB().isComplete()) {
232+
} else if (!getModem().getDB().isComplete()) {
250233
console.println("The modem database is not loaded yet.");
251-
return;
252-
}
253-
if (!getModem().getDB().getChanges().isEmpty()) {
254-
console.println("The modem database has pending changes.");
255-
return;
256-
}
257-
int onLevel = OnLevel.getHexValue(args[4], feature.getType());
258-
if (onLevel == -1) {
259-
console.println("The feature " + args[3] + " onLevel " + args[4] + " is not valid.");
260-
return;
261-
}
262-
RampRate rampRate = null;
263-
if (RampRate.supportsFeatureType(feature.getType())) {
264-
rampRate = args.length == 6 ? RampRate.fromString(args[5]) : RampRate.DEFAULT;
265-
if (rampRate == null) {
266-
console.println("The feature " + args[3] + " rampRate " + args[5] + " is not valid.");
267-
return;
234+
} else {
235+
DeviceFeature feature = device.getFeature(args[3]);
236+
if (feature == null) {
237+
console.println("The device " + args[2] + " feature " + args[3] + " is not configured!");
238+
} else if (!feature.isResponderFeature()) {
239+
console.println("The device " + args[2] + " feature " + args[3] + " is not a responder feature.");
240+
} else {
241+
int onLevel = OnLevel.getHexValue(args[4], feature.getType());
242+
if (onLevel == -1) {
243+
console.println("The feature " + args[3] + " onLevel " + args[4] + " is not valid.");
244+
return;
245+
}
246+
RampRate rampRate = null;
247+
if (RampRate.supportsFeatureType(feature.getType())) {
248+
rampRate = args.length == 6 ? RampRate.fromString(args[5]) : RampRate.DEFAULT;
249+
if (rampRate == null) {
250+
console.println("The feature " + args[3] + " rampRate " + args[5] + " is not valid.");
251+
return;
252+
}
253+
}
254+
255+
console.println("Adding device " + device.getAddress() + " feature " + feature.getName() + " to scene "
256+
+ scene.getGroup() + ".");
257+
scene.addDeviceFeature(device, onLevel, rampRate, feature.getComponentId());
268258
}
269259
}
270-
271-
console.println("Adding device " + device.getAddress() + " feature " + feature.getName() + " to scene "
272-
+ scene.getGroup() + ".");
273-
scene.addDeviceFeature(device, onLevel, rampRate, feature.getComponentId());
274260
}
275261

276262
private void removeDevice(Console console, String[] args) {
277263
InsteonScene scene = getInsteonScene(args[1]);
264+
InsteonDevice device = getInsteonDevice(args[2]);
278265
if (scene == null) {
279266
console.println("The scene " + args[1] + " is not configured or enabled!");
280-
return;
281-
}
282-
InsteonDevice device = getInsteonDevice(args[2]);
283-
if (device == null) {
267+
} else if (device == null) {
284268
console.println("The device " + args[2] + " is not configured or enabled!");
285-
return;
286-
}
287-
DeviceFeature feature = device.getFeature(args[3]);
288-
if (feature == null) {
289-
console.println("The device " + args[2] + " feature " + args[3] + " is not configured!");
290-
return;
291-
}
292-
if (!device.getLinkDB().isComplete()) {
269+
} else if (!device.getLinkDB().isComplete()) {
293270
console.println("The link database for device " + args[2] + " is not loaded yet.");
294-
return;
295-
}
296-
if (!device.getLinkDB().getChanges().isEmpty()) {
297-
console.println("The link database for device " + args[2] + " has pending changes.");
298-
return;
299-
}
300-
if (!getModem().getDB().isComplete()) {
271+
} else if (!getModem().getDB().isComplete()) {
301272
console.println("The modem database is not loaded yet.");
302-
return;
303-
}
304-
if (!getModem().getDB().getChanges().isEmpty()) {
305-
console.println("The modem database has pending changes.");
306-
return;
307-
}
308-
if (!scene.hasEntry(device.getAddress(), feature.getName())) {
309-
console.println(
310-
"The device " + args[2] + " feature " + args[3] + " is not associated to scene" + args[1] + ".");
311-
return;
273+
} else {
274+
DeviceFeature feature = device.getFeature(args[3]);
275+
if (feature == null) {
276+
console.println("The device " + args[2] + " feature " + args[3] + " is not configured!");
277+
} else if (!scene.hasEntry(device.getAddress(), feature.getName())) {
278+
console.println("The device " + args[2] + " feature " + args[3] + " is not associated to scene"
279+
+ args[1] + ".");
280+
} else {
281+
console.println("Removing device " + device.getAddress() + " feature " + feature.getName()
282+
+ " from scene " + scene.getGroup() + ".");
283+
scene.removeDeviceFeature(device, feature.getComponentId());
284+
}
312285
}
313-
314-
console.println("Removing device " + device.getAddress() + " feature " + feature.getName() + " from scene "
315-
+ scene.getGroup() + ".");
316-
scene.removeDeviceFeature(device, feature.getComponentId());
317286
}
318287
}

‎bundles/org.openhab.binding.insteon/src/main/java/org/openhab/binding/insteon/internal/device/InsteonScene.java

+13-22
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,10 @@ public String toString() {
133133
* @param entry the scene entry to add
134134
*/
135135
private void addEntry(SceneEntry entry) {
136-
logger.trace("adding entry to scene {}: {}", group, entry);
137-
138136
synchronized (entries) {
139137
if (entries.add(entry)) {
140138
entry.register();
139+
logger.trace("added entry to scene {}: {}", group, entry);
141140
}
142141
}
143142
}
@@ -168,18 +167,14 @@ public void deleteEntries() {
168167
* @param address the device address
169168
*/
170169
public void deleteEntries(InsteonAddress address) {
171-
logger.trace("removing entries from scene {} for device {}", group, address);
172-
173170
getEntries(address).forEach(this::deleteEntry);
174171
}
175172

176173
/**
177174
* Updates all entries for this scene
178175
*/
179176
public void updateEntries() {
180-
synchronized (entries) {
181-
entries.clear();
182-
}
177+
deleteEntries();
183178

184179
InsteonModem modem = getModem();
185180
if (modem != null) {
@@ -204,7 +199,7 @@ public void updateEntries(InsteonDevice device) {
204199

205200
logger.trace("updating entries for scene {} device {}", group, address);
206201

207-
getEntries(address).forEach(this::deleteEntry);
202+
deleteEntries(address);
208203

209204
InsteonModem modem = getModem();
210205
if (modem != null) {
@@ -325,13 +320,17 @@ public void refresh() {
325320
public class SceneEntry implements FeatureListener {
326321
private InsteonAddress address;
327322
private DeviceFeature feature;
328-
private byte[] data;
323+
private State onState;
324+
private @Nullable RampRate rampRate;
329325
private State state = UnDefType.NULL;
330326

331327
public SceneEntry(InsteonAddress address, DeviceFeature feature, byte[] data) {
332328
this.address = address;
333329
this.feature = feature;
334-
this.data = data;
330+
this.onState = OnLevel.getState(Byte.toUnsignedInt(data[0]), feature.getType());
331+
this.rampRate = RampRate.supportsFeatureType(feature.getType())
332+
? RampRate.valueOf(Byte.toUnsignedInt(data[1]))
333+
: null;
335334
}
336335

337336
public InsteonAddress getAddress() {
@@ -342,14 +341,6 @@ public DeviceFeature getFeature() {
342341
return feature;
343342
}
344343

345-
public State getOnState() {
346-
return OnLevel.getState(Byte.toUnsignedInt(data[0]), feature.getType());
347-
}
348-
349-
public RampRate getRampRate() {
350-
return RampRate.valueOf(Byte.toUnsignedInt(data[1]));
351-
}
352-
353344
public State getState() {
354345
return state;
355346
}
@@ -359,7 +350,7 @@ public boolean isStateDefined() {
359350
}
360351

361352
public boolean isStateOn() {
362-
return getOnState().equals(state);
353+
return onState.equals(state);
363354
}
364355

365356
public void setState(State state) {
@@ -378,9 +369,9 @@ public void unregister() {
378369

379370
@Override
380371
public String toString() {
381-
String s = address + " " + feature.getName() + " currentState: " + state + " onState: " + getOnState();
382-
if (RampRate.supportsFeatureType(feature.getType())) {
383-
s += " rampRate: " + getRampRate();
372+
String s = address + " " + feature.getName() + " currentState: " + state + " onState: " + onState;
373+
if (rampRate != null) {
374+
s += " rampRate: " + rampRate;
384375
}
385376
return s;
386377
}

0 commit comments

Comments
 (0)