[mqtt.homeassistant] Fix binding crash when home assistant discovery topics update with content (#13518)

* [mqtt.homeassistant] Fix for discovery topics that update with content

Fixes #13517
Possibly resolves #9711 and #12295 as well.

* [mqtt.homeassistant] Sort channels before changing thing
* [mqtt.homeassistant] logging + removed unnecessary synchronization
* Resolve bunch of warnings in homeassistant bundle
* [mqtt.homeassistant] Handling null warnings and unnecessary null checks
* [mqtt.homeassistant] Removing unnecessary null checks

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
Co-Authored-by: @antroids github handle
This commit is contained in:
Sami Salonen 2022-11-12 13:46:43 +02:00 committed by GitHub
parent 159054a99c
commit a1270c7822
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 205 additions and 62 deletions

View File

@ -241,9 +241,10 @@ public class Climate extends AbstractComponent<Climate.ChannelConfiguration> {
updateListener, channelConfiguration.fanModeCommandTemplate, channelConfiguration.fanModeCommandTopic, updateListener, channelConfiguration.fanModeCommandTemplate, channelConfiguration.fanModeCommandTopic,
channelConfiguration.fanModeStateTemplate, channelConfiguration.fanModeStateTopic, commandFilter); channelConfiguration.fanModeStateTemplate, channelConfiguration.fanModeStateTopic, commandFilter);
if (channelConfiguration.holdModes != null && !channelConfiguration.holdModes.isEmpty()) { List<String> holdModes = channelConfiguration.holdModes;
buildOptionalChannel(HOLD_CH_ID, new TextValue(channelConfiguration.holdModes.toArray(new String[0])), if (holdModes != null && !holdModes.isEmpty()) {
updateListener, channelConfiguration.holdCommandTemplate, channelConfiguration.holdCommandTopic, buildOptionalChannel(HOLD_CH_ID, new TextValue(holdModes.toArray(new String[0])), updateListener,
channelConfiguration.holdCommandTemplate, channelConfiguration.holdCommandTopic,
channelConfiguration.holdStateTemplate, channelConfiguration.holdStateTopic, commandFilter); channelConfiguration.holdStateTemplate, channelConfiguration.holdStateTopic, commandFilter);
} }

View File

@ -197,9 +197,10 @@ public class Vacuum extends AbstractComponent<Vacuum.ChannelConfiguration> {
final var allowedSupportedFeatures = channelConfiguration.schema == Schema.LEGACY ? LEGACY_SUPPORTED_FEATURES final var allowedSupportedFeatures = channelConfiguration.schema == Schema.LEGACY ? LEGACY_SUPPORTED_FEATURES
: STATE_SUPPORTED_FEATURES; : STATE_SUPPORTED_FEATURES;
final var configSupportedFeatures = channelConfiguration.supportedFeatures == null final var supportedFeatures = channelConfiguration.supportedFeatures;
final var configSupportedFeatures = supportedFeatures == null
? channelConfiguration.schema == Schema.LEGACY ? LEGACY_DEFAULT_FEATURES : STATE_DEFAULT_FEATURES ? channelConfiguration.schema == Schema.LEGACY ? LEGACY_DEFAULT_FEATURES : STATE_DEFAULT_FEATURES
: channelConfiguration.supportedFeatures; : supportedFeatures;
List<String> deviceSupportedFeatures = Collections.emptyList(); List<String> deviceSupportedFeatures = Collections.emptyList();
if (!configSupportedFeatures.isEmpty()) { if (!configSupportedFeatures.isEmpty()) {

View File

@ -15,6 +15,7 @@ package org.openhab.binding.mqtt.homeassistant.internal.config;
import java.io.IOException; import java.io.IOException;
import java.lang.reflect.Field; import java.lang.reflect.Field;
import java.util.Arrays; import java.util.Arrays;
import java.util.Objects;
import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jdt.annotation.Nullable;
@ -119,6 +120,7 @@ public class ChannelConfigurationTypeAdapterFactory implements TypeAdapterFactor
String parentTopic = config.getParentTopic(); String parentTopic = config.getParentTopic();
while (type != Object.class) { while (type != Object.class) {
Objects.requireNonNull(type, "Bug: type is null"); // Should not happen? Making compiler happy
Arrays.stream(type.getDeclaredFields()).filter(this::isMqttTopicField) Arrays.stream(type.getDeclaredFields()).filter(this::isMqttTopicField)
.forEach(field -> replacePlaceholderByParentTopic(config, field, parentTopic)); .forEach(field -> replacePlaceholderByParentTopic(config, field, parentTopic));
type = type.getSuperclass(); type = type.getSuperclass();

View File

@ -33,8 +33,9 @@ import com.google.gson.JsonParseException;
*/ */
@NonNullByDefault @NonNullByDefault
public class ConnectionDeserializer implements JsonDeserializer<Connection> { public class ConnectionDeserializer implements JsonDeserializer<Connection> {
public @Nullable Connection deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) @Override
throws JsonParseException { public @Nullable Connection deserialize(@Nullable JsonElement json, Type typeOfT,
JsonDeserializationContext context) throws JsonParseException {
JsonArray list; JsonArray list;
if (json == null) { if (json == null) {
throw new JsonParseException("JSON element is null, but must be connection definition."); throw new JsonParseException("JSON element is null, but must be connection definition.");

View File

@ -66,7 +66,6 @@ import com.google.gson.GsonBuilder;
@Component(service = DiscoveryService.class, configurationPid = "discovery.mqttha") @Component(service = DiscoveryService.class, configurationPid = "discovery.mqttha")
@NonNullByDefault @NonNullByDefault
public class HomeAssistantDiscovery extends AbstractMQTTDiscovery { public class HomeAssistantDiscovery extends AbstractMQTTDiscovery {
@SuppressWarnings("unused")
private final Logger logger = LoggerFactory.getLogger(HomeAssistantDiscovery.class); private final Logger logger = LoggerFactory.getLogger(HomeAssistantDiscovery.class);
protected final Map<String, Set<HaID>> componentsPerThingID = new TreeMap<>(); protected final Map<String, Set<HaID>> componentsPerThingID = new TreeMap<>();
protected final Map<String, ThingUID> thingIDPerTopic = new TreeMap<>(); protected final Map<String, ThingUID> thingIDPerTopic = new TreeMap<>();
@ -260,14 +259,16 @@ public class HomeAssistantDiscovery extends AbstractMQTTDiscovery {
} }
if (thingIDPerTopic.containsKey(topic)) { if (thingIDPerTopic.containsKey(topic)) {
ThingUID thingUID = thingIDPerTopic.remove(topic); ThingUID thingUID = thingIDPerTopic.remove(topic);
final String thingID = thingUID.getId(); if (thingUID != null) {
final String thingID = thingUID.getId();
HaID haID = new HaID(topic); HaID haID = new HaID(topic);
Set<HaID> components = componentsPerThingID.getOrDefault(thingID, Collections.emptySet()); Set<HaID> components = componentsPerThingID.getOrDefault(thingID, Collections.emptySet());
components.remove(haID); components.remove(haID);
if (components.isEmpty()) { if (components.isEmpty()) {
thingRemoved(thingUID); thingRemoved(thingUID);
}
} }
} }
} }

View File

@ -21,6 +21,8 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
*/ */
@NonNullByDefault @NonNullByDefault
public class ConfigurationException extends RuntimeException { public class ConfigurationException extends RuntimeException {
private static final long serialVersionUID = -4828651603869498942L;
public ConfigurationException(String message) { public ConfigurationException(String message) {
super(message); super(message);
} }

View File

@ -21,6 +21,8 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
*/ */
@NonNullByDefault @NonNullByDefault
public class UnsupportedComponentException extends ConfigurationException { public class UnsupportedComponentException extends ConfigurationException {
private static final long serialVersionUID = 5134690914728956232L;
public UnsupportedComponentException(String message) { public UnsupportedComponentException(String message) {
super(message); super(message);
} }

View File

@ -12,7 +12,8 @@
*/ */
package org.openhab.binding.mqtt.homeassistant.internal.handler; package org.openhab.binding.mqtt.homeassistant.internal.handler;
import java.util.Collection; import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
@ -49,6 +50,7 @@ import org.openhab.core.thing.ThingStatus;
import org.openhab.core.thing.ThingStatusDetail; import org.openhab.core.thing.ThingStatusDetail;
import org.openhab.core.thing.ThingTypeUID; import org.openhab.core.thing.ThingTypeUID;
import org.openhab.core.thing.ThingUID; import org.openhab.core.thing.ThingUID;
import org.openhab.core.thing.binding.builder.ThingBuilder;
import org.openhab.core.thing.type.ChannelDefinition; import org.openhab.core.thing.type.ChannelDefinition;
import org.openhab.core.thing.type.ChannelGroupDefinition; import org.openhab.core.thing.type.ChannelGroupDefinition;
import org.openhab.core.thing.type.ChannelGroupType; import org.openhab.core.thing.type.ChannelGroupType;
@ -80,6 +82,8 @@ import com.google.gson.GsonBuilder;
public class HomeAssistantThingHandler extends AbstractMQTTThingHandler public class HomeAssistantThingHandler extends AbstractMQTTThingHandler
implements ComponentDiscovered, Consumer<List<AbstractComponent<?>>> { implements ComponentDiscovered, Consumer<List<AbstractComponent<?>>> {
public static final String AVAILABILITY_CHANNEL = "availability"; public static final String AVAILABILITY_CHANNEL = "availability";
private static final Comparator<Channel> CHANNEL_COMPARATOR_BY_UID = Comparator
.comparing(channel -> channel.getUID().toString());;
private final Logger logger = LoggerFactory.getLogger(HomeAssistantThingHandler.class); private final Logger logger = LoggerFactory.getLogger(HomeAssistantThingHandler.class);
@ -120,13 +124,12 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler
this.transformationServiceProvider); this.transformationServiceProvider);
} }
@SuppressWarnings({ "null", "unused" })
@Override @Override
public void initialize() { public void initialize() {
started = false; started = false;
config = getConfigAs(HandlerConfiguration.class); config = getConfigAs(HandlerConfiguration.class);
if (config.topics == null || config.topics.isEmpty()) { if (config.topics.isEmpty()) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Device topics unknown"); updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Device topics unknown");
return; return;
} }
@ -222,7 +225,6 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler
super.stop(); super.stop();
} }
@SuppressWarnings({ "null", "unused" })
@Override @Override
public @Nullable ChannelState getChannelState(ChannelUID channelUID) { public @Nullable ChannelState getChannelState(ChannelUID channelUID) {
String groupID = channelUID.getGroupId(); String groupID = channelUID.getGroupId();
@ -255,7 +257,6 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler
* Callback of {@link DelayedBatchProcessing}. * Callback of {@link DelayedBatchProcessing}.
* Add all newly discovered components to the Thing and start the components. * Add all newly discovered components to the Thing and start the components.
*/ */
@SuppressWarnings("null")
@Override @Override
public void accept(List<AbstractComponent<?>> discoveredComponentsList) { public void accept(List<AbstractComponent<?>> discoveredComponentsList) {
MqttBrokerConnection connection = this.connection; MqttBrokerConnection connection = this.connection;
@ -288,13 +289,44 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler
return null; return null;
}); });
Collection<Channel> channels = discovered.getChannelMap().values().stream() List<Channel> discoveredChannels = discovered.getChannelMap().values().stream()
.map(ComponentChannel::getChannel).collect(Collectors.toList()); .map(ComponentChannel::getChannel).collect(Collectors.toList());
ThingHelper.addChannelsToThing(thing, channels); if (known != null) {
} // We had previously known component with different config hash
} // We remove all conflicting old channels, they will be re-added below based on the new discovery
logger.debug(
"Received component {} with slightly different config. Making sure we re-create conflicting channels...",
discovered.getGroupUID());
removeJustRediscoveredChannels(discoveredChannels);
}
updateThingType(); // Add newly discovered channels. We sort the channels
// for (mostly) consistent jsondb serialization
discoveredChannels.sort(CHANNEL_COMPARATOR_BY_UID);
ThingHelper.addChannelsToThing(thing, discoveredChannels);
}
updateThingType();
}
}
private void removeJustRediscoveredChannels(List<Channel> discoveredChannels) {
ArrayList<Channel> mutableChannels = new ArrayList<>(getThing().getChannels());
Set<ChannelUID> newChannelUIDs = discoveredChannels.stream().map(Channel::getUID).collect(Collectors.toSet());
// Take current channels but remove those channels that were just re-discovered
List<Channel> existingChannelsWithNewlyDiscoveredChannelsRemoved = mutableChannels.stream()
.filter(existingChannel -> !newChannelUIDs.contains(existingChannel.getUID()))
.collect(Collectors.toList());
if (existingChannelsWithNewlyDiscoveredChannelsRemoved.size() < mutableChannels.size()) {
// We sort the channels for (mostly) consistent jsondb serialization
existingChannelsWithNewlyDiscoveredChannelsRemoved.sort(CHANNEL_COMPARATOR_BY_UID);
updateThingChannels(existingChannelsWithNewlyDiscoveredChannelsRemoved);
}
}
private void updateThingChannels(List<Channel> channelList) {
ThingBuilder thingBuilder = editThing();
thingBuilder.withChannels(channelList);
updateThing(thingBuilder.build());
} }
@Override @Override

View File

@ -12,19 +12,15 @@
*/ */
package org.openhab.binding.mqtt.homeassistant.internal; package org.openhab.binding.mqtt.homeassistant.internal;
import static org.mockito.Mockito.any; import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.*;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;
import java.io.IOException; import java.io.IOException;
import java.net.URISyntaxException; import java.net.URISyntaxException;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.nio.file.Paths; import java.nio.file.Paths;
import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.UUID; import java.util.UUID;
import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletableFuture;
@ -64,7 +60,6 @@ import org.openhab.transform.jinja.internal.profiles.JinjaTransformationProfile;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings({ "ConstantConditions" })
@ExtendWith(MockitoExtension.class) @ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT) @MockitoSettings(strictness = Strictness.LENIENT)
@NonNullByDefault @NonNullByDefault
@ -87,7 +82,6 @@ public abstract class AbstractHomeAssistantTests extends JavaTest {
protected @Mock @NonNullByDefault({}) ThingTypeRegistry thingTypeRegistry; protected @Mock @NonNullByDefault({}) ThingTypeRegistry thingTypeRegistry;
protected @Mock @NonNullByDefault({}) TransformationServiceProvider transformationServiceProvider; protected @Mock @NonNullByDefault({}) TransformationServiceProvider transformationServiceProvider;
@SuppressWarnings("NotNullFieldNotInitialized")
protected @NonNullByDefault({}) MqttChannelTypeProvider channelTypeProvider; protected @NonNullByDefault({}) MqttChannelTypeProvider channelTypeProvider;
protected final Bridge bridgeThing = BridgeBuilder.create(BRIDGE_TYPE_UID, BRIDGE_UID).build(); protected final Bridge bridgeThing = BridgeBuilder.create(BRIDGE_TYPE_UID, BRIDGE_UID).build();
@ -126,7 +120,9 @@ public abstract class AbstractHomeAssistantTests extends JavaTest {
final var subscriber = (MqttMessageSubscriber) invocation.getArgument(1); final var subscriber = (MqttMessageSubscriber) invocation.getArgument(1);
subscriptions.putIfAbsent(topic, ConcurrentHashMap.newKeySet()); subscriptions.putIfAbsent(topic, ConcurrentHashMap.newKeySet());
subscriptions.get(topic).add(subscriber); Set<MqttMessageSubscriber> subscribers = subscriptions.get(topic);
Objects.requireNonNull(subscribers); // Invariant, thanks to putIfAbsent above. To make compiler happy
subscribers.add(subscriber);
return CompletableFuture.completedFuture(true); return CompletableFuture.completedFuture(true);
}).when(bridgeConnection).subscribe(any(), any()); }).when(bridgeConnection).subscribe(any(), any());
@ -154,6 +150,7 @@ public abstract class AbstractHomeAssistantTests extends JavaTest {
* @param relativePath path from src/test/java/org/openhab/binding/mqtt/homeassistant/internal * @param relativePath path from src/test/java/org/openhab/binding/mqtt/homeassistant/internal
* @return path * @return path
*/ */
@SuppressWarnings("null")
protected Path getResourcePath(String relativePath) { protected Path getResourcePath(String relativePath) {
try { try {
return Paths.get(AbstractHomeAssistantTests.class.getResource(relativePath).toURI()); return Paths.get(AbstractHomeAssistantTests.class.getResource(relativePath).toURI());

View File

@ -12,19 +12,11 @@
*/ */
package org.openhab.binding.mqtt.homeassistant.internal.component; package org.openhab.binding.mqtt.homeassistant.internal.component;
import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.*;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.util.List; import java.util.List;
@ -38,6 +30,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jdt.annotation.Nullable;
import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.mockito.ArgumentMatchers;
import org.mockito.Mock; import org.mockito.Mock;
import org.openhab.binding.mqtt.generic.MqttChannelTypeProvider; import org.openhab.binding.mqtt.generic.MqttChannelTypeProvider;
import org.openhab.binding.mqtt.generic.TransformationServiceProvider; import org.openhab.binding.mqtt.generic.TransformationServiceProvider;
@ -59,7 +52,6 @@ import org.openhab.core.types.State;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings({ "ConstantConditions" })
@NonNullByDefault @NonNullByDefault
public abstract class AbstractComponentTests extends AbstractHomeAssistantTests { public abstract class AbstractComponentTests extends AbstractHomeAssistantTests {
private static final int SUBSCRIBE_TIMEOUT = 10000; private static final int SUBSCRIBE_TIMEOUT = 10000;
@ -178,6 +170,7 @@ public abstract class AbstractComponentTests extends AbstractHomeAssistantTests
* @param channelId channel * @param channelId channel
* @param state expected state * @param state expected state
*/ */
@SuppressWarnings("null")
protected static void assertState(AbstractComponent<@NonNull ? extends AbstractChannelConfiguration> component, protected static void assertState(AbstractComponent<@NonNull ? extends AbstractChannelConfiguration> component,
String channelId, State state) { String channelId, State state) {
assertThat(component.getChannel(channelId).getState().getCache().getChannelState(), is(state)); assertThat(component.getChannel(channelId).getState().getCache().getChannelState(), is(state));
@ -190,8 +183,8 @@ public abstract class AbstractComponentTests extends AbstractHomeAssistantTests
* @param payload payload * @param payload payload
*/ */
protected void assertPublished(String mqttTopic, String payload) { protected void assertPublished(String mqttTopic, String payload) {
verify(bridgeConnection).publish(eq(mqttTopic), eq(payload.getBytes(StandardCharsets.UTF_8)), anyInt(), verify(bridgeConnection).publish(eq(mqttTopic), ArgumentMatchers.eq(payload.getBytes(StandardCharsets.UTF_8)),
anyBoolean()); anyInt(), anyBoolean());
} }
/** /**
@ -202,8 +195,8 @@ public abstract class AbstractComponentTests extends AbstractHomeAssistantTests
* @param t payload must be published N times on given topic * @param t payload must be published N times on given topic
*/ */
protected void assertPublished(String mqttTopic, String payload, int t) { protected void assertPublished(String mqttTopic, String payload, int t) {
verify(bridgeConnection, times(t)).publish(eq(mqttTopic), eq(payload.getBytes(StandardCharsets.UTF_8)), verify(bridgeConnection, times(t)).publish(eq(mqttTopic),
anyInt(), anyBoolean()); ArgumentMatchers.eq(payload.getBytes(StandardCharsets.UTF_8)), anyInt(), anyBoolean());
} }
/** /**
@ -213,8 +206,8 @@ public abstract class AbstractComponentTests extends AbstractHomeAssistantTests
* @param payload payload * @param payload payload
*/ */
protected void assertNotPublished(String mqttTopic, String payload) { protected void assertNotPublished(String mqttTopic, String payload) {
verify(bridgeConnection, never()).publish(eq(mqttTopic), eq(payload.getBytes(StandardCharsets.UTF_8)), anyInt(), verify(bridgeConnection, never()).publish(eq(mqttTopic),
anyBoolean()); ArgumentMatchers.eq(payload.getBytes(StandardCharsets.UTF_8)), anyInt(), anyBoolean());
} }
/** /**

View File

@ -27,11 +27,11 @@ import org.openhab.core.library.types.StringType;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings("ConstantConditions")
@NonNullByDefault @NonNullByDefault
public class AlarmControlPanelTests extends AbstractComponentTests { public class AlarmControlPanelTests extends AbstractComponentTests {
public static final String CONFIG_TOPIC = "alarm_control_panel/0x0000000000000000_alarm_control_panel_zigbee2mqtt"; public static final String CONFIG_TOPIC = "alarm_control_panel/0x0000000000000000_alarm_control_panel_zigbee2mqtt";
@SuppressWarnings("null")
@Test @Test
public void testAlarmControlPanel() { public void testAlarmControlPanel() {
// @formatter:off // @formatter:off

View File

@ -34,11 +34,11 @@ import org.openhab.core.library.unit.SIUnits;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings("ConstantConditions")
@NonNullByDefault @NonNullByDefault
public class ClimateTests extends AbstractComponentTests { public class ClimateTests extends AbstractComponentTests {
public static final String CONFIG_TOPIC = "climate/0x847127fffe11dd6a_climate_zigbee2mqtt"; public static final String CONFIG_TOPIC = "climate/0x847127fffe11dd6a_climate_zigbee2mqtt";
@SuppressWarnings("null")
@Test @Test
public void testTS0601Climate() { public void testTS0601Climate() {
var component = discoverComponent(configTopicToMqtt(CONFIG_TOPIC), "{" var component = discoverComponent(configTopicToMqtt(CONFIG_TOPIC), "{"
@ -102,6 +102,7 @@ public class ClimateTests extends AbstractComponentTests {
assertPublished("zigbee2mqtt/th1/set/current_heating_setpoint", "25"); assertPublished("zigbee2mqtt/th1/set/current_heating_setpoint", "25");
} }
@SuppressWarnings("null")
@Test @Test
public void testTS0601ClimateNotSendIfOff() { public void testTS0601ClimateNotSendIfOff() {
var component = discoverComponent(configTopicToMqtt(CONFIG_TOPIC), "{" var component = discoverComponent(configTopicToMqtt(CONFIG_TOPIC), "{"
@ -185,6 +186,7 @@ public class ClimateTests extends AbstractComponentTests {
assertPublished("zigbee2mqtt/th1/set/current_heating_setpoint", "25"); assertPublished("zigbee2mqtt/th1/set/current_heating_setpoint", "25");
} }
@SuppressWarnings("null")
@Test @Test
public void testClimate() { public void testClimate() {
var component = discoverComponent(configTopicToMqtt(CONFIG_TOPIC), var component = discoverComponent(configTopicToMqtt(CONFIG_TOPIC),

View File

@ -28,11 +28,11 @@ import org.openhab.core.library.types.StopMoveType;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings("ConstantConditions")
@NonNullByDefault @NonNullByDefault
public class CoverTests extends AbstractComponentTests { public class CoverTests extends AbstractComponentTests {
public static final String CONFIG_TOPIC = "cover/0x0000000000000000_cover_zigbee2mqtt"; public static final String CONFIG_TOPIC = "cover/0x0000000000000000_cover_zigbee2mqtt";
@SuppressWarnings("null")
@Test @Test
public void test() throws InterruptedException { public void test() throws InterruptedException {
// @formatter:off // @formatter:off

View File

@ -27,11 +27,11 @@ import org.openhab.core.library.types.OnOffType;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings("ALL")
@NonNullByDefault @NonNullByDefault
public class FanTests extends AbstractComponentTests { public class FanTests extends AbstractComponentTests {
public static final String CONFIG_TOPIC = "fan/0x0000000000000000_fan_zigbee2mqtt"; public static final String CONFIG_TOPIC = "fan/0x0000000000000000_fan_zigbee2mqtt";
@SuppressWarnings("null")
@Test @Test
public void test() throws InterruptedException { public void test() throws InterruptedException {
// @formatter:off // @formatter:off

View File

@ -148,6 +148,7 @@ public class HAConfigurationTests {
} }
} }
@SuppressWarnings("null")
@Test @Test
public void testTS0601ClimateConfig() { public void testTS0601ClimateConfig() {
String json = readTestJson("configTS0601ClimateThermostat.json"); String json = readTestJson("configTS0601ClimateThermostat.json");

View File

@ -27,11 +27,11 @@ import org.openhab.core.library.types.OnOffType;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings("ALL")
@NonNullByDefault @NonNullByDefault
public class LockTests extends AbstractComponentTests { public class LockTests extends AbstractComponentTests {
public static final String CONFIG_TOPIC = "lock/0x0000000000000000_lock_zigbee2mqtt"; public static final String CONFIG_TOPIC = "lock/0x0000000000000000_lock_zigbee2mqtt";
@SuppressWarnings("null")
@Test @Test
public void test() throws InterruptedException { public void test() throws InterruptedException {
// @formatter:off // @formatter:off

View File

@ -31,11 +31,11 @@ import org.openhab.core.types.UnDefType;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings("ConstantConditions")
@NonNullByDefault @NonNullByDefault
public class SensorTests extends AbstractComponentTests { public class SensorTests extends AbstractComponentTests {
public static final String CONFIG_TOPIC = "sensor/0x0000000000000000_sensor_zigbee2mqtt"; public static final String CONFIG_TOPIC = "sensor/0x0000000000000000_sensor_zigbee2mqtt";
@SuppressWarnings("null")
@Test @Test
public void test() throws InterruptedException { public void test() throws InterruptedException {
// @formatter:off // @formatter:off

View File

@ -27,11 +27,11 @@ import org.openhab.core.library.types.OnOffType;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings("ConstantConditions")
@NonNullByDefault @NonNullByDefault
public class SwitchTests extends AbstractComponentTests { public class SwitchTests extends AbstractComponentTests {
public static final String CONFIG_TOPIC = "switch/0x847127fffe11dd6a_auto_lock_zigbee2mqtt"; public static final String CONFIG_TOPIC = "switch/0x847127fffe11dd6a_auto_lock_zigbee2mqtt";
@SuppressWarnings("null")
@Test @Test
public void testSwitchWithStateAndCommand() { public void testSwitchWithStateAndCommand() {
var component = discoverComponent(configTopicToMqtt(CONFIG_TOPIC), var component = discoverComponent(configTopicToMqtt(CONFIG_TOPIC),
@ -92,6 +92,7 @@ public class SwitchTests extends AbstractComponentTests {
assertState(component, Switch.SWITCH_CHANNEL_ID, OnOffType.ON); assertState(component, Switch.SWITCH_CHANNEL_ID, OnOffType.ON);
} }
@SuppressWarnings("null")
@Test @Test
public void testSwitchWithCommand() { public void testSwitchWithCommand() {
var component = discoverComponent(configTopicToMqtt(CONFIG_TOPIC), var component = discoverComponent(configTopicToMqtt(CONFIG_TOPIC),

View File

@ -32,11 +32,11 @@ import org.openhab.core.types.UnDefType;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings("ConstantConditions")
@NonNullByDefault @NonNullByDefault
public class VacuumTests extends AbstractComponentTests { public class VacuumTests extends AbstractComponentTests {
public static final String CONFIG_TOPIC = "vacuum/rockrobo_vacuum"; public static final String CONFIG_TOPIC = "vacuum/rockrobo_vacuum";
@SuppressWarnings("null")
@Test @Test
public void testRoborockValetudo() { public void testRoborockValetudo() {
// @formatter:off // @formatter:off
@ -157,6 +157,7 @@ public class VacuumTests extends AbstractComponentTests {
assertState(component, Vacuum.JSON_ATTRIBUTES_CH_ID, new StringType(jsonValue)); assertState(component, Vacuum.JSON_ATTRIBUTES_CH_ID, new StringType(jsonValue));
} }
@SuppressWarnings("null")
@Test @Test
public void testLegacySchema() { public void testLegacySchema() {
// @formatter:off // @formatter:off

View File

@ -44,7 +44,7 @@ import org.openhab.core.thing.ThingUID;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings({ "ConstantConditions", "unchecked" }) @SuppressWarnings({ "unchecked" })
@ExtendWith(MockitoExtension.class) @ExtendWith(MockitoExtension.class)
@NonNullByDefault @NonNullByDefault
public class HomeAssistantDiscoveryTests extends AbstractHomeAssistantTests { public class HomeAssistantDiscoveryTests extends AbstractHomeAssistantTests {

View File

@ -19,6 +19,7 @@ import static org.mockito.Mockito.*;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.NonNullByDefault;
@ -32,7 +33,9 @@ import org.openhab.binding.mqtt.homeassistant.internal.AbstractHomeAssistantTest
import org.openhab.binding.mqtt.homeassistant.internal.HaID; import org.openhab.binding.mqtt.homeassistant.internal.HaID;
import org.openhab.binding.mqtt.homeassistant.internal.HandlerConfiguration; import org.openhab.binding.mqtt.homeassistant.internal.HandlerConfiguration;
import org.openhab.binding.mqtt.homeassistant.internal.component.Climate; import org.openhab.binding.mqtt.homeassistant.internal.component.Climate;
import org.openhab.binding.mqtt.homeassistant.internal.component.Sensor;
import org.openhab.binding.mqtt.homeassistant.internal.component.Switch; import org.openhab.binding.mqtt.homeassistant.internal.component.Switch;
import org.openhab.core.thing.Channel;
import org.openhab.core.thing.binding.ThingHandlerCallback; import org.openhab.core.thing.binding.ThingHandlerCallback;
/** /**
@ -40,7 +43,6 @@ import org.openhab.core.thing.binding.ThingHandlerCallback;
* *
* @author Anton Kharuzhy - Initial contribution * @author Anton Kharuzhy - Initial contribution
*/ */
@SuppressWarnings({ "ConstantConditions" })
@ExtendWith(MockitoExtension.class) @ExtendWith(MockitoExtension.class)
@NonNullByDefault @NonNullByDefault
public class HomeAssistantThingHandlerTests extends AbstractHomeAssistantTests { public class HomeAssistantThingHandlerTests extends AbstractHomeAssistantTests {
@ -60,6 +62,7 @@ public class HomeAssistantThingHandlerTests extends AbstractHomeAssistantTests {
private @Mock @NonNullByDefault({}) ThingHandlerCallback callbackMock; private @Mock @NonNullByDefault({}) ThingHandlerCallback callbackMock;
private @NonNullByDefault({}) HomeAssistantThingHandler thingHandler; private @NonNullByDefault({}) HomeAssistantThingHandler thingHandler;
private @NonNullByDefault({}) HomeAssistantThingHandler nonSpyThingHandler;
@BeforeEach @BeforeEach
public void setup() { public void setup() {
@ -74,6 +77,7 @@ public class HomeAssistantThingHandlerTests extends AbstractHomeAssistantTests {
SUBSCRIBE_TIMEOUT, ATTRIBUTE_RECEIVE_TIMEOUT); SUBSCRIBE_TIMEOUT, ATTRIBUTE_RECEIVE_TIMEOUT);
thingHandler.setConnection(bridgeConnection); thingHandler.setConnection(bridgeConnection);
thingHandler.setCallback(callbackMock); thingHandler.setCallback(callbackMock);
nonSpyThingHandler = thingHandler;
thingHandler = spy(thingHandler); thingHandler = spy(thingHandler);
} }
@ -117,6 +121,108 @@ public class HomeAssistantThingHandlerTests extends AbstractHomeAssistantTests {
verify(channelTypeProvider, times(2)).setChannelGroupType(any(), any()); verify(channelTypeProvider, times(2)).setChannelGroupType(any(), any());
} }
/**
* Test where the same component is published twice to MQTT. The binding should handle this.
*
* @throws InterruptedException
*/
@Test
public void testDuplicateComponentPublish() throws InterruptedException {
thingHandler.initialize();
verify(callbackMock).statusUpdated(eq(haThing), any());
// Expect a call to the bridge status changed, the start, the propertiesChanged method
verify(thingHandler).bridgeStatusChanged(any());
verify(thingHandler, timeout(SUBSCRIBE_TIMEOUT)).start(any());
// Expect subscription on each topic from config
MQTT_TOPICS.forEach(t -> {
verify(bridgeConnection, timeout(SUBSCRIBE_TIMEOUT)).subscribe(eq(t), any());
});
verify(thingHandler, never()).componentDiscovered(any(), any());
assertThat(haThing.getChannels().size(), CoreMatchers.is(0));
//
//
// Publish sensor components with identical payload except for
// change in "name" field. The binding should respect the latest discovery result.
//
// This simulates how multiple OpenMQTTGateway devices would publish
// the same discovery topics for a particular Bluetooth sensor, and thus "competing" with similar but slightly
// different discovery topics.
//
// In fact, only difference is actually "via_device" additional metadata field telling which OpenMQTTGateway
// published the discovery topic.
//
//
//
// 1. publish corridor temperature sensor
//
var configTopicTempCorridor = "homeassistant/sensor/tempCorridor/config";
thingHandler.discoverComponents.processMessage(configTopicTempCorridor, new String("{"//
+ "\"temperature_state_topic\": \"+/+/BTtoMQTT/mysensor\","//
+ "\"temperature_state_template\": \"{{ value_json.temperature }}\", "//
+ "\"name\": \"CorridorTemp\", "//
+ "\"unit_of_measurement\": \"°C\" "//
+ "}").getBytes(StandardCharsets.UTF_8));
verify(thingHandler, times(1)).componentDiscovered(eq(new HaID(configTopicTempCorridor)), any(Sensor.class));
thingHandler.delayedProcessing.forceProcessNow();
waitForAssert(() -> {
assertThat("1 channel created", thingHandler.getThing().getChannels().size() == 1);
});
//
// 2. publish outside temperature sensor
//
var configTopicTempOutside = "homeassistant/sensor/tempOutside/config";
thingHandler.discoverComponents.processMessage(configTopicTempOutside, new String("{"//
+ "\"temperature_state_topic\": \"+/+/BTtoMQTT/mysensor\","//
+ "\"temperature_state_template\": \"{{ value_json.temperature }}\", " //
+ "\"name\": \"OutsideTemp\", "//
+ "\"source\": \"gateway2\" "//
+ "}").getBytes(StandardCharsets.UTF_8));
thingHandler.delayedProcessing.forceProcessNow();
verify(thingHandler, times(1)).componentDiscovered(eq(new HaID(configTopicTempOutside)), any(Sensor.class));
waitForAssert(() -> {
assertThat("2 channel created", thingHandler.getThing().getChannels().size() == 2);
});
//
// 3. publish corridor temperature sensor, this time with different name (openHAB channel label)
//
thingHandler.discoverComponents.processMessage(configTopicTempCorridor, new String("{"//
+ "\"temperature_state_topic\": \"+/+/BTtoMQTT/mysensor\","//
+ "\"temperature_state_template\": \"{{ value_json.temperature }}\", "//
+ "\"name\": \"CorridorTemp NEW\", "//
+ "\"unit_of_measurement\": \"°C\" "//
+ "}").getBytes(StandardCharsets.UTF_8));
thingHandler.delayedProcessing.forceProcessNow();
waitForAssert(() -> {
assertThat("2 channel created", thingHandler.getThing().getChannels().size() == 2);
});
//
// verify that both channels are there and the label corresponds to newer discovery topic payload
//
Channel corridorTempChannel = nonSpyThingHandler.getThing().getChannel("tempCorridor_5Fsensor#sensor");
assertThat("Corridor temperature channel is created", corridorTempChannel, CoreMatchers.notNullValue());
Objects.requireNonNull(corridorTempChannel); // for compiler
assertThat("Corridor temperature channel is having the updated label from 2nd discovery topic publish",
corridorTempChannel.getLabel(), CoreMatchers.is("CorridorTemp NEW"));
Channel outsideTempChannel = nonSpyThingHandler.getThing().getChannel("tempOutside_5Fsensor#sensor");
assertThat("Outside temperature channel is created", outsideTempChannel, CoreMatchers.notNullValue());
verify(thingHandler, times(2)).componentDiscovered(eq(new HaID(configTopicTempCorridor)), any(Sensor.class));
waitForAssert(() -> {
assertThat("2 channel created", thingHandler.getThing().getChannels().size() == 2);
});
}
@Test @Test
public void testDispose() { public void testDispose() {
thingHandler.initialize(); thingHandler.initialize();