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

[mqtt] Have a working Group Id #5156

Merged
merged 5 commits into from
Mar 23, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -24,9 +24,9 @@ public class HaIDTests {
public void testWithoutNode() {
HaID subject = new HaID("homeassistant/switch/name/config");

assertThat(subject.getObjectID(), is("name"));
assertThat(subject.objectID, is("name"));

assertThat(subject.getComponent(), is("switch"));
assertThat(subject.component, is("switch"));
assertThat(subject.getTopic("suffix"), is("homeassistant/switch/name/suffix"));

Configuration config = new Configuration();
Expand All @@ -46,9 +46,9 @@ public void testWithoutNode() {
public void testWithNode() {
HaID subject = new HaID("homeassistant/switch/node/name/config");

assertThat(subject.getObjectID(), is("name"));
assertThat(subject.objectID, is("name"));

assertThat(subject.getComponent(), is("switch"));
assertThat(subject.component, is("switch"));
assertThat(subject.getTopic("suffix"), is("homeassistant/switch/node/name/suffix"));

Configuration config = new Configuration();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public class CFactory {
channelConfigurationJSON, gson).listener(updateListener)
.transformationProvider(transformationServiceProvider);
try {
switch (haID.getComponent()) {
switch (haID.component) {
case "alarm_control_panel":
return new ComponentAlarmControlPanel(componentConfiguration);
case "binary_sensor":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ public void processMessage(String topic, byte[] payload) {
AbstractComponent<?> component = CFactory.createComponent(thingUID, haID, config, updateListener, gson,
transformationServiceProvider);
if (component != null) {
logger.trace("Found HomeAssistant thing {} component {}", haID.getObjectID(), haID.getComponent());
logger.trace("Found HomeAssistant thing {} component {}", haID.objectID, haID.component);
if (discoveredListener != null) {
discoveredListener.componentDiscovered(haID, component);
}
} else {
logger.debug("Configuration of HomeAssistant thing {} invalid: {}", haID.getObjectID(), config);
logger.debug("Configuration of HomeAssistant thing {} invalid: {}", haID.objectID, config);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,19 @@
* followed by the component id, an optional node id and the object id.
*
* This helper class can split up an MQTT topic into such parts.
* <p>
* Implementation note: This is an immutable class.
*
* @author David Graeff - Initial contribution
*/
@NonNullByDefault
public class HaID {
final private String baseTopic;
final private String component;
final private String nodeID;
final private String objectID;
public final String baseTopic;
public final String component;
public final String nodeID;
public final String objectID;

private final String _topic;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a correct name according to the Java / openHAB coding guidelines


/**
* Creates a {@link HaID} object for a given HomeAssistant MQTT topic.
Expand All @@ -41,19 +45,25 @@ public class HaID {
*/
public HaID(String mqttTopic) {
String[] strings = mqttTopic.split("/");
if (strings.length < 3) {
throw new IllegalArgumentException("MQTT topic not a HomeAssistant topic!");
if (strings.length < 4 || strings.length > 5) {
throw new IllegalArgumentException("MQTT topic not a HomeAssistant topic (wrong length)!");
}
if (!"config".equals(strings[strings.length - 1])) {
throw new IllegalArgumentException("MQTT topic not a HomeAssistant topic ('config' missing)!");
}
if (strings.length >= 5) {
component = strings[1];

baseTopic = strings[0];
component = strings[1];

if (strings.length == 5) {
nodeID = strings[2];
objectID = strings[3];
} else {
component = strings[1];
nodeID = "";
objectID = strings[2];
}
baseTopic = strings[0];

this._topic = createTopic(this);
}

public HaID() {
Expand All @@ -73,33 +83,64 @@ private HaID(String baseTopic, String objectID, String nodeID, String component)
this.objectID = objectID;
this.nodeID = nodeID;
this.component = component;
this._topic = createTopic(this);
}

private static final String createTopic(HaID id) {
StringBuilder str = new StringBuilder();
str.append(id.baseTopic).append('/').append(id.component).append('/');
if (StringUtils.isNotBlank(id.nodeID)) {
str.append(id.nodeID).append('/');
}
str.append(id.objectID).append('/');
return str.toString();
}

/**
* Extract the HaID information from a channel configuration.
* <p>
* <code>objectid</code>, <code>nodeid</code>, and <code>component</code> values are fetched from the configuration.
*
* @param baseTopic
* @param config
* @return newly created HaID
*/
public static HaID fromConfig(String baseTopic, Configuration config) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The factory pattern is like public constructors -> a fellow developer would really enjoy javadoc here.
There are also new public methods in this class, can you please add javadoc there as well? thanks

String objectID = (String) config.get("objectid");
String nodeID = (String) config.getProperties().getOrDefault("nodeid", "");
String component = (String) config.get("component");
return new HaID(baseTopic, objectID, nodeID, component);
}

public void toConfig(Configuration config) {
/**
* Add the HaID information to a channel configuration.
* <p>
* <code>objectid</code>, <code>nodeid</code>, and <code>component</code> values are added to the configuration.
*
* @param config
* @return the modified configuration
*/
public Configuration toConfig(Configuration config) {
config.put("objectid", objectID);
config.put("nodeid", nodeID);
config.put("component", component);
return config;
}

public HandlerConfiguration toHandlerConfiguration() {
String objectID = this.objectID;
if (StringUtils.isNotBlank(nodeID)) {
objectID = nodeID + "/" + objectID;
}

return new HandlerConfiguration(baseTopic, objectID);
}

/**
* Extract the HaID information from a thing configuration.
* <p>
* <code>basetpoic</code> and <code>objectid</code> are taken from the configuration.
* The <code>objectid</code> string may be in the form <code>nodeid/objectid</code>.
* <p>
* The <code>component</code> component in the resulting HaID will be set to <code>+</code>.
* This enables the HaID to be used as an mqtt subscription topic.
*
* @param config
* @return newly created HaID
*/
public static HaID fromConfig(HandlerConfiguration config) {
String baseTopic = config.getBasetopic();
String objectID = config.getObjectid();
String objectID = config.objectid;
String nodeID = "";

if (StringUtils.contains(objectID, '/')) {
Expand All @@ -112,9 +153,34 @@ public static HaID fromConfig(HandlerConfiguration config) {
nodeID = parts[0];
objectID = parts[1];
}
return new HaID(baseTopic, objectID, nodeID, "+");
return new HaID(config.basetopic, objectID, nodeID, "+");
}

/**
* Create a new thing configuration which contains the information from this HaID.
* <p>
* <code>objectid</code> in the thing configuration will be
* <code>nodeID/objectID<code> from the HaID, if <code>nodeID</code> is not empty.
* <p>
* <code>component</code> value will not be preserved.
*
* @return the new thing configuration
*/
public HandlerConfiguration toHandlerConfiguration() {
String objectID = this.objectID;
if (StringUtils.isNotBlank(nodeID)) {
objectID = nodeID + "/" + objectID;
}

return new HandlerConfiguration(baseTopic, objectID);
}

/**
* The default group id is the unique_id of the component, given in the config-json.
* If the unique id is not set, then a fallback is constructed from the HaID information.
*
* @return fallback group id
*/
public String getFallbackGroupId() {
StringBuilder str = new StringBuilder();

Expand All @@ -125,28 +191,18 @@ public String getFallbackGroupId() {
return str.toString();
}

public String getComponent() {
return component;
}

public String getObjectID() {
return objectID;
}

/**
* Return a topic, which can be used for a mqtt subscription.
* Defined values for suffix are:
* <ul>
* <li>config</li>
* <li>state</li>
* </ul>
*
* @return fallback group id
*/
public String getTopic(String suffix) {
StringBuilder str = new StringBuilder();

str.append(baseTopic).append('/').append(component).append('/');

if (StringUtils.isNotBlank(nodeID)) {
str.append(nodeID).append('/');
}
str.append(objectID);
if (StringUtils.isNotBlank(suffix)) {
str.append('/').append(suffix);
}

return str.toString();
return _topic + suffix;
}

@Override
Expand Down Expand Up @@ -189,6 +245,6 @@ public boolean equals(@Nullable Object obj) {

@Override
public String toString() {
return baseTopic + "/" + component + "/" + nodeID + "/" + objectID;
return _topic;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@ public class HandlerConfiguration {
/**
* The MQTT prefix topic
*/
private String basetopic = "homeassistant";
public final String basetopic;
/**
* The object id. This is comparable to a Homie Device.
*/
private String objectid = "";
public final String objectid;

public HandlerConfiguration() {
this("homeassistant", "");
}

public HandlerConfiguration(String basetopic, String objectid) {
Expand All @@ -44,16 +45,15 @@ public HandlerConfiguration(String basetopic, String objectid) {
this.objectid = objectid;
}

public String getBasetopic() {
return basetopic;
}

public String getObjectid() {
return objectid;
}

public void toProperties(Map<String, Object> properties) {
/**
* Add the <code>basetopic</code> and <code>objectid</code> to teh properties.
*
* @param properties
* @return the modified propertiess
*/
public <T extends Map<String, Object>> T appendToProperties(T properties) {
properties.put("basetopic", basetopic);
properties.put("objectid", objectid);
return properties;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void receivedMessage(ThingUID connectionBridge, MqttBrokerConnection conn
// Therefore the components are assembled into a list and given to the DiscoveryResult label for the user to
// easily recognize object capabilities.
HaID topicParts = determineTopicParts(topic);
final String thingID = topicParts.getObjectID();
final String thingID = topicParts.objectID;
final ThingUID thingUID = new ThingUID(MqttBindingConstants.HOMEASSISTANT_MQTT_THING, connectionBridge,
thingID);

Expand All @@ -146,11 +146,11 @@ public void receivedMessage(ThingUID connectionBridge, MqttBrokerConnection conn

// We need to keep track of already found component topics for a specific object_id/node_id
Set<String> components = componentsPerThingID.getOrDefault(thingID, new HashSet<>());
if (components.contains(topicParts.getComponent())) {
logger.trace("Discovered an already known component {}", topicParts.getComponent());
if (components.contains(topicParts.component)) {
logger.trace("Discovered an already known component {}", topicParts.component);
return; // If we already know about this object component, ignore the discovered topic.
}
components.add(topicParts.getComponent());
components.add(topicParts.component);
componentsPerThingID.put(thingID, components);

final String componentNames = components.stream().map(c -> HA_COMP_TO_NAME.getOrDefault(c, c))
Expand All @@ -161,7 +161,7 @@ public void receivedMessage(ThingUID connectionBridge, MqttBrokerConnection conn

Map<String, Object> properties = new HashMap<>();
HandlerConfiguration handlerConfig = topicParts.toHandlerConfiguration();
handlerConfig.toProperties(properties);
handlerConfig.appendToProperties(properties);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it like so: properties = handlerConfig.appendToProperties(properties);
I'm not sure if the java compiler is smart enough to figure out that it means the same (in c++ that's possible). But for someone reading the code it is more obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, should have thought about this...
Its too late after a working day...

config.addDeviceProperties(properties);
// First remove an already discovered thing with the same ID
thingRemoved(thingUID);
Expand All @@ -176,7 +176,7 @@ public void topicVanished(ThingUID connectionBridge, MqttBrokerConnection connec
if (!topic.endsWith("/config")) {
return;
}
final String thingID = determineTopicParts(topic).getObjectID();
final String thingID = determineTopicParts(topic).objectID;
componentsPerThingID.remove(thingID);
thingRemoved(new ThingUID(MqttBindingConstants.HOMEASSISTANT_MQTT_THING, connectionBridge, thingID));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler
protected HandlerConfiguration config = new HandlerConfiguration();
private HaID discoveryHomeAssistantID = new HaID();

protected final TransformationServiceProvider transformationServiceProvider;
protected final TransformationServiceProvider transformationServiceProvider;

/**
* Create a new thing handler for HomeAssistant MQTT components.
Expand Down Expand Up @@ -110,7 +110,7 @@ public HomeAssistantThingHandler(Thing thing, MqttChannelTypeProvider channelTyp
@Override
public void initialize() {
config = getConfigAs(HandlerConfiguration.class);
if (StringUtils.isEmpty(config.getObjectid())) {
if (StringUtils.isEmpty(config.objectid)) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Device ID unknown");
return;
}
Expand All @@ -129,7 +129,7 @@ public void initialize() {
continue;
}

HaID haID = HaID.fromConfig(config.getBasetopic(), channel.getConfiguration());
HaID haID = HaID.fromConfig(config.basetopic, channel.getConfiguration());
ThingUID thingUID = channel.getUID().getThingUID();
String channelConfigurationJSON = (String) channel.getConfiguration().get("config");
if (channelConfigurationJSON == null) {
Expand Down