[mqtt] Have a working Group Id

addons/binding/org.openhab.binding.mqtt.generic.test/src/test/java/org/openhab/binding/mqtt/generic/HomeAssistantMQTTImplementationTests.java

@@ -45,10 +45,10 @@
 import org.junit.Test;
 import org.mockito.Mock;
 import org.openhab.binding.mqtt.generic.internal.convention.homeassistant.AbstractComponent;
+import org.openhab.binding.mqtt.generic.internal.convention.homeassistant.ChannelConfigurationTypeAdapterFactory;
 import org.openhab.binding.mqtt.generic.internal.convention.homeassistant.ComponentSwitch;
 import org.openhab.binding.mqtt.generic.internal.convention.homeassistant.DiscoverComponents;
 import org.openhab.binding.mqtt.generic.internal.convention.homeassistant.DiscoverComponents.ComponentDiscovered;
-import org.openhab.binding.mqtt.generic.internal.convention.homeassistant.ChannelConfigurationTypeAdapterFactory;
 import org.openhab.binding.mqtt.generic.internal.convention.homeassistant.HaID;
 import org.openhab.binding.mqtt.generic.internal.generic.ChannelStateUpdateListener;
 import org.openhab.binding.mqtt.generic.internal.generic.MqttChannelTypeProvider;
@@ -175,7 +175,7 @@ public void parseHATree() throws InterruptedException, ExecutionException, Timeo
  // 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(haID.getChannelGroupID(), c);
+ haComponents.put(c.uid().getId(), c);
  c.addChannelTypes(channelTypeProvider);
  channelTypeProvider.setChannelGroupType(c.groupTypeUID(), c.type());
  latch.countDown();
@@ -192,17 +192,19 @@ public void parseHATree() throws InterruptedException, ExecutionException, Timeo
  assertTrue(latch.await(300, TimeUnit.MILLISECONDS));
  future.get(100, TimeUnit.MILLISECONDS);
 
- // No failure expected and one discoverd result
+ // No failure expected and one discovered result
  assertNull(failure);
  assertThat(haComponents.size(), is(1));
 
  // For the switch component we should have one channel group type and one channel type
  verify(channelTypeProvider, times(1)).setChannelGroupType(any(), any());
  verify(channelTypeProvider, times(1)).setChannelType(any(), any());
 
+ String channelGroupId = ThingChannelConstants.testHomeAssistantThing.getId() + "_switch";
+
  // We expect a switch component with an OnOff channel with the initial value UNDEF:
- State value = haComponents.get(haID.getChannelGroupID()).channelTypes().get(ComponentSwitch.switchChannelID)
- .getState().getCache().getChannelState();
+ State value = haComponents.get(channelGroupId).channelTypes().get(ComponentSwitch.switchChannelID).getState()
+ .getCache().getChannelState();
  assertThat(value, is(UnDefType.UNDEF));
 
  haComponents.values().stream().map(e -> e.start(connection, scheduler, 100))
@@ -215,8 +217,8 @@ public void parseHATree() throws InterruptedException, ExecutionException, Timeo
  verify(channelStateUpdateListener, times(1)).updateChannelState(any(), any());
 
  // Value should be ON now.
- value = haComponents.get(haID.getChannelGroupID()).channelTypes().get(ComponentSwitch.switchChannelID)
- .getState().getCache().getChannelState();
+ value = haComponents.get(channelGroupId).channelTypes().get(ComponentSwitch.switchChannelID).getState()
+ .getCache().getChannelState();
  assertThat(value, is(OnOffType.ON));
 
  }

addons/binding/org.openhab.binding.mqtt.generic.test/src/test/java/org/openhab/binding/mqtt/generic/internal/convention/homeassistant/DiscoverComponentsTests.java

@@ -74,8 +74,9 @@ public void discoveryTimeTest() throws InterruptedException, ExecutionException,
  DiscoverComponents discover = spy(new DiscoverComponents(ThingChannelConstants.testHomeAssistantThing,
  scheduler, null, gson, transformationServiceProvider));
 
- discover.startDiscovery(connection, 50, new HaID("homeassistant", "object", "node", "component"), discovered)
- .get(100, TimeUnit.MILLISECONDS);
+ HandlerConfiguration config = new HandlerConfiguration("homeassistant", "object");
+
+ discover.startDiscovery(connection, 50, HaID.fromConfig(config), discovered).get(100, TimeUnit.MILLISECONDS);
 
  }
 }

addons/binding/org.openhab.binding.mqtt.generic.test/src/test/java/org/openhab/binding/mqtt/generic/internal/convention/homeassistant/HaIDTests.java

@@ -15,30 +15,53 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
 
-import org.eclipse.smarthome.core.thing.type.ChannelTypeUID;
+import org.eclipse.smarthome.config.core.Configuration;
 import org.junit.Test;
-import org.openhab.binding.mqtt.generic.internal.convention.homeassistant.HaID;
 
 public class HaIDTests {
 
  @Test
  public void testWithoutNode() {
  HaID subject = new HaID("homeassistant/switch/name/config");
 
- assertThat(subject.getThingID(), is("name"));
- assertThat(subject.getChannelGroupTypeID(), is("name_switch"));
- assertThat(subject.getChannelTypeID("channel"), is(new ChannelTypeUID("mqtt:name_switch_channel")));
- assertThat(subject.getChannelGroupID(), is("switch_"));
+ assertThat(subject.objectID, is("name"));
+
+ assertThat(subject.component, is("switch"));
+ assertThat(subject.getTopic("suffix"), is("homeassistant/switch/name/suffix"));
+
+ Configuration config = new Configuration();
+ subject.toConfig(config);
+
+ HaID restore = HaID.fromConfig("homeassistant", config);
+
+ assertThat(restore, is(subject));
+
+ HandlerConfiguration haConfig = subject.toHandlerConfiguration();
+
+ restore = HaID.fromConfig(haConfig);
+ assertThat(restore, is(new HaID("homeassistant/+/name/config")));
  }
 
  @Test
  public void testWithNode() {
  HaID subject = new HaID("homeassistant/switch/node/name/config");
 
- assertThat(subject.getThingID(), is("name"));
- assertThat(subject.getChannelGroupTypeID(), is("name_switchnode"));
- assertThat(subject.getChannelTypeID("channel"), is(new ChannelTypeUID("mqtt:name_switchnode_channel")));
- assertThat(subject.getChannelGroupID(), is("switch_node"));
+ assertThat(subject.objectID, is("name"));
+
+ assertThat(subject.component, is("switch"));
+ assertThat(subject.getTopic("suffix"), is("homeassistant/switch/node/name/suffix"));
+
+ Configuration config = new Configuration();
+ subject.toConfig(config);
+
+ HaID restore = HaID.fromConfig("homeassistant", config);
+
+ assertThat(restore, is(subject));
+
+ HandlerConfiguration haConfig = subject.toHandlerConfiguration();
+
+ restore = HaID.fromConfig(haConfig);
+ assertThat(restore, is(new HaID("homeassistant/+/node/name/config")));
  }
 
 }

addons/binding/org.openhab.binding.mqtt.generic/ESH-INF/config/homeassistant-channel-config.xml

@@ -5,7 +5,22 @@
  xsi:schemaLocation="http://eclipse.org/smarthome/schemas/config-description/v1.0.0 http://eclipse.org/smarthome/schemas/config-description-1.0.0.xsd">
 
  <config-description uri="mqtt:ha_channel">
- <parameter name="config" type="text">
+ <parameter name="component" type="text" readOnly="true" required="true">
+ <label>Component</label>
+ <description>Type of the channel group.</description>
+ <default></default>
+ </parameter>
+ <parameter name="nodeid" type="text" readOnly="true">
+ <label>Node ID</label>
+ <description>Optional node name of the component</description>
+ <default></default>
+ </parameter>
+ <parameter name="objectid" type="text" readOnly="true" required="true">
+ <label>Object ID</label>
+ <description>Object id of the component</description>
+ <default></default>
+ </parameter>
+ <parameter name="config" type="text" readOnly="true" required="true">
  <label>Json Configuration</label>
  <description>The json configuration string received by the component via MQTT.</description>
  <default></default>

addons/binding/org.openhab.binding.mqtt.generic/README.md

@@ -23,6 +23,7 @@ Find the next table to understand the topology mapping from Homie to the Framewo
 | Property | Channel | homie/super-car/engine/temperature |
 
 System trigger channels are supported using non-retained properties, with *enum* data type and with the following formats:
+
 * Format: "PRESSED,RELEASED" -> system.rawbutton
 * Format: "SHORT\_PRESSED,DOUBLE\_PRESSED,LONG\_PRESSED" -> system.button
 * Format: "DIR1\_PRESSED,DIR1\_RELEASED,DIR2\_PRESSED,DIR2\_RELEASED" -> system.rawrocker
@@ -124,6 +125,7 @@ You can connect this channel to a Contact or Switch item.
 * __on__: An optional string (like "BRIGHT") that is recognized as on state. (ON will always be recognized.)
 * __off__: An optional string (like "DARK") that is recognized as off state. (OFF will always be recognized.)
 * __onBrightness__: If you connect this channel to a Switch item and turn it on,
+
 color and saturation are preserved from the last state, but
 the brightness will be set to this configured initial brightness (default: 10%).
 
@@ -234,9 +236,9 @@ Here are a few examples:
 ## Troubleshooting
 
 * If you get the error "No MQTT client": Please update your installation.
-* If you use the Mosquitto broker: Please be aware that there is a relatively low setting
- for retained messages. At some point messages will just not being delivered
- anymore: Change the setting 
+* If you use the Mosquitto broker: Please be aware that there is a relatively low setting 
+for retained messages. At some point messages will just not being delivered anymore: 
+Change the setting 
 
 ## Examples
 

addons/binding/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/internal/convention/homeassistant/AbstractComponent.java

@@ -19,6 +19,7 @@
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.stream.Collectors;
 
+import org.apache.commons.lang.StringUtils;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.smarthome.core.thing.ChannelGroupUID;
@@ -65,14 +66,20 @@ public abstract class AbstractComponent<C extends BaseChannelConfiguration> {
  */
  public AbstractComponent(CFactory.ComponentConfiguration componentConfiguration, Class<C> clazz) {
  this.componentConfiguration = componentConfiguration;
- this.haID = componentConfiguration.getHaID();
- this.channelGroupTypeUID = new ChannelGroupTypeUID(MqttBindingConstants.BINDING_ID,
- haID.getChannelGroupTypeID());
- this.channelGroupUID = new ChannelGroupUID(componentConfiguration.getThingUID(), haID.getChannelGroupID());
 
  this.channelConfigurationJson = componentConfiguration.getConfigJSON();
  this.channelConfiguration = componentConfiguration.getConfig(clazz);
  this.configHash = channelConfigurationJson.hashCode();
+
+ this.haID = componentConfiguration.getHaID();
+
+ String groupId = channelConfiguration.unique_id;
+ if (groupId == null || StringUtils.isBlank(groupId)) {
+ groupId = this.haID.getFallbackGroupId();
+ }
+
+ this.channelGroupTypeUID = new ChannelGroupTypeUID(MqttBindingConstants.BINDING_ID, groupId);
+ this.channelGroupUID = new ChannelGroupUID(componentConfiguration.getThingUID(), groupId);
  }
 
  protected CChannel.Builder buildChannel(String channelID, Value valueState, String label) {

addons/binding/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/internal/convention/homeassistant/CChannel.java

@@ -180,7 +180,8 @@ public CChannel build(boolean addToComponent) {
  ChannelTypeUID channelTypeUID;
 
  channelUID = new ChannelUID(component.channelGroupUID, channelID);
- channelTypeUID = component.haID.getChannelTypeID(channelID);
+ channelTypeUID = new ChannelTypeUID(MqttBindingConstants.BINDING_ID,
+ channelUID.getGroupId() + "_" + channelID);
  channelState = new ChannelState(
  ChannelConfigBuilder.create().withRetain(retain).withStateTopic(state_topic)
  .withCommandTopic(command_topic).build(),
@@ -197,6 +198,8 @@ public CChannel build(boolean addToComponent) {
 
  Configuration configuration = new Configuration();
  configuration.put("config", component.channelConfigurationJson);
+ component.haID.toConfig(configuration);
+
  channel = ChannelBuilder.create(channelUID, channelState.getItemType()).withType(channelTypeUID)
  .withKind(type.getKind()).withLabel(label).withConfiguration(configuration).build();
 

addons/binding/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/internal/convention/homeassistant/CFactory.java

@@ -14,7 +14,6 @@
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
-import org.eclipse.smarthome.core.thing.Channel;
 import org.eclipse.smarthome.core.thing.ThingUID;
 import org.openhab.binding.mqtt.generic.internal.generic.ChannelStateUpdateListener;
 import org.openhab.binding.mqtt.generic.internal.generic.TransformationServiceProvider;
@@ -44,11 +43,12 @@ public class CFactory {
  * @param updateListener A channel state update listener
  * @return A HA MQTT Component
  */
- public static @Nullable AbstractComponent<?> createComponent(ThingUID thingUID, HaID haID, String configJSON,
- @Nullable ChannelStateUpdateListener updateListener, Gson gson,
+ public static @Nullable AbstractComponent<?> createComponent(ThingUID thingUID, HaID haID,
+ String channelConfigurationJSON, @Nullable ChannelStateUpdateListener updateListener, Gson gson,
  TransformationServiceProvider transformationServiceProvider) {
- ComponentConfiguration componentConfiguration = new ComponentConfiguration(thingUID, haID, configJSON, gson)
- .listener(updateListener).transformationProvider(transformationServiceProvider);
+ ComponentConfiguration componentConfiguration = new ComponentConfiguration(thingUID, haID,
+ channelConfigurationJSON, gson).listener(updateListener)
+ .transformationProvider(transformationServiceProvider);
  try {
  switch (haID.component) {
  case "alarm_control_panel":
@@ -78,27 +78,6 @@ public class CFactory {
  return null;
  }
 
- /**
- * Create a HA MQTT component by a given channel configuration.
- *
- * @param basetopic The MQTT base topic, usually "homeassistant"
- * @param channel A channel with the JSON configuration embedded as configuration (key: 'config')
- * @param updateListener A channel state update listener
- * @return A HA MQTT Component
- */
- public static @Nullable AbstractComponent<?> createComponent(String basetopic, Channel channel,
- @Nullable ChannelStateUpdateListener updateListener, Gson gson,
- TransformationServiceProvider transformationServiceProvider) {
- HaID haID = new HaID(basetopic, channel.getUID());
- ThingUID thingUID = channel.getUID().getThingUID();
- String configJSON = (String) channel.getConfiguration().get("config");
- if (configJSON == null) {
- logger.warn("Provided channel does not have a 'config' configuration key!");
- return null;
- }
- return createComponent(thingUID, haID, configJSON, updateListener, gson, transformationServiceProvider);
- }
-
  protected static class ComponentConfiguration {
  private ThingUID thingUID;
  private HaID haID;

addons/binding/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/internal/convention/homeassistant/DiscoverComponents.java

@@ -51,7 +51,6 @@ public class DiscoverComponents implements MqttMessageSubscriber {
  private WeakReference<@Nullable MqttBrokerConnection> connectionRef = new WeakReference<>(null);
  protected @NonNullByDefault({}) ComponentDiscovered discoveredListener;
  private int discoverTime;
- private String topicWithNode = "";
  private String topic = "";
 
  /**
@@ -106,8 +105,10 @@ public void processMessage(String topic, byte[] payload) {
  * </p>
  *
  * @param connection A MQTT broker connection
- * @param discoverTime The time in milliseconds for the discovery to run. Can be 0 to disable the timeout.
- * You need to call {@link #stopDiscovery(MqttBrokerConnection)} at some point in that case.
+ * @param discoverTime The time in milliseconds for the discovery to run. Can be 0 to disable the
+ * timeout.
+ * You need to call {@link #stopDiscovery(MqttBrokerConnection)} at some
+ * point in that case.
  * @param topicDescription Contains the object-id (=device id) and potentially a node-id as well.
  * @param componentsDiscoveredListener Listener for results
  * @return A future that completes normally after the given time in milliseconds or exceptionally on any error.
@@ -116,15 +117,13 @@ public void processMessage(String topic, byte[] payload) {
  public CompletableFuture<@Nullable Void> startDiscovery(MqttBrokerConnection connection, int discoverTime,
  HaID topicDescription, ComponentDiscovered componentsDiscoveredListener) {
 
- this.topicWithNode = topicDescription.baseTopic + "/+/+/" + topicDescription.objectID + "/config";
- this.topic = topicDescription.baseTopic + "/+/" + topicDescription.objectID + "/config";
+ this.topic = topicDescription.getTopic("config");
  this.discoverTime = discoverTime;
  this.discoveredListener = componentsDiscoveredListener;
  this.connectionRef = new WeakReference<>(connection);
 
- // Subscribe to the wildcard topics and start receive MQTT retained topics
- CompletableFuture.allOf(connection.subscribe(topic, this), connection.subscribe(topicWithNode, this))
- .thenRun(this::subscribeSuccess).exceptionally(this::subscribeFail);
+ // Subscribe to the wildcard topic and start receive MQTT retained topics
+ connection.subscribe(topic, this).thenRun(this::subscribeSuccess).exceptionally(this::subscribeFail);
 
  return discoverFinishedFuture;
  }
@@ -135,7 +134,6 @@ private void subscribeSuccess() {
  if (connection != null && discoverTime > 0) {
  this.stopDiscoveryFuture = scheduler.schedule(() -> {
  this.stopDiscoveryFuture = null;
- connection.unsubscribe(topicWithNode, this);
  connection.unsubscribe(topic, this);
  this.discoveredListener = null;
  discoverFinishedFuture.complete(null);
@@ -155,7 +153,6 @@ private void subscribeSuccess() {
  this.discoveredListener = null;
  final MqttBrokerConnection connection = connectionRef.get();
  if (connection != null) {
- connection.unsubscribe(topicWithNode, this);
  connection.unsubscribe(topic, this);
  connectionRef.clear();
  }