Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[govee] Fix invalid status response handling #17048

Merged
merged 9 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.debug("Sending {} to {}", message, hostname);
lsiepel marked this conversation as resolved.
Show resolved Hide resolved
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");
lsiepel marked this conversation as resolved.
Show resolved Hide resolved
logger.trace("Response >> {}", response);
lsiepel marked this conversation as resolved.
Show resolved Hide resolved
}
} 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,6 +81,7 @@ public class GoveeHandler extends BaseThingHandler {
private static final Gson GSON = new Gson();

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

@Nullable
private ScheduledFuture<?> triggerStatusJob; // send device status update job
Expand Down Expand Up @@ -110,9 +112,11 @@ public class GoveeHandler extends BaseThingHandler {
}
};

public GoveeHandler(Thing thing, CommunicationManager communicationManager) {
public GoveeHandler(Thing thing, CommunicationManager communicationManager,
@Nullable ScheduledExecutorService executorService) {
super(thing);
this.communicationManager = communicationManager;
this.executorService = executorService == null ? this.scheduler : executorService;
lsiepel marked this conversation as resolved.
Show resolved Hide resolved
}

public String getHostname() {
Expand Down Expand Up @@ -318,8 +322,13 @@ public void updateDeviceState(@Nullable StatusResponse message) {
if (newColorTempInKelvin != lastColorTempInKelvin) {
logger.trace("Color-Temperature Status: old: {} K {}% vs new: {} K", lastColorTempInKelvin,
newColorTempInPercent, newColorTempInKelvin);
updateState(CHANNEL_COLOR_TEMPERATURE_ABS, new QuantityType<>(lastColorTempInKelvin, Units.KELVIN));
updateState(CHANNEL_COLOR_TEMPERATURE, new PercentType(newColorTempInPercent));
try {
updateState(CHANNEL_COLOR_TEMPERATURE_ABS, new QuantityType<>(lastColorTempInKelvin, Units.KELVIN));
updateState(CHANNEL_COLOR_TEMPERATURE, new PercentType(newColorTempInPercent));
} catch (IllegalArgumentException e) {
lsiepel marked this conversation as resolved.
Show resolved Hide resolved
logger.debug(
"Updating the channel state failed due to IllegalArgumentException, probably StatusResponse has a value out of bounds");
}
}

lastOnOff = newOnOff;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
ThingTypeUID thingTypeUID = thing.getThingTypeUID();

if (THING_TYPE_LIGHT.equals(thingTypeUID)) {
return new GoveeHandler(thing, communicationManager);
return new GoveeHandler(thing, communicationManager, null);
}

return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/**
* 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.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
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 java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;

import javax.measure.Unit;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
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 Stefan Höhn - Initial contribution
lsiepel marked this conversation as resolved.
Show resolved Hide resolved
*/
@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 GoveeHandler createAndInitHandler(final ThingHandlerCallback callback, final Thing thing) {
final ScheduledExecutorService executorStub = Mockito.mock(ScheduledExecutorService.class);
doAnswer((InvocationOnMock invocation) -> {
((Runnable) invocation.getArguments()[0]).run();
return null;
}).when(executorStub).scheduleWithFixedDelay(any(Runnable.class), anyLong(), anyLong(), any(TimeUnit.class));

final GoveeHandler handler = spy(new GoveeHandler(thing, new CommunicationManager(), executorStub));

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

private static State getState(final int input) {
lsiepel marked this conversation as resolved.
Show resolved Hide resolved
return new DecimalType(input);
}

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

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

@Test
public void testInvalidConfiguration() {
final Thing thing = mockThing(false);
final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
final GoveeHandler 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 GoveeHandler 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).statusUpdated(eq(thing), argThat(arg -> arg.getStatus().equals(ThingStatus.ONLINE)));
lsiepel marked this conversation as resolved.
Show resolved Hide resolved

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_ABS),
getState(2000, Units.KELVIN));
} finally {
handler.dispose();
}
}
}