Skip to content

Commit 1b6e237

Browse files
jsettonjsetton
authored
[insteon] Fix duplicate scene entry feature listeners (openhab#18275)
Signed-off-by: Jeremy Setton <[email protected]>
1 parent f4338cc commit 1b6e237

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

275261
private void removeDevice(Console console, String[] args) {
276262
InsteonScene scene = getInsteonScene(args[1]);
263+
InsteonDevice device = getInsteonDevice(args[2]);
277264
if (scene == null) {
278265
console.println("The scene " + args[1] + " is not configured or enabled!");
279-
return;
280-
}
281-
InsteonDevice device = getInsteonDevice(args[2]);
282-
if (device == null) {
266+
} else if (device == null) {
283267
console.println("The device " + args[2] + " is not configured or enabled!");
284-
return;
285-
}
286-
DeviceFeature feature = device.getFeature(args[3]);
287-
if (feature == null) {
288-
console.println("The device " + args[2] + " feature " + args[3] + " is not configured!");
289-
return;
290-
}
291-
if (!device.getLinkDB().isComplete()) {
268+
} else if (!device.getLinkDB().isComplete()) {
292269
console.println("The link database for device " + args[2] + " is not loaded yet.");
293-
return;
294-
}
295-
if (!device.getLinkDB().getChanges().isEmpty()) {
296-
console.println("The link database for device " + args[2] + " has pending changes.");
297-
return;
298-
}
299-
if (!getModem().getDB().isComplete()) {
270+
} else if (!getModem().getDB().isComplete()) {
300271
console.println("The modem database is not loaded yet.");
301-
return;
302-
}
303-
if (!getModem().getDB().getChanges().isEmpty()) {
304-
console.println("The modem database has pending changes.");
305-
return;
306-
}
307-
if (!scene.hasEntry(device.getAddress(), feature.getName())) {
308-
console.println(
309-
"The device " + args[2] + " feature " + args[3] + " is not associated to scene" + args[1] + ".");
310-
return;
272+
} else {
273+
DeviceFeature feature = device.getFeature(args[3]);
274+
if (feature == null) {
275+
console.println("The device " + args[2] + " feature " + args[3] + " is not configured!");
276+
} else if (!scene.hasEntry(device.getAddress(), feature.getName())) {
277+
console.println("The device " + args[2] + " feature " + args[3] + " is not associated to scene"
278+
+ args[1] + ".");
279+
} else {
280+
console.println("Removing device " + device.getAddress() + " feature " + feature.getName()
281+
+ " from scene " + scene.getGroup() + ".");
282+
scene.removeDeviceFeature(device, feature.getComponentId());
283+
}
311284
}
312-
313-
console.println("Removing device " + device.getAddress() + " feature " + feature.getName() + " from scene "
314-
+ scene.getGroup() + ".");
315-
scene.removeDeviceFeature(device, feature.getComponentId());
316285
}
317286
}

‎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)