Skip to content

Commit

Permalink
further simplify channel IDs
Browse files Browse the repository at this point in the history 
don't include the component type in the channel ID unless it's
necessary to resolve a conflicting object ID from another component.
this is now possible since we don't have to rely on the channel ID
matching the component ID to get its channel state

Signed-off-by: Cody Cutrer <cody@cutrer.us>
  • Loading branch information
@ccutrer
ccutrer committed Oct 1, 2024
1 parent 9711e53 commit 80f6ad7
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 27 deletions.
6 changes: 5 additions & 1 deletion ...stant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/ComponentChannel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,17 @@ public Channel getChannel() {
return channel;
}

public void replaceChannelUID(ChannelUID channelUID) {
public void resetUID(ChannelUID channelUID) {
channel = ChannelBuilder.create(channelUID, channel.getAcceptedItemType()).withType(channel.getChannelTypeUID())
.withKind(channel.getKind()).withLabel(Objects.requireNonNull(channel.getLabel()))
.withConfiguration(channel.getConfiguration()).withAutoUpdatePolicy(channel.getAutoUpdatePolicy())
.build();
}

public void clearConfiguration() {
channel = ChannelBuilder.create(channel).withConfiguration(new Configuration()).build();
}

public ChannelState getState() {
return channelState;
}
Expand Down
31 changes: 26 additions & 5 deletions ...ain/java/org/openhab/binding/mqtt/homeassistant/internal/component/AbstractComponent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,13 @@ public AbstractComponent(ComponentFactory.ComponentConfiguration componentConfig
this.haID = componentConfiguration.getHaID();

String name = channelConfiguration.getName();
if (newStyleChannels || (name != null && !name.isEmpty())) {
groupId = componentId = this.haID.getGroupId(channelConfiguration.getUniqueId(), newStyleChannels);
if (newStyleChannels) {
// try for a simple component/group ID first; if there are conflicts
// (components of different types, but the same object id)
// we'll resolve them later
groupId = componentId = haID.objectID.replace('-', '_');
} else if (name != null && !name.isEmpty()) {
groupId = componentId = this.haID.getGroupId(channelConfiguration.getUniqueId(), false);
} else {
groupId = null;
componentId = "";
Expand Down Expand Up @@ -148,13 +153,25 @@ public AbstractComponent(ComponentFactory.ComponentConfiguration componentConfig
}

protected void finalizeChannels() {
if (newStyleChannels && channels.size() == 1) {
if (!newStyleChannels) {
return;
}
if (channels.size() == 1) {
groupId = null;
channels.forEach(
(id, componentChannel) -> componentChannel.replaceChannelUID(buildChannelUID(componentId)));
channels.values().forEach(c -> c.resetUID(buildChannelUID(componentId)));
} else {
// only the first channel needs to persist the configuration
channels.values().stream().skip(1).forEach(c -> {
c.clearConfiguration();
});
}
}

public void resolveConflict() {
componentId = this.haID.getGroupId(channelConfiguration.getUniqueId(), newStyleChannels);
channels.values().forEach(c -> c.resetUID(buildChannelUID(c.getChannel().getUID().getIdWithoutGroup())));
}

protected ComponentChannel.Builder buildChannel(String channelID, ComponentChannelType channelType,
Value valueState, String label, ChannelStateUpdateListener channelStateUpdateListener) {
if (groupId == null) {
Expand Down Expand Up @@ -227,6 +244,10 @@ public String getComponentId() {
return componentId;
}

public String getUniqueId() {
return uniqueId;
}

/**
* Component (Channel Group) name.
*/
Expand Down
59 changes: 41 additions & 18 deletions ...va/org/openhab/binding/mqtt/homeassistant/internal/handler/HomeAssistantThingHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.openhab.binding.mqtt.homeassistant.internal.component.Update;
import org.openhab.binding.mqtt.homeassistant.internal.config.ChannelConfigurationTypeAdapterFactory;
import org.openhab.binding.mqtt.homeassistant.internal.exception.ConfigurationException;
import org.openhab.core.config.core.Configuration;
import org.openhab.core.config.core.validation.ConfigValidationException;
import org.openhab.core.io.transport.mqtt.MqttBrokerConnection;
import org.openhab.core.thing.Channel;
Expand Down Expand Up @@ -97,6 +98,7 @@ public class HomeAssistantThingHandler extends AbstractMQTTThingHandler

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<ChannelUID, ChannelState> channelStates = new HashMap<>();

protected HandlerConfiguration config = new HandlerConfiguration();
Expand Down Expand Up @@ -154,7 +156,14 @@ public void initialize() {
continue;
}
}
HaID haID = HaID.fromConfig(config.basetopic, channel.getConfiguration());
Configuration channelConfig = channel.getConfiguration();
if (!channelConfig.containsKey("component")
|| !channelConfig.containsKey("objectid") | !channelConfig.containsKey("config")) {
// Must be a secondary channel
continue;
}

HaID haID = HaID.fromConfig(config.basetopic, channelConfig);

if (!config.topics.contains(haID.getTopic())) {
// don't add a component for this channel that isn't configured on the thing
Expand All @@ -165,21 +174,17 @@ public void initialize() {

discoveryHomeAssistantIDs.add(haID);
ThingUID thingUID = channel.getUID().getThingUID();
String channelConfigurationJSON = (String) channel.getConfiguration().get("config");
if (channelConfigurationJSON == null) {
logger.warn("Provided channel does not have a 'config' configuration key!");
} else {
try {
AbstractComponent<?> component = ComponentFactory.createComponent(thingUID, haID,
channelConfigurationJSON, this, this, scheduler, gson, jinjava, newStyleChannels);
if (typeID.equals(MqttBindingConstants.HOMEASSISTANT_MQTT_THING)) {
typeID = calculateThingTypeUID(component);
}

haComponents.put(component.getComponentId(), component);
} catch (ConfigurationException e) {
logger.error("Cannot restore component {}: {}", thing, e.getMessage());
String channelConfigurationJSON = (String) channelConfig.get("config");
try {
AbstractComponent<?> component = ComponentFactory.createComponent(thingUID, haID,
channelConfigurationJSON, this, this, scheduler, gson, jinjava, newStyleChannels);
if (typeID.equals(MqttBindingConstants.HOMEASSISTANT_MQTT_THING)) {
typeID = calculateThingTypeUID(component);
}

addComponent(component);
} catch (ConfigurationException e) {
logger.error("Cannot restore component {}: {}", thing, e.getMessage());
}
}
if (updateThingType(typeID)) {
Expand Down Expand Up @@ -272,22 +277,26 @@ public void accept(List<AbstractComponent<?>> discoveredComponentsList) {
if (typeID.equals(MqttBindingConstants.HOMEASSISTANT_MQTT_THING)) {
typeID = calculateThingTypeUID(discovered);
}
String id = discovered.getComponentId();
AbstractComponent<?> known = haComponents.get(id);
AbstractComponent<?> known = haComponentsByUniqueId.get(discovered.getUniqueId());
// Is component already known?
if (known != null) {
if (discovered.getConfigHash() != known.getConfigHash()) {
// Don't wait for the future to complete. We are also not interested in failures.
// The component will be replaced in a moment.
known.stop();
haComponentsByUniqueId.remove(discovered.getUniqueId());
haComponents.remove(known.getComponentId());
if (!known.getComponentId().equals(discovered.getComponentId())) {
discovered.resolveConflict();
}
} else {
known.setConfigSeen();
continue;
}
}

// Add component to the component map
haComponents.put(id, discovered);
addComponent(discovered);
// Start component / Subscribe to channel topics
discovered.start(connection, scheduler, 0).exceptionally(e -> {
logger.warn("Failed to start component {}", discovered.getHaID(), e);
Expand Down Expand Up @@ -408,4 +417,18 @@ private void releaseStateUpdated(Update.ReleaseState state) {
properties = state.appendToProperties(properties);
updateProperties(properties);
}

// should only be called when it's safe to access haComponents
private void addComponent(AbstractComponent component) {
AbstractComponent existing = haComponents.get(component.getComponentId());
if (existing != null) {
// rename the conflict
haComponents.remove(existing.getComponentId());
existing.resolveConflict();
component.resolveConflict();
haComponents.put(existing.getComponentId(), existing);
}
haComponents.put(component.getComponentId(), component);
haComponentsByUniqueId.put(component.getUniqueId(), component);
}
}
6 changes: 3 additions & 3 deletions ...ding.mqtt.homeassistant/src/main/resources/OH-INF/config/homeassistant-channel-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
xsi:schemaLocation="https://openhab.org/schemas/config-description/v1.0.0 https://openhab.org/schemas/config-description-1.0.0.xsd">

<config-description uri="channel-type:mqtt:ha-channel">
<parameter name="component" type="text" readOnly="true" required="true">
<parameter name="component" type="text" readOnly="true">
<label>Component</label>
<description>Home Assistant component type (e.g. binary_sensor, switch, light)</description>
<default></default>
Expand All @@ -15,12 +15,12 @@
<description>Optional node name of the component</description>
<default></default>
</parameter>
<parameter name="objectid" type="text" readOnly="true" required="true">
<parameter name="objectid" type="text" readOnly="true">
<label>Object ID</label>
<description>Object ID of the component</description>
<default></default>
</parameter>
<parameter name="config" type="text" readOnly="true" required="true">
<parameter name="config" type="text" readOnly="true">
<label>JSON Configuration</label>
<description>The JSON configuration string received by the component via MQTT.</description>
<default></default>
Expand Down

0 comments on commit 80f6ad7

Please sign in to comment.