Skip to content

Commit

Permalink
[govee] Fix invalid status response handling (openhab#17048)
Browse files Browse the repository at this point in the history
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
  • Loading branch information
lsiepel authored and andrewfg committed Jul 18, 2024
1 parent 9739a2a commit 96abfd2
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
@NonNullByDefault
@Component(service = CommunicationManager.class)
public class CommunicationManager {
private final Logger logger = LoggerFactory.getLogger(CommunicationManager.class);
private final Gson gson = new Gson();
// Holds a list of all thing handlers to send them thing updates via the receiver-Thread
private final Map<String, GoveeHandler> thingHandlers = new HashMap<>();
Expand Down Expand Up @@ -101,7 +102,7 @@ public void sendRequest(GoveeHandler handler, GenericGoveeRequest request) throw
final byte[] data = message.getBytes();
final InetAddress address = InetAddress.getByName(hostname);
DatagramPacket packet = new DatagramPacket(data, data.length, address, REQUEST_PORT);
// logger.debug("Sending {} to {}", message, hostname);
logger.trace("Sending {} to {}", message, hostname);
socket.send(packet);
socket.close();
}
Expand Down Expand Up @@ -224,7 +225,9 @@ public void run() {
}
}
} catch (JsonParseException e) {
// this probably was a status message
logger.debug(
"JsonParseException when trying to parse the response, probably a status message",
e);
}
} else {
final @Nullable GoveeHandler handler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.openhab.binding.govee.internal.GoveeBindingConstants.*;

import java.io.IOException;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

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

private final Logger logger = LoggerFactory.getLogger(GoveeHandler.class);

protected ScheduledExecutorService executorService = scheduler;
@Nullable
private ScheduledFuture<?> triggerStatusJob; // send device status update job
private GoveeConfiguration goveeConfiguration = new GoveeConfiguration();
Expand All @@ -100,9 +101,7 @@ public class GoveeHandler extends BaseThingHandler {
private final Runnable thingRefreshSender = () -> {
try {
triggerDeviceStatusRefresh();
if (!thing.getStatus().equals(ThingStatus.ONLINE)) {
updateStatus(ThingStatus.ONLINE);
}
updateStatus(ThingStatus.ONLINE);
} catch (IOException e) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
"@text/offline.communication-error.could-not-query-device [\"" + goveeConfiguration.hostname
Expand Down Expand Up @@ -134,7 +133,7 @@ public void initialize() {
if (triggerStatusJob == null) {
logger.debug("Starting refresh trigger job for thing {} ", thing.getLabel());

triggerStatusJob = scheduler.scheduleWithFixedDelay(thingRefreshSender, 100,
triggerStatusJob = executorService.scheduleWithFixedDelay(thingRefreshSender, 100,
goveeConfiguration.refreshInterval * 1000L, TimeUnit.MILLISECONDS);
}
}
Expand Down Expand Up @@ -195,9 +194,7 @@ public void handleCommand(ChannelUID channelUID, Command command) {
break;
}
}
if (!thing.getStatus().equals(ThingStatus.ONLINE)) {
updateStatus(ThingStatus.ONLINE);
}
updateStatus(ThingStatus.ONLINE);
} catch (IOException e) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
"@text/offline.communication-error.could-not-query-device [\"" + goveeConfiguration.hostname
Expand Down Expand Up @@ -300,6 +297,10 @@ public void updateDeviceState(@Nullable StatusResponse message) {
newColorTempInKelvin = (newColorTempInKelvin < COLOR_TEMPERATURE_MIN_VALUE)
? COLOR_TEMPERATURE_MIN_VALUE.intValue()
: newColorTempInKelvin;
newColorTempInKelvin = (newColorTempInKelvin > COLOR_TEMPERATURE_MAX_VALUE)
? COLOR_TEMPERATURE_MAX_VALUE.intValue()
: newColorTempInKelvin;

int newColorTempInPercent = ((Double) ((newColorTempInKelvin - COLOR_TEMPERATURE_MIN_VALUE)
/ (COLOR_TEMPERATURE_MAX_VALUE - COLOR_TEMPERATURE_MIN_VALUE) * 100.0)).intValue();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* Copyright (c) 2010-2024 Contributors to the openHAB project
*
* See the NOTICE file(s) distributed with this work for additional
* information.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.openhab.binding.govee.internal;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.doAnswer;

import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
import org.openhab.core.thing.Thing;

/**
* The {@link GoveeHandlerMock} is responsible for mocking {@link GoveeHandler}
*
* @author Leo Siepel - Initial contribution
*/
@NonNullByDefault
public class GoveeHandlerMock extends GoveeHandler {

public GoveeHandlerMock(Thing thing, CommunicationManager communicationManager) {
super(thing, communicationManager);

executorService = Mockito.mock(ScheduledExecutorService.class);
doAnswer((InvocationOnMock invocation) -> {
((Runnable) invocation.getArguments()[0]).run();
return null;
}).when(executorService).scheduleWithFixedDelay(any(Runnable.class), anyLong(), anyLong(), any(TimeUnit.class));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/**
* Copyright (c) 2010-2024 Contributors to the openHAB project
*
* See the NOTICE file(s) distributed with this work for additional
* information.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.openhab.binding.govee.internal;

import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.util.Arrays;
import java.util.List;

import javax.measure.Unit;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.openhab.binding.govee.internal.model.StatusResponse;
import org.openhab.core.config.core.Configuration;
import org.openhab.core.library.types.DecimalType;
import org.openhab.core.library.types.HSBType;
import org.openhab.core.library.types.PercentType;
import org.openhab.core.library.types.QuantityType;
import org.openhab.core.library.unit.Units;
import org.openhab.core.thing.Channel;
import org.openhab.core.thing.ChannelUID;
import org.openhab.core.thing.Thing;
import org.openhab.core.thing.ThingStatus;
import org.openhab.core.thing.ThingStatusDetail;
import org.openhab.core.thing.ThingUID;
import org.openhab.core.thing.binding.ThingHandlerCallback;
import org.openhab.core.types.State;

import com.google.gson.Gson;

/**
* @author Leo Siepel - Initial contribution
*/
@NonNullByDefault
public class GoveeSerializeGoveeHandlerTest {

private static final Gson GSON = new Gson();
private final String invalidValueJsonString = "{\"msg\": {\"cmd\": \"devStatus\", \"data\": {\"onOff\": 0, \"brightness\": 100, \"color\": {\"r\": 1, \"g\": 10, \"b\": 0}, \"colorTemInKelvin\": 9070}}}";

private static final Configuration CONFIG = createConfig(true);
private static final Configuration BAD_CONFIG = createConfig(false);

private static Configuration createConfig(boolean returnValid) {
final Configuration config = new Configuration();
if (returnValid) {
config.put("hostname", "1.2.3.4");
}
return config;
}

private static Thing mockThing(boolean withConfiguration) {
final Thing thing = mock(Thing.class);
when(thing.getUID()).thenReturn(new ThingUID(GoveeBindingConstants.THING_TYPE_LIGHT, "govee-test-thing"));
when(thing.getConfiguration()).thenReturn(withConfiguration ? CONFIG : BAD_CONFIG);

final List<Channel> channelList = Arrays.asList(
mockChannel(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR), //
mockChannel(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR_TEMPERATURE), //
mockChannel(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR_TEMPERATURE_ABS));

when(thing.getChannels()).thenReturn(channelList);
return thing;
}

private static Channel mockChannel(final ThingUID thingId, final String channelId) {
final Channel channel = Mockito.mock(Channel.class);
when(channel.getUID()).thenReturn(new ChannelUID(thingId, channelId));
return channel;
}

private static GoveeHandlerMock createAndInitHandler(final ThingHandlerCallback callback, final Thing thing) {
CommunicationManager communicationManager = mock(CommunicationManager.class);
final GoveeHandlerMock handler = spy(new GoveeHandlerMock(thing, communicationManager));

handler.setCallback(callback);
handler.initialize();

return handler;
}

private static State getState(final int input, Unit<?> unit) {
return new QuantityType<>(input, unit);
}

@Test
public void testInvalidConfiguration() {
final Thing thing = mockThing(false);
final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
final GoveeHandlerMock handler = createAndInitHandler(callback, thing);

try {
verify(callback).statusUpdated(eq(thing), argThat(arg -> arg.getStatus().equals(ThingStatus.OFFLINE)
&& arg.getStatusDetail().equals(ThingStatusDetail.CONFIGURATION_ERROR)));
} finally {
handler.dispose();
}
}

@Test
public void testInvalidResponseMessage() {
final Thing thing = mockThing(true);
final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
final GoveeHandlerMock handler = createAndInitHandler(callback, thing);

// inject StatusResponseMessage
StatusResponse statusMessage = GSON.fromJson(invalidValueJsonString, StatusResponse.class);
handler.updateDeviceState(statusMessage);

try {
verify(callback).statusUpdated(eq(thing), argThat(arg -> arg.getStatus().equals(ThingStatus.UNKNOWN)));

verify(callback).stateUpdated(new ChannelUID(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR),
new HSBType(new DecimalType(114), new PercentType(100), new PercentType(0)));

verify(callback).stateUpdated(
new ChannelUID(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR_TEMPERATURE),
new PercentType(100));

verify(callback).stateUpdated(
new ChannelUID(thing.getUID(), GoveeBindingConstants.CHANNEL_COLOR_TEMPERATURE_ABS),
getState(2000, Units.KELVIN));
} finally {
handler.dispose();
}
}
}

0 comments on commit 96abfd2

Please sign in to comment.