Skip to content

Commit

Permalink
[mqtt.homeassistant] Improve support for Lock component
Browse files Browse the repository at this point in the history 
 * expose full state possibilities
 * expose OPEN command
  • Loading branch information
@ccutrer
ccutrer committed Nov 14, 2023
1 parent 0d4b679 commit a76400e
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 14 deletions.
53 changes: 47 additions & 6 deletions ...sistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Lock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@
*/
package org.openhab.binding.mqtt.homeassistant.internal.component;

import java.util.HashSet;
import java.util.Set;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.mqtt.generic.values.OnOffValue;
import org.openhab.binding.mqtt.generic.values.TextValue;
import org.openhab.binding.mqtt.homeassistant.internal.config.dto.AbstractChannelConfiguration;
import org.openhab.binding.mqtt.homeassistant.internal.exception.ConfigurationException;

Expand All @@ -27,7 +31,9 @@
*/
@NonNullByDefault
public class Lock extends AbstractComponent<Lock.ChannelConfiguration> {
public static final String SWITCH_CHANNEL_ID = "lock"; // Randomly chosen channel "ID"
public static final String LOCK_CHANNEL_ID = "lock";
public static final String STATE_CHANNEL_ID = "state";
public static final String OPEN_CHANNEL_ID = "open";

/**
* Configuration class for MQTT component
Expand All @@ -39,14 +45,26 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {

protected boolean optimistic = false;

@SerializedName("command_topic")
protected @Nullable String commandTopic;
@SerializedName("state_topic")
protected String stateTopic = "";
@SerializedName("payload_lock")
protected String payloadLock = "LOCK";
@SerializedName("payload_unlock")
protected String payloadUnlock = "UNLOCK";
@SerializedName("command_topic")
protected @Nullable String commandTopic;
@SerializedName("payload_open")
protected String payloadOpen = "OPEN";
@SerializedName("state_jammed")
protected String stateJammed = "JAMMED";
@SerializedName("state_locked")
protected String stateLocked = "LOCKED";
@SerializedName("state_locking")
protected String stateLocking = "LOCKING";
@SerializedName("state_unlocked")
protected String stateUnlocked = "UNLOCKED";
@SerializedName("state_unlocking")
protected String stateUnlocking = "UNLOCKING";
}

public Lock(ComponentFactory.ComponentConfiguration componentConfiguration) {
Expand All @@ -57,12 +75,35 @@ public Lock(ComponentFactory.ComponentConfiguration componentConfiguration) {
throw new ConfigurationException("Component:Lock does not support forced optimistic mode");
}

buildChannel(SWITCH_CHANNEL_ID,
new OnOffValue(channelConfiguration.payloadLock, channelConfiguration.payloadUnlock), getName(),
componentConfiguration.getUpdateListener())
String stateTopic = channelConfiguration.stateTopic;

// State can indicate additional information than just
// locked/unlocked, so expose it as a separate channel
if (stateTopic != null) {
Set<String> states = new HashSet<>();
states.add(channelConfiguration.stateJammed);
states.add(channelConfiguration.stateLocked);
states.add(channelConfiguration.stateLocking);
states.add(channelConfiguration.stateUnlocked);
states.add(channelConfiguration.stateUnlocking);

TextValue value = new TextValue(states);
buildChannel(STATE_CHANNEL_ID, value, "State", componentConfiguration.getUpdateListener())
.stateTopic(stateTopic).isAdvanced(true).build();
}

buildChannel(LOCK_CHANNEL_ID,
new OnOffValue(channelConfiguration.stateLocked, channelConfiguration.stateUnlocked,
channelConfiguration.payloadLock, channelConfiguration.payloadUnlock),
"Lock", componentConfiguration.getUpdateListener())
.stateTopic(channelConfiguration.stateTopic, channelConfiguration.getValueTemplate())
.commandTopic(channelConfiguration.commandTopic, channelConfiguration.isRetain(),
channelConfiguration.getQos())
.build();
buildChannel(OPEN_CHANNEL_ID, new OnOffValue(channelConfiguration.payloadOpen), "Open",
componentConfiguration.getUpdateListener())
.commandTopic(channelConfiguration.commandTopic, channelConfiguration.isRetain(),
channelConfiguration.getQos())
.build();
}
}
23 changes: 15 additions & 8 deletions ...nt/src/test/java/org/openhab/binding/mqtt/homeassistant/internal/component/LockTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.junit.jupiter.api.Test;
import org.openhab.binding.mqtt.generic.values.OnOffValue;
import org.openhab.binding.mqtt.generic.values.TextValue;
import org.openhab.core.library.types.OnOffType;

/**
Expand Down Expand Up @@ -55,31 +56,37 @@ public void test() throws InterruptedException {
"name": "lock", \
"payload_unlock": "UNLOCK_", \
"payload_lock": "LOCK_", \
"state_unlocked": "UNLOCK_", \
"state_locked": "LOCK_", \
"state_topic": "zigbee2mqtt/lock/state", \
"command_topic": "zigbee2mqtt/lock/set/state" \
}\
""");
// @formatter:on

assertThat(component.channels.size(), is(1));
assertThat(component.channels.size(), is(3));
assertThat(component.getName(), is("lock"));

assertChannel(component, Lock.SWITCH_CHANNEL_ID, "zigbee2mqtt/lock/state", "zigbee2mqtt/lock/set/state", "lock",
assertChannel(component, Lock.LOCK_CHANNEL_ID, "zigbee2mqtt/lock/state", "zigbee2mqtt/lock/set/state", "Lock",
OnOffValue.class);
assertChannel(component, Lock.STATE_CHANNEL_ID, "zigbee2mqtt/lock/state", "", "State", TextValue.class);
assertChannel(component, Lock.OPEN_CHANNEL_ID, "", "zigbee2mqtt/lock/set/state", "Open", OnOffValue.class);

publishMessage("zigbee2mqtt/lock/state", "LOCK_");
assertState(component, Lock.SWITCH_CHANNEL_ID, OnOffType.ON);
assertState(component, Lock.LOCK_CHANNEL_ID, OnOffType.ON);
publishMessage("zigbee2mqtt/lock/state", "LOCK_");
assertState(component, Lock.SWITCH_CHANNEL_ID, OnOffType.ON);
assertState(component, Lock.LOCK_CHANNEL_ID, OnOffType.ON);
publishMessage("zigbee2mqtt/lock/state", "UNLOCK_");
assertState(component, Lock.SWITCH_CHANNEL_ID, OnOffType.OFF);
assertState(component, Lock.LOCK_CHANNEL_ID, OnOffType.OFF);
publishMessage("zigbee2mqtt/lock/state", "LOCK_");
assertState(component, Lock.SWITCH_CHANNEL_ID, OnOffType.ON);
assertState(component, Lock.LOCK_CHANNEL_ID, OnOffType.ON);

component.getChannel(Lock.SWITCH_CHANNEL_ID).getState().publishValue(OnOffType.OFF);
component.getChannel(Lock.LOCK_CHANNEL_ID).getState().publishValue(OnOffType.OFF);
assertPublished("zigbee2mqtt/lock/set/state", "UNLOCK_");
component.getChannel(Lock.SWITCH_CHANNEL_ID).getState().publishValue(OnOffType.ON);
component.getChannel(Lock.LOCK_CHANNEL_ID).getState().publishValue(OnOffType.ON);
assertPublished("zigbee2mqtt/lock/set/state", "LOCK_");
component.getChannel(Lock.OPEN_CHANNEL_ID).getState().publishValue(OnOffType.ON);
assertPublished("zigbee2mqtt/lock/set/state", "OPEN");
}

@Test
Expand Down

3 comments on commit a76400e

@jshatch
Copy link

Choose a reason for hiding this comment

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

You mentioned that this release has the code change that permits null names, however I am actually still getting the errors I told you about. Maybe that PR doesn't handle this particular case?

23:47:38.107 [WARN ] [assistant.internal.DiscoverComponents] - HomeAssistant discover error: Label for a ChannelType must not be empty.
23:47:38.108 [WARN ] [assistant.internal.DiscoverComponents] - HomeAssistant discover error: Label for a ChannelType must not be empty.
java.lang.IllegalArgumentException: Label for a ChannelGroupType must not be empty.
        at org.openhab.core.thing.type.ChannelGroupTypeBuilder.instance(ChannelGroupTypeBuilder.java:45) ~[?:?]
        at org.openhab.binding.mqtt.homeassistant.internal.component.AbstractComponent.getType(AbstractComponent.java:253) ~[?:?]
        at org.openhab.binding.mqtt.homeassistant.internal.component.AbstractComponent.addChannelTypes(AbstractComponent.java:161) ~[?:?]
        at org.openhab.binding.mqtt.homeassistant.internal.handler.HomeAssistantThingHandler.accept(HomeAssistantThingHandler.java:292) ~[?:?]
        at org.openhab.binding.mqtt.homeassistant.internal.handler.HomeAssistantThingHandler.accept(HomeAssistantThingHandler.java:1) ~[?:?]
        at org.openhab.binding.mqtt.generic.tools.DelayedBatchProcessing.run(DelayedBatchProcessing.java:110) ~[?:?]
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) ~[?:?]
        at java.util.concurrent.FutureTask.run(FutureTask.java:264) ~[?:?]
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) ~[?:?]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
        at java.lang.Thread.run(Thread.java:833) [?:?]

I think this is what I'm running into... https://rc.home-assistant.io/integrations/mqtt#naming-of-mqtt-entities but I could be wrong.
FWIW I'm getting this error from the ring-mqtt application as of v5.6.0.

If this is outside of your PR then don't worry about it, for now I'm going to downgrade back to v5.5.1 of ring-mqtt.

@ccutrer
Copy link
Owner Author

Choose a reason for hiding this comment

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

It should be working. I'm running both zigbee2mqtt and ESPHome with this update, but it's possible ring-mqtt is doing something different. If you can dump all of the discovery topics for that device and send them to me, I can try to reproduce. A private Gist is fine.

@jshatch
Copy link

Choose a reason for hiding this comment

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

Gist sent separately.

Please sign in to comment.