Skip to content

Commit 30fa2d2

Browse files
lsiepellsiepel
authored
[govee] Fix invalid status response handling (openhab#17048)
Signed-off-by: Leo Siepel <[email protected]>
1 parent a48f72f commit 30fa2d2

File tree

4 files changed

+201
-10
lines changed

4 files changed

+201
-10
lines changed

‎bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
@NonNullByDefault
4848
@Component(service = CommunicationManager.class)
4949
public class CommunicationManager {
50+
private final Logger logger = LoggerFactory.getLogger(CommunicationManager.class);
5051
private final Gson gson = new Gson();
5152
// Holds a list of all thing handlers to send them thing updates via the receiver-Thread
5253
private final Map<String, GoveeHandler> thingHandlers = new HashMap<>();
@@ -101,7 +102,7 @@ public void sendRequest(GoveeHandler handler, GenericGoveeRequest request) throw
101102
final byte[] data = message.getBytes();
102103
final InetAddress address = InetAddress.getByName(hostname);
103104
DatagramPacket packet = new DatagramPacket(data, data.length, address, REQUEST_PORT);
104-
// logger.debug("Sending {} to {}", message, hostname);
105+
logger.trace("Sending {} to {}", message, hostname);
105106
socket.send(packet);
106107
socket.close();
107108
}
@@ -224,7 +225,9 @@ public void run() {
224225
}
225226
}
226227
} catch (JsonParseException e) {
227-
// this probably was a status message
228+
logger.debug(
229+
"JsonParseException when trying to parse the response, probably a status message",
230+
e);
228231
}
229232
} else {
230233
final @Nullable GoveeHandler handler;

‎bundles/org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java

+9-8
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import static org.openhab.binding.govee.internal.GoveeBindingConstants.*;
1616

1717
import java.io.IOException;
18+
import java.util.concurrent.ScheduledExecutorService;
1819
import java.util.concurrent.ScheduledFuture;
1920
import java.util.concurrent.TimeUnit;
2021

@@ -80,7 +81,7 @@ public class GoveeHandler extends BaseThingHandler {
8081
private static final Gson GSON = new Gson();
8182

8283
private final Logger logger = LoggerFactory.getLogger(GoveeHandler.class);
83-
84+
protected ScheduledExecutorService executorService = scheduler;
8485
@Nullable
8586
private ScheduledFuture<?> triggerStatusJob; // send device status update job
8687
private GoveeConfiguration goveeConfiguration = new GoveeConfiguration();
@@ -100,9 +101,7 @@ public class GoveeHandler extends BaseThingHandler {
100101
private final Runnable thingRefreshSender = () -> {
101102
try {
102103
triggerDeviceStatusRefresh();
103-
if (!thing.getStatus().equals(ThingStatus.ONLINE)) {
104-
updateStatus(ThingStatus.ONLINE);
105-
}
104+
updateStatus(ThingStatus.ONLINE);
106105
} catch (IOException e) {
107106
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
108107
"@text/offline.communication-error.could-not-query-device [\"" + goveeConfiguration.hostname
@@ -134,7 +133,7 @@ public void initialize() {
134133
if (triggerStatusJob == null) {
135134
logger.debug("Starting refresh trigger job for thing {} ", thing.getLabel());
136135

137-
triggerStatusJob = scheduler.scheduleWithFixedDelay(thingRefreshSender, 100,
136+
triggerStatusJob = executorService.scheduleWithFixedDelay(thingRefreshSender, 100,
138137
goveeConfiguration.refreshInterval * 1000L, TimeUnit.MILLISECONDS);
139138
}
140139
}
@@ -195,9 +194,7 @@ public void handleCommand(ChannelUID channelUID, Command command) {
195194
break;
196195
}
197196
}
198-
if (!thing.getStatus().equals(ThingStatus.ONLINE)) {
199-
updateStatus(ThingStatus.ONLINE);
200-
}
197+
updateStatus(ThingStatus.ONLINE);
201198
} catch (IOException e) {
202199
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
203200
"@text/offline.communication-error.could-not-query-device [\"" + goveeConfiguration.hostname
@@ -300,6 +297,10 @@ public void updateDeviceState(@Nullable StatusResponse message) {
300297
newColorTempInKelvin = (newColorTempInKelvin < COLOR_TEMPERATURE_MIN_VALUE)
301298
? COLOR_TEMPERATURE_MIN_VALUE.intValue()
302299
: newColorTempInKelvin;
300+
newColorTempInKelvin = (newColorTempInKelvin > COLOR_TEMPERATURE_MAX_VALUE)
301+
? COLOR_TEMPERATURE_MAX_VALUE.intValue()
302+
: newColorTempInKelvin;
303+
303304
int newColorTempInPercent = ((Double) ((newColorTempInKelvin - COLOR_TEMPERATURE_MIN_VALUE)
304305
/ (COLOR_TEMPERATURE_MAX_VALUE - COLOR_TEMPERATURE_MIN_VALUE) * 100.0)).intValue();
305306

‎bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeHandlerMock.java

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* Copyright (c) 2010-2024 Contributors to the openHAB project
3+
*
4+
* See the NOTICE file(s) distributed with this work for additional
5+
* information.
6+
*
7+
* This program and the accompanying materials are made available under the
8+
* terms of the Eclipse Public License 2.0 which is available at
9+
* http://www.eclipse.org/legal/epl-2.0
10+
*
11+
* SPDX-License-Identifier: EPL-2.0
12+
*/
13+
package org.openhab.binding.govee.internal;
14+
15+
import static org.mockito.ArgumentMatchers.any;
16+
import static org.mockito.ArgumentMatchers.anyLong;
17+
import static org.mockito.Mockito.doAnswer;
18+
19+
import java.util.concurrent.ScheduledExecutorService;
20+
import java.util.concurrent.TimeUnit;
21+
22+
import org.eclipse.jdt.annotation.NonNullByDefault;
23+
import org.mockito.Mockito;
24+
import org.mockito.invocation.InvocationOnMock;
25+
import org.openhab.core.thing.Thing;
26+
27+
/**
28+
* The {@link GoveeHandlerMock} is responsible for mocking {@link GoveeHandler}
29+
*
30+
* @author Leo Siepel - Initial contribution
31+
*/
32+
@NonNullByDefault
33+
public class GoveeHandlerMock extends GoveeHandler {
34+
35+
public GoveeHandlerMock(Thing thing, CommunicationManager communicationManager) {
36+
super(thing, communicationManager);
37+
38+
executorService = Mockito.mock(ScheduledExecutorService.class);
39+
doAnswer((InvocationOnMock invocation) -> {
40+
((Runnable) invocation.getArguments()[0]).run();
41+
return null;
42+
}).when(executorService).scheduleWithFixedDelay(any(Runnable.class), anyLong(), anyLong(), any(TimeUnit.class));
43+
}
44+
}

‎bundles/org.openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java

+143
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/**
2+
* Copyright (c) 2010-2024 Contributors to the openHAB project
3+
*
4+
* See the NOTICE file(s) distributed with this work for additional
5+
* information.
6+
*
7+
* This program and the accompanying materials are made available under the
8+
* terms of the Eclipse Public License 2.0 which is available at
9+
* http://www.eclipse.org/legal/epl-2.0
10+
*
11+
* SPDX-License-Identifier: EPL-2.0
12+
*/
13+
package org.openhab.binding.govee.internal;
14+
15+
import static org.mockito.ArgumentMatchers.argThat;
16+
import static org.mockito.ArgumentMatchers.eq;
17+
import static org.mockito.Mockito.mock;
18+
import static org.mockito.Mockito.spy;
19+
import static org.mockito.Mockito.verify;
20+
import static org.mockito.Mockito.when;
21+
22+
import java.util.Arrays;
23+
import java.util.List;
24+
25+
import javax.measure.Unit;
26+
27+
import org.eclipse.jdt.annotation.NonNullByDefault;
28+
import org.junit.jupiter.api.Test;
29+
import org.mockito.Mockito;
30+
import org.openhab.binding.govee.internal.model.StatusResponse;
31+
import org.openhab.core.config.core.Configuration;
32+
import org.openhab.core.library.types.DecimalType;
33+
import org.openhab.core.library.types.HSBType;
34+
import org.openhab.core.library.types.PercentType;
35+
import org.openhab.core.library.types.QuantityType;
36+
import org.openhab.core.library.unit.Units;
37+
import org.openhab.core.thing.Channel;
38+
import org.openhab.core.thing.ChannelUID;
39+
import org.openhab.core.thing.Thing;
40+
import org.openhab.core.thing.ThingStatus;
41+
import org.openhab.core.thing.ThingStatusDetail;
42+
import org.openhab.core.thing.ThingUID;
43+
import org.openhab.core.thing.binding.ThingHandlerCallback;
44+
import org.openhab.core.types.State;
45+
46+
import com.google.gson.Gson;
47+
48+
/**
49+
* @author Leo Siepel - Initial contribution
50+
*/
51+
@NonNullByDefault
52+
public class GoveeSerializeGoveeHandlerTest {
53+
54+
private static final Gson GSON = new Gson();
55+
private final String invalidValueJsonString = "{\"msg\": {\"cmd\": \"devStatus\", \"data\": {\"onOff\": 0, \"brightness\": 100, \"color\": {\"r\": 1, \"g\": 10, \"b\": 0}, \"colorTemInKelvin\": 9070}}}";
56+
57+
private static final Configuration CONFIG = createConfig(true);
58+
private static final Configuration BAD_CONFIG = createConfig(false);
59+
60+
private static Configuration createConfig(boolean returnValid) {
61+
final Configuration config = new Configuration();
62+
if (returnValid) {
63+
config.put("hostname", "1.2.3.4");
64+
}
65+
return config;
66+
}
67+
68+
private static Thing mockThing(boolean withConfiguration) {
69+
final Thing thing = mock(Thing.class);
70+
when(thing.getUID()).thenReturn(new ThingUID(GoveeBindingConstants.THING_TYPE_LIGHT, "govee-test-thing"));
71+
when(thing.getConfiguration()).thenReturn(withConfiguration ? CONFIG : BAD_CONFIG);
72+
73+
final List<Channel> channelList = Arrays.asList(
74+
mockChannel(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR), //
75+
mockChannel(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR_TEMPERATURE), //
76+
mockChannel(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR_TEMPERATURE_ABS));
77+
78+
when(thing.getChannels()).thenReturn(channelList);
79+
return thing;
80+
}
81+
82+
private static Channel mockChannel(final ThingUID thingId, final String channelId) {
83+
final Channel channel = Mockito.mock(Channel.class);
84+
when(channel.getUID()).thenReturn(new ChannelUID(thingId, channelId));
85+
return channel;
86+
}
87+
88+
private static GoveeHandlerMock createAndInitHandler(final ThingHandlerCallback callback, final Thing thing) {
89+
CommunicationManager communicationManager = mock(CommunicationManager.class);
90+
final GoveeHandlerMock handler = spy(new GoveeHandlerMock(thing, communicationManager));
91+
92+
handler.setCallback(callback);
93+
handler.initialize();
94+
95+
return handler;
96+
}
97+
98+
private static State getState(final int input, Unit<?> unit) {
99+
return new QuantityType<>(input, unit);
100+
}
101+
102+
@Test
103+
public void testInvalidConfiguration() {
104+
final Thing thing = mockThing(false);
105+
final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
106+
final GoveeHandlerMock handler = createAndInitHandler(callback, thing);
107+
108+
try {
109+
verify(callback).statusUpdated(eq(thing), argThat(arg -> arg.getStatus().equals(ThingStatus.OFFLINE)
110+
&& arg.getStatusDetail().equals(ThingStatusDetail.CONFIGURATION_ERROR)));
111+
} finally {
112+
handler.dispose();
113+
}
114+
}
115+
116+
@Test
117+
public void testInvalidResponseMessage() {
118+
final Thing thing = mockThing(true);
119+
final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
120+
final GoveeHandlerMock handler = createAndInitHandler(callback, thing);
121+
122+
// inject StatusResponseMessage
123+
StatusResponse statusMessage = GSON.fromJson(invalidValueJsonString, StatusResponse.class);
124+
handler.updateDeviceState(statusMessage);
125+
126+
try {
127+
verify(callback).statusUpdated(eq(thing), argThat(arg -> arg.getStatus().equals(ThingStatus.UNKNOWN)));
128+
129+
verify(callback).stateUpdated(new ChannelUID(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR),
130+
new HSBType(new DecimalType(114), new PercentType(100), new PercentType(0)));
131+
132+
verify(callback).stateUpdated(
133+
new ChannelUID(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR_TEMPERATURE),
134+
new PercentType(100));
135+
136+
verify(callback).stateUpdated(
137+
new ChannelUID(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR_TEMPERATURE_ABS),
138+
getState(2000, Units.KELVIN));
139+
} finally {
140+
handler.dispose();
141+
}
142+
}
143+
}

0 commit comments

Comments
 (0)