diff --git a/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/DiscoverComponents.java b/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/DiscoverComponents.java index 810d99d988d..b88b596489a 100644 --- a/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/DiscoverComponents.java +++ b/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/DiscoverComponents.java @@ -69,6 +69,8 @@ public class DiscoverComponents implements MqttMessageSubscriber { */ public static interface ComponentDiscovered { void componentDiscovered(HaID homeAssistantTopicID, AbstractComponent component); + + void componentRemoved(HaID homeAssistantTopicID); } /** @@ -121,7 +123,9 @@ public class DiscoverComponents implements MqttMessageSubscriber { logger.warn("HomeAssistant discover error: {}", e.getMessage()); } } else { - logger.warn("Configuration of HomeAssistant thing {} is empty", haID.objectID); + if (discoveredListener != null) { + discoveredListener.componentRemoved(haID); + } } } diff --git a/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/discovery/HomeAssistantDiscovery.java b/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/discovery/HomeAssistantDiscovery.java index ca675d346f8..0d8d99b29ff 100644 --- a/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/discovery/HomeAssistantDiscovery.java +++ b/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/discovery/HomeAssistantDiscovery.java @@ -18,18 +18,14 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.TreeMap; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; -import java.util.function.Function; -import java.util.stream.Collector; import java.util.stream.Collectors; -import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -76,27 +72,14 @@ import com.google.gson.GsonBuilder; public class HomeAssistantDiscovery extends AbstractMQTTDiscovery { private final Logger logger = LoggerFactory.getLogger(HomeAssistantDiscovery.class); private HomeAssistantConfiguration configuration; - protected final Map> componentsPerThingID = new TreeMap<>(); - protected final Map thingIDPerTopic = new TreeMap<>(); - protected final Map results = new ConcurrentHashMap<>(); + protected final Map> componentsPerThingID = new HashMap<>(); + protected final Map thingIDPerTopic = new HashMap<>(); + protected final Map results = new HashMap<>(); + protected final Map allResults = new HashMap<>(); private @Nullable ScheduledFuture future; private final Gson gson; - public static final Map HA_COMP_TO_NAME = new TreeMap<>(); - { - HA_COMP_TO_NAME.put("alarm_control_panel", "Alarm Control Panel"); - HA_COMP_TO_NAME.put("binary_sensor", "Sensor"); - HA_COMP_TO_NAME.put("camera", "Camera"); - HA_COMP_TO_NAME.put("cover", "Blind"); - HA_COMP_TO_NAME.put("fan", "Fan"); - HA_COMP_TO_NAME.put("climate", "Climate Control"); - HA_COMP_TO_NAME.put("light", "Light"); - HA_COMP_TO_NAME.put("lock", "Lock"); - HA_COMP_TO_NAME.put("sensor", "Sensor"); - HA_COMP_TO_NAME.put("switch", "Switch"); - } - static final String BASE_TOPIC = "homeassistant"; static final String BIRTH_TOPIC = "homeassistant/status"; static final String ONLINE_STATUS = "online"; @@ -148,36 +131,8 @@ public class HomeAssistantDiscovery extends AbstractMQTTDiscovery { return typeProvider.getThingTypes(null).stream().map(ThingType::getUID).collect(Collectors.toSet()); } - /** - * Summarize components such as {Switch, Switch, Sensor} into string "Sensor, 2x Switch" - * - * @param componentNames stream of component names - * @return summary string of component names and their counts - */ - static String getComponentNamesSummary(Stream componentNames) { - StringBuilder summary = new StringBuilder(); - Collector countingCollector = Collectors.counting(); - Map componentCounts = componentNames - .collect(Collectors.groupingBy(Function.identity(), countingCollector)); - componentCounts.entrySet().stream().sorted(Map.Entry.comparingByKey()).forEach(entry -> { - String componentName = entry.getKey(); - long count = entry.getValue(); - if (summary.length() > 0) { - // not the first entry, so let's add the separating comma - summary.append(", "); - } - if (count > 1) { - summary.append(count); - summary.append("x "); - } - summary.append(componentName); - }); - return summary.toString(); - } - @Override - public void receivedMessage(ThingUID connectionBridge, MqttBrokerConnection connection, String topic, - byte[] payload) { + public void receivedMessage(ThingUID bridgeUID, MqttBrokerConnection connection, String topic, byte[] payload) { resetTimeout(); // For HomeAssistant we need to subscribe to a wildcard topic, because topics can either be: @@ -188,13 +143,7 @@ public class HomeAssistantDiscovery extends AbstractMQTTDiscovery { return; } - // Reset the found-component timer. - // We will collect components for the thing label description for another 2 seconds. - final ScheduledFuture future = this.future; - if (future != null) { - future.cancel(false); - } - this.future = scheduler.schedule(this::publishResults, 2, TimeUnit.SECONDS); + resetPublishTimer(); // We will of course find multiple of the same unique Thing IDs, for each different component another one. // Therefore the components are assembled into a list and given to the DiscoveryResult label for the user to @@ -206,45 +155,18 @@ public class HomeAssistantDiscovery extends AbstractMQTTDiscovery { .fromString(new String(payload, StandardCharsets.UTF_8), gson); final String thingID = config.getThingId(haID.objectID); - final ThingUID thingUID = new ThingUID(MqttBindingConstants.HOMEASSISTANT_MQTT_THING, connectionBridge, - thingID); + final ThingUID thingUID = new ThingUID(MqttBindingConstants.HOMEASSISTANT_MQTT_THING, bridgeUID, thingID); - thingIDPerTopic.put(topic, thingUID); + synchronized (results) { + thingIDPerTopic.put(topic, thingUID); - // We need to keep track of already found component topics for a specific thing - final List components; - { - Set componentsUnordered = componentsPerThingID.computeIfAbsent(thingID, - key -> ConcurrentHashMap.newKeySet()); + Map properties = new HashMap<>(); + properties = config.appendToProperties(properties); + properties.put("deviceId", thingID); + properties.put("newStyleChannels", "true"); - // Invariant. For compiler, computeIfAbsent above returns always - // non-null - Objects.requireNonNull(componentsUnordered); - componentsUnordered.add(haID); - - components = componentsUnordered.stream().collect(Collectors.toList()); - // We sort the components for consistent jsondb serialization order of 'topics' thing property - // Sorting key is HaID::toString, i.e. using the full topic string - components.sort(Comparator.comparing(HaID::toString)); + buildResult(thingID, thingUID, config.getThingName(), haID, properties, bridgeUID); } - - final String componentNames = getComponentNamesSummary( - components.stream().map(id -> id.component).map(c -> HA_COMP_TO_NAME.getOrDefault(c, c))); - - final List topics = components.stream().map(HaID::toShortTopic).collect(Collectors.toList()); - - Map properties = new HashMap<>(); - HandlerConfiguration handlerConfig = new HandlerConfiguration(haID.baseTopic, topics); - properties = handlerConfig.appendToProperties(properties); - properties = config.appendToProperties(properties); - properties.put("deviceId", thingID); - properties.put("newStyleChannels", "true"); - - // Because we need the new properties map with the updated "components" list - results.put(thingUID.getAsString(), - DiscoveryResultBuilder.create(thingUID).withProperties(properties) - .withRepresentationProperty("deviceId").withBridge(connectionBridge) - .withLabel(config.getThingName() + " (" + componentNames + ")").build()); } catch (ConfigurationException e) { logger.warn("HomeAssistant discover error: invalid configuration of thing {} component {}: {}", haID.objectID, haID.component, e.getMessage()); @@ -273,23 +195,64 @@ public class HomeAssistantDiscovery extends AbstractMQTTDiscovery { getDiscoveryService().publish(BIRTH_TOPIC, ONLINE_STATUS.getBytes(), 1, false); } + private void resetPublishTimer() { + // Reset the found-component timer. + // We will collect components for the thing label description for another 2 seconds. + final ScheduledFuture future = this.future; + if (future != null) { + future.cancel(false); + } + this.future = scheduler.schedule(this::publishResults, 2, TimeUnit.SECONDS); + } + + private void buildResult(String thingID, ThingUID thingUID, String thingName, HaID haID, + Map properties, ThingUID bridgeUID) { + // We need to keep track of already found component topics for a specific thing + final List components; + { + Set componentsUnordered = componentsPerThingID.computeIfAbsent(thingID, key -> new HashSet<>()); + + // Invariant. For compiler, computeIfAbsent above returns always + // non-null + Objects.requireNonNull(componentsUnordered); + componentsUnordered.add(haID); + + components = componentsUnordered.stream().collect(Collectors.toList()); + // We sort the components for consistent jsondb serialization order of 'topics' thing property + // Sorting key is HaID::toString, i.e. using the full topic string + components.sort(Comparator.comparing(HaID::toString)); + } + + final List topics = components.stream().map(HaID::toShortTopic).collect(Collectors.toList()); + + HandlerConfiguration handlerConfig = new HandlerConfiguration(haID.baseTopic, topics); + properties = handlerConfig.appendToProperties(properties); + + DiscoveryResult result = DiscoveryResultBuilder.create(thingUID).withProperties(properties) + .withRepresentationProperty("deviceId").withBridge(bridgeUID).withLabel(thingName).build(); + // Because we need the new properties map with the updated "components" list + results.put(thingUID.toString(), result); + allResults.put(thingUID.toString(), result); + } + protected void publishResults() { Collection localResults; - localResults = new ArrayList<>(results.values()); - results.clear(); - componentsPerThingID.clear(); + synchronized (results) { + localResults = new ArrayList<>(results.values()); + results.clear(); + } for (DiscoveryResult result : localResults) { thingDiscovered(result); } } @Override - public void topicVanished(ThingUID connectionBridge, MqttBrokerConnection connection, String topic) { + public void topicVanished(ThingUID bridgeUID, MqttBrokerConnection connection, String topic) { if (!topic.endsWith("/config")) { return; } - if (thingIDPerTopic.containsKey(topic)) { + synchronized (results) { ThingUID thingUID = thingIDPerTopic.remove(topic); if (thingUID != null) { final String thingID = thingUID.getId(); @@ -299,7 +262,20 @@ public class HomeAssistantDiscovery extends AbstractMQTTDiscovery { Set components = componentsPerThingID.getOrDefault(thingID, Collections.emptySet()); components.remove(haID); if (components.isEmpty()) { + allResults.remove(thingUID.toString()); + results.remove(thingUID.toString()); thingRemoved(thingUID); + } else { + resetPublishTimer(); + + DiscoveryResult existingThing = allResults.get(thingUID.toString()); + if (existingThing == null) { + logger.warn("Could not find discovery result for removed component {}; this is a bug", + thingUID); + return; + } + Map properties = new HashMap<>(existingThing.getProperties()); + buildResult(thingID, thingUID, existingThing.getLabel(), haID, properties, bridgeUID); } } } diff --git a/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/handler/HomeAssistantThingHandler.java b/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/handler/HomeAssistantThingHandler.java index 11ce67c84f7..739c834d7ee 100644 --- a/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/handler/HomeAssistantThingHandler.java +++ b/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/handler/HomeAssistantThingHandler.java @@ -82,7 +82,7 @@ import com.hubspot.jinjava.Jinjava; */ @NonNullByDefault public class HomeAssistantThingHandler extends AbstractMQTTThingHandler - implements ComponentDiscovered, Consumer>> { + implements ComponentDiscovered, Consumer> { public static final String AVAILABILITY_CHANNEL = "availability"; private static final Comparator> COMPONENT_COMPARATOR = Comparator .comparing((AbstractComponent component) -> component.hasGroup()) @@ -96,12 +96,13 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler protected final ChannelTypeRegistry channelTypeRegistry; protected final Jinjava jinjava; public final int attributeReceiveTimeout; - protected final DelayedBatchProcessing> delayedProcessing; + protected final DelayedBatchProcessing delayedProcessing; protected final DiscoverComponents discoverComponents; private final Gson gson; protected final Map<@Nullable String, AbstractComponent> haComponents = new HashMap<>(); protected final Map<@Nullable String, AbstractComponent> haComponentsByUniqueId = new HashMap<>(); + protected final Map> haComponentsByHaId = new HashMap<>(); protected final Map channelStates = new HashMap<>(); protected HandlerConfiguration config = new HandlerConfiguration(); @@ -267,12 +268,38 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler delayedProcessing.accept(component); } + @Override + public void componentRemoved(HaID haID) { + delayedProcessing.accept(haID); + } + /** * Callback of {@link DelayedBatchProcessing}. - * Add all newly discovered components to the Thing and start the components. + * Add all newly discovered and removed components to the Thing and start the components. */ @Override - public void accept(List> discoveredComponentsList) { + public void accept(List actions) { + List> discoveredComponents = new ArrayList<>(); + List removedComponents = new ArrayList<>(); + for (Object item : actions) { + if (item instanceof AbstractComponent component) { + discoveredComponents.add(component); + } else if (item instanceof HaID removedComponent) { + removedComponents.add(removedComponent); + } + } + if (!discoveredComponents.isEmpty()) { + addComponents(discoveredComponents); + } + if (!removedComponents.isEmpty()) { + removeComponents(removedComponents); + } + } + + /** + * Add all newly discovered components to the Thing and start the components. + */ + private void addComponents(List> discoveredComponentsList) { MqttBrokerConnection connection = this.connection; if (connection == null) { return; @@ -293,6 +320,7 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler // The component will be replaced in a moment. known.stop(); haComponentsByUniqueId.remove(discovered.getUniqueId()); + haComponentsByHaId.remove(known.getHaID()); haComponents.remove(known.getComponentId()); if (!known.getComponentId().equals(discovered.getComponentId())) { discovered.resolveConflict(); @@ -321,6 +349,29 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler } } + /** + * Remove all matching deleted components. + */ + private void removeComponents(List removedComponentsList) { + synchronized (haComponents) { + boolean componentActuallyRemoved = false; + for (HaID removed : removedComponentsList) { + AbstractComponent known = haComponentsByHaId.get(removed); + if (known != null) { + // Don't wait for the future to complete. We are also not interested in failures. + known.stop(); + haComponentsByUniqueId.remove(known.getUniqueId()); + haComponents.remove(known.getComponentId()); + haComponentsByHaId.remove(removed); + componentActuallyRemoved = true; + } + } + if (componentActuallyRemoved) { + updateThingType(getThing().getThingTypeUID()); + } + } + } + @Override protected void updateThingStatus(boolean messageReceived, Optional availabilityTopicsSeen) { if (availabilityTopicsSeen.orElse(messageReceived)) { @@ -402,7 +453,7 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler return true; } - private ThingTypeUID calculateThingTypeUID(AbstractComponent component) { + private ThingTypeUID calculateThingTypeUID(AbstractComponent component) { return new ThingTypeUID(MqttBindingConstants.BINDING_ID, MqttBindingConstants.HOMEASSISTANT_MQTT_THING.getId() + "_" + component.getChannelConfiguration().getThingId(component.getHaID().objectID)); } @@ -428,8 +479,8 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler } // should only be called when it's safe to access haComponents - private boolean addComponent(AbstractComponent component) { - AbstractComponent existing = haComponents.get(component.getComponentId()); + private boolean addComponent(AbstractComponent component) { + AbstractComponent existing = haComponents.get(component.getComponentId()); if (existing != null) { // DeviceTriggers that are for the same subtype, topic, and value template // can be coalesced together @@ -455,6 +506,7 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler }); } haComponentsByUniqueId.put(component.getUniqueId(), component); + haComponentsByHaId.put(component.getHaID(), component); return false; } } @@ -467,6 +519,7 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler } haComponents.put(component.getComponentId(), component); haComponentsByUniqueId.put(component.getUniqueId(), component); + haComponentsByHaId.put(component.getHaID(), component); return true; } @@ -478,16 +531,16 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler ChannelUID channelUID) { Object component = multiComponentChannelConfig.get("component"); Object nodeid = multiComponentChannelConfig.get("nodeid"); - if ((multiComponentChannelConfig.get("objectid") instanceof List objectIds) - && (multiComponentChannelConfig.get("config") instanceof List configurations)) { + if ((multiComponentChannelConfig.get("objectid") instanceof List objectIds) + && (multiComponentChannelConfig.get("config") instanceof List configurations)) { if (objectIds.size() != configurations.size()) { logger.warn("objectid and config for channel {} do not have the same number of items; ignoring", channelUID); return List.of(); } - List result = new ArrayList(); - Iterator objectIdIterator = objectIds.iterator(); - Iterator configIterator = configurations.iterator(); + List result = new ArrayList<>(); + Iterator objectIdIterator = objectIds.iterator(); + Iterator configIterator = configurations.iterator(); while (objectIdIterator.hasNext()) { Configuration componentConfiguration = new Configuration(); componentConfiguration.put("component", component); diff --git a/bundles/org.openhab.binding.mqtt.homeassistant/src/test/java/org/openhab/binding/mqtt/homeassistant/internal/discovery/HomeAssistantDiscoveryTests.java b/bundles/org.openhab.binding.mqtt.homeassistant/src/test/java/org/openhab/binding/mqtt/homeassistant/internal/discovery/HomeAssistantDiscoveryTests.java index 15999087f4b..d44e5a9763f 100644 --- a/bundles/org.openhab.binding.mqtt.homeassistant/src/test/java/org/openhab/binding/mqtt/homeassistant/internal/discovery/HomeAssistantDiscoveryTests.java +++ b/bundles/org.openhab.binding.mqtt.homeassistant/src/test/java/org/openhab/binding/mqtt/homeassistant/internal/discovery/HomeAssistantDiscoveryTests.java @@ -15,13 +15,13 @@ package org.openhab.binding.mqtt.homeassistant.internal.discovery; import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.MatcherAssert.assertThat; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; -import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -55,14 +55,6 @@ public class HomeAssistantDiscoveryTests extends AbstractHomeAssistantTests { discovery = new TestHomeAssistantDiscovery(channelTypeProvider); } - @Test - public void testComponentNameSummary() { - assertThat( - HomeAssistantDiscovery.getComponentNamesSummary( - Stream.of("Sensor", "Switch", "Sensor", "Foobar", "Foobar", "Foobar")), // - is("3x Foobar, 2x Sensor, Switch")); - } - @Test public void testOneThingDiscovery() throws Exception { var discoveryListener = new LatchDiscoveryListener(); @@ -88,11 +80,107 @@ public class HomeAssistantDiscoveryTests extends AbstractHomeAssistantTests { assertThat(result.getProperties().get(Thing.PROPERTY_VENDOR), is("TuYa")); assertThat(result.getProperties().get(Thing.PROPERTY_FIRMWARE_VERSION), is("Zigbee2MQTT 1.18.2")); assertThat(result.getProperties().get(HandlerConfiguration.PROPERTY_BASETOPIC), is("homeassistant")); - assertThat(result.getLabel(), is("th1 (Climate Control, Switch)")); + assertThat(result.getLabel(), is("th1")); assertThat((List) result.getProperties().get(HandlerConfiguration.PROPERTY_TOPICS), hasItems( "climate/0x847127fffe11dd6a_climate_zigbee2mqtt", "switch/0x847127fffe11dd6a_auto_lock_zigbee2mqtt")); } + @Test + public void testComponentAddedToExistingThing() throws Exception { + var discoveryListener = new LatchDiscoveryListener(); + var latch = discoveryListener.createWaitForThingsDiscoveredLatch(1); + + // When discover one thing with two channels + discovery.addDiscoveryListener(discoveryListener); + discovery.receivedMessage(HA_UID, bridgeConnection, + "homeassistant/climate/0x847127fffe11dd6a_climate_zigbee2mqtt/config", + getResourceAsByteArray("component/configTS0601ClimateThermostat.json")); + + // Then one thing found + assert latch.await(3, TimeUnit.SECONDS); + var discoveryResults = discoveryListener.getDiscoveryResults(); + assertThat(discoveryResults.size(), is(1)); + var result = discoveryResults.get(0); + assertThat(result.getBridgeUID(), is(HA_UID)); + assertThat(result.getProperties().get(Thing.PROPERTY_MODEL_ID), + is("Radiator valve with thermostat (TS0601_thermostat)")); + assertThat(result.getProperties().get(Thing.PROPERTY_VENDOR), is("TuYa")); + assertThat(result.getProperties().get(Thing.PROPERTY_FIRMWARE_VERSION), is("Zigbee2MQTT 1.18.2")); + assertThat(result.getProperties().get(HandlerConfiguration.PROPERTY_BASETOPIC), is("homeassistant")); + assertThat(result.getLabel(), is("th1")); + assertThat((List) result.getProperties().get(HandlerConfiguration.PROPERTY_TOPICS), + hasItems("climate/0x847127fffe11dd6a_climate_zigbee2mqtt")); + + // Now another component added to the same thing + latch = discoveryListener.createWaitForThingsDiscoveredLatch(1); + discovery.receivedMessage(HA_UID, bridgeConnection, + "homeassistant/switch/0x847127fffe11dd6a_auto_lock_zigbee2mqtt/config", + getResourceAsByteArray("component/configTS0601AutoLock.json")); + + assert latch.await(3, TimeUnit.SECONDS); + discoveryResults = discoveryListener.getDiscoveryResults(); + assertThat(discoveryResults.size(), is(1)); + result = discoveryResults.get(0); + assertThat(result.getBridgeUID(), is(HA_UID)); + assertThat(result.getProperties().get(Thing.PROPERTY_MODEL_ID), + is("Radiator valve with thermostat (TS0601_thermostat)")); + assertThat(result.getProperties().get(Thing.PROPERTY_VENDOR), is("TuYa")); + assertThat(result.getProperties().get(Thing.PROPERTY_FIRMWARE_VERSION), is("Zigbee2MQTT 1.18.2")); + assertThat(result.getProperties().get(HandlerConfiguration.PROPERTY_BASETOPIC), is("homeassistant")); + assertThat(result.getLabel(), is("th1")); + assertThat((List) result.getProperties().get(HandlerConfiguration.PROPERTY_TOPICS), hasItems( + "climate/0x847127fffe11dd6a_climate_zigbee2mqtt", "switch/0x847127fffe11dd6a_auto_lock_zigbee2mqtt")); + } + + @Test + public void testComponentRemovedFromExistingThing() throws Exception { + var discoveryListener = new LatchDiscoveryListener(); + var latch = discoveryListener.createWaitForThingsDiscoveredLatch(1); + + // When discover one thing with two channels + discovery.addDiscoveryListener(discoveryListener); + discovery.receivedMessage(HA_UID, bridgeConnection, + "homeassistant/climate/0x847127fffe11dd6a_climate_zigbee2mqtt/config", + getResourceAsByteArray("component/configTS0601ClimateThermostat.json")); + discovery.receivedMessage(HA_UID, bridgeConnection, + "homeassistant/switch/0x847127fffe11dd6a_auto_lock_zigbee2mqtt/config", + getResourceAsByteArray("component/configTS0601AutoLock.json")); + + // Then one thing found + assert latch.await(3, TimeUnit.SECONDS); + var discoveryResults = discoveryListener.getDiscoveryResults(); + assertThat(discoveryResults.size(), is(1)); + var result = discoveryResults.get(0); + assertThat(result.getBridgeUID(), is(HA_UID)); + assertThat(result.getProperties().get(Thing.PROPERTY_MODEL_ID), + is("Radiator valve with thermostat (TS0601_thermostat)")); + assertThat(result.getProperties().get(Thing.PROPERTY_VENDOR), is("TuYa")); + assertThat(result.getProperties().get(Thing.PROPERTY_FIRMWARE_VERSION), is("Zigbee2MQTT 1.18.2")); + assertThat(result.getProperties().get(HandlerConfiguration.PROPERTY_BASETOPIC), is("homeassistant")); + assertThat(result.getLabel(), is("th1")); + assertThat((List) result.getProperties().get(HandlerConfiguration.PROPERTY_TOPICS), hasItems( + "climate/0x847127fffe11dd6a_climate_zigbee2mqtt", "switch/0x847127fffe11dd6a_auto_lock_zigbee2mqtt")); + + // Now remove the second component + latch = discoveryListener.createWaitForThingsDiscoveredLatch(1); + discovery.topicVanished(HA_UID, bridgeConnection, + "homeassistant/switch/0x847127fffe11dd6a_auto_lock_zigbee2mqtt/config"); + + assert latch.await(3, TimeUnit.SECONDS); + discoveryResults = discoveryListener.getDiscoveryResults(); + assertThat(discoveryResults.size(), is(1)); + result = discoveryResults.get(0); + assertThat(result.getBridgeUID(), is(HA_UID)); + assertThat(result.getProperties().get(Thing.PROPERTY_MODEL_ID), + is("Radiator valve with thermostat (TS0601_thermostat)")); + assertThat(result.getProperties().get(Thing.PROPERTY_VENDOR), is("TuYa")); + assertThat(result.getProperties().get(Thing.PROPERTY_FIRMWARE_VERSION), is("Zigbee2MQTT 1.18.2")); + assertThat(result.getProperties().get(HandlerConfiguration.PROPERTY_BASETOPIC), is("homeassistant")); + assertThat(result.getLabel(), is("th1")); + assertThat((List) result.getProperties().get(HandlerConfiguration.PROPERTY_TOPICS), + hasItems("climate/0x847127fffe11dd6a_climate_zigbee2mqtt")); + } + private static class TestHomeAssistantDiscovery extends HomeAssistantDiscovery { public TestHomeAssistantDiscovery(MqttChannelTypeProvider typeProvider) { super(null); @@ -122,8 +210,10 @@ public class HomeAssistantDiscoveryTests extends AbstractHomeAssistantTests { return Collections.emptyList(); } - public CopyOnWriteArrayList getDiscoveryResults() { - return discoveryResults; + public List getDiscoveryResults() { + ArrayList localResults = new ArrayList<>(discoveryResults); + discoveryResults.clear(); + return localResults; } public CountDownLatch createWaitForThingsDiscoveredLatch(int count) { diff --git a/itests/org.openhab.binding.mqtt.homeassistant.tests/src/main/java/org/openhab/binding/mqtt/homeassistant/HomeAssistantMQTTImplementationTest.java b/itests/org.openhab.binding.mqtt.homeassistant.tests/src/main/java/org/openhab/binding/mqtt/homeassistant/HomeAssistantMQTTImplementationTest.java index 64781741ea1..d001274eb60 100644 --- a/itests/org.openhab.binding.mqtt.homeassistant.tests/src/main/java/org/openhab/binding/mqtt/homeassistant/HomeAssistantMQTTImplementationTest.java +++ b/itests/org.openhab.binding.mqtt.homeassistant.tests/src/main/java/org/openhab/binding/mqtt/homeassistant/HomeAssistantMQTTImplementationTest.java @@ -138,6 +138,26 @@ public class HomeAssistantMQTTImplementationTest extends MqttOSGiTest { "Connection " + haConnection.getClientId() + " not retrieving all topics"); } + private static class ComponentDiscoveredProxy implements ComponentDiscovered { + private final Map> haComponents; + private final CountDownLatch latch; + + public ComponentDiscoveredProxy(Map> haComponents, CountDownLatch latch) { + this.haComponents = haComponents; + this.latch = latch; + } + + @Override + public void componentDiscovered(HaID homeAssistantTopicID, AbstractComponent component) { + haComponents.put(component.getComponentId(), component); + latch.countDown(); + } + + @Override + public void componentRemoved(HaID homeAssistantTopicID) { + } + } + @Test public void parseHATree() throws Exception { MqttChannelTypeProvider channelTypeProvider = mock(MqttChannelTypeProvider.class); @@ -154,10 +174,7 @@ public class HomeAssistantMQTTImplementationTest extends MqttOSGiTest { // In the following implementation we add the found component to the `haComponents` map // and add the types to the channelTypeProvider, like in the real Thing handler. final CountDownLatch latch = new CountDownLatch(1); - ComponentDiscovered cd = (haID, c) -> { - haComponents.put(c.getComponentId(), c); - latch.countDown(); - }; + ComponentDiscovered cd = new ComponentDiscoveredProxy(haComponents, latch); // Start the discovery for 2000ms. Forced timeout after 4000ms. HaID haID = new HaID(testObjectTopic + "/config");