[mqtt] Support multiple, comma-separated values for on/off param of switch/contact channels #17242

florian-h05 · 2024-08-13T10:21:47Z

This allows to map multiple string values to the on or the off state.

… on/off param of switch/contact channels Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

lsiepel · 2024-08-13T11:00:15Z

Would it make sense to make use of openhab/openhab-core#4330 look at the related HomeKit PR for example configuration?

florian-h05 · 2024-08-13T11:12:53Z

Hmm I think the core PR is related to extending the syntax of the .items file to allow definint these lists in item metadata.
This PR is about defining these lists not at Item level, but instead at Thing (binding) level.

I would like to map the contact channel's OPEN to multiple states, this is only possible at a Thing level. The other option would be to have a string channel and transform its value at channel link level, but this is more complicated to set up.

ccutrer · 2024-08-13T14:46:55Z

Things (and specifically channels) already support multiple values for their config in thing files (https://github.com/ccutrer/openhab-core/blob/c3fdd86428939a0a5b01816cfbfd84ec0a86b6df/bundles/org.openhab.core.model.thing/src/org/openhab/core/model/thing/Thing.xtext#L67) - it's actually what I based the Item metadata PR on. That said, the binding still needs to handle a List for it. And the UI definitely does not support a friendly way to enter native lists. I'd come asking you, @florian-h05, about that! I'd still much prefer if we could do it that way, though. While it may seem unusual, I could see a comma being a legitimate value (perhaps it's 100,0 (as in 100%, with a European-style decimal point). A pipe (|) seems less likely, but if we can get some nice UI support for lists, then we wouldn't have to roll the dice that no one uses that in a legitimate value.

jimtng · 2024-08-20T12:01:24Z

@florian-h05 I just had to change the config parameter type from String to List<String> in java, and added multiple="true" in the channel config xml. Then it will accept an array / list in .things file, and in the UI. See this, those two are the only things I changed, because the implementation class already supports
Lists (which may not be the case here). https://github.com/openhab/openhab-addons/pull/17288/files#diff-1a86ba1ab3ca46c5eb34575ba95d85c0b87453b4bfd361711829b783c557cf44

florian-h05 · 2024-08-20T13:36:12Z

@jimtng Thanks for the hint.
I have refactored this PR to use this, would be great if you and @ccutrer could have a look at it.

To-Do: Update README, regenerate default translations.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

ccutrer · 2024-08-20T14:16:39Z

...nding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/values/OpenCloseValue.java

-    private final String closeString;
+    private final Set<String> openStates;
+    private final Set<String> closedStates;
+    private final String openCommand;


Is it possible to somehow send a command to an OpenCloseValue? You didn't introduce that in this PR, but you definitely extend it. Assuming it's not possible, I'd rather just remove it (possibly throwing an exception in getMQTTpublishValue and parseCommand, and renaming the current parseCommand to parseMessage - though if we go that way, you need to make sure to maintain the UnDefType condition from the parent class)

Normally it's not possible to send commands to Contact Items, so I guess it's unlikely a command is sent here, however I am not sure if this code is possibly needed to "publish" a contact Item state to MQTT.

ccutrer · 2024-08-20T14:19:40Z

...binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/values/ValueFactory.java

                break;
            case MqttBindingConstants.CONTACT:
-                value = new OpenCloseValue(config.on, config.off);
+                value = new OpenCloseValue(config.onState == null && on != null ? List.of(on) : config.onState,
+                        config.offState == null && off != null ? List.of(off) : config.offState);
                break;
            case MqttBindingConstants.ROLLERSHUTTER:
                value = new RollershutterValue(config.on, config.off, config.stop, config.onState, config.offState,


do you not also need to handle multiple values for Rollershutter here?

This is implemented inside RollershutterValue: It gets the List<String> onState and offState passed.

ccutrer · 2024-08-20T14:22:05Z

.../org.openhab.binding.mqtt.generic/src/main/resources/OH-INF/config/switch-channel-config.xml

+				off/closed
+				state. You can use this parameter for additional keywords, next to OFF (CLOSED respectively on a Contact).</description>
+		</parameter>
+		<parameter name="on" type="text" advanced="true">


why rename the parameter? I know it makes it somewhat clearer, but it also means people will need to update their configuration at some point. my understanding from @jimtng's investigations are that you can add multiple="true" to an existing parameter, and then the core framework will automatically handle always presenting it to the binding as a list, regardless of if it's actually a list in the persisted configuration (either json or .things file)

The problem is, that I would need to change the type of the on and off params in the ChannelConfig class from String to List<String>, which would require changes to several types, whilst changes to the onState and offState param require less changes.

This makes it really difficult IMO to implement the changes I want to do in a "clean" way - FYI I have already changed my approach and actually don't need the changes from this PR at all. Just use a SCRIPT transformation on the Item channel link and you can do whatever you want ...

FYI I have already changed my approach and actually don't need the changes from this PR at all.

What's your plan for this PR? continue or drop ?

I would tend to drop, as it is just way easier to evaluate the string with a script transformation ...
I will close this PR then.

ccutrer · 2024-08-20T14:23:31Z

...b.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/values/OnOffValue.java

     */
-    public OnOffValue(@Nullable String onValue, @Nullable String offValue) {


maintaining this constructor as a convenience seems quite useful for uses besides the mqtt.generic binding (i.e. from within homie and homeassistant, and tests in general)

jimtng · 2024-08-20T14:21:53Z

...b.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/values/OnOffValue.java

+                : onStates.stream().filter(not(String::isBlank)).collect(Collectors.toSet());
+        this.offStates = offStates == null ? Set.of(OnOffType.ON.name())
+                : offStates.stream().filter(not(String::isBlank)).collect(Collectors.toSet());
+        this.onCommand = onCommand == null ? (onStates == null ? OnOffType.ON.name() : onStates.get(0)) : onCommand;


I'm wondering whether onStates could be not null, but having size() == 0, in which case, get(0) will cause index out of bounds.

Could be possible, true.

jimtng · 2024-08-20T14:33:39Z

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

@@ -128,13 +128,13 @@ You can connect this channel to a Rollershutter or Dimmer item.
 - **on**: An optional number (like 1, 10) or a string (like "ON"/"Open") that is recognized as on/open state.
 - **off**: An optional number (like 0, -10) or a string (like "OFF"/"Close") that is recognized as off/closed state.

+You can provide multiple values for **on** and **off** separated by a comma. The first value is used for publishing to MQTT.


this needs to be updated

openhab-bot · 2024-09-13T10:38:51Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/custom-off-closed-value-multiple-values/158348/9

[mqtt] Add support for providing multiple, comma-separated values for…

20fa9fe

… on/off param of switch/contact channels Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 requested review from antroids and ccutrer as code owners August 13, 2024 10:21

jlaur added the enhancement An enhancement or new feature for an existing add-on label Aug 13, 2024

florian-h05 marked this pull request as draft August 15, 2024 11:29

[mqtt] Refactor to use multiple="true"

0e5c30b

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 force-pushed the mqtt-multiple-values-onoff branch from ec04c8a to 0e5c30b Compare August 20, 2024 13:45

ccutrer reviewed Aug 20, 2024

View reviewed changes

jimtng reviewed Aug 20, 2024

View reviewed changes

florian-h05 closed this Sep 16, 2024

florian-h05 deleted the mqtt-multiple-values-onoff branch October 23, 2024 21:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[mqtt] Support multiple, comma-separated values for on/off param of switch/contact channels #17242

[mqtt] Support multiple, comma-separated values for on/off param of switch/contact channels #17242

florian-h05 commented Aug 13, 2024

lsiepel commented Aug 13, 2024

florian-h05 commented Aug 13, 2024

ccutrer commented Aug 13, 2024

jimtng commented Aug 20, 2024

florian-h05 commented Aug 20, 2024 •

edited

Loading

ccutrer Aug 20, 2024

florian-h05 Aug 20, 2024

ccutrer Aug 20, 2024

florian-h05 Aug 20, 2024

ccutrer Aug 20, 2024

florian-h05 Aug 20, 2024

florian-h05 Aug 20, 2024

jimtng Sep 15, 2024

florian-h05 Sep 16, 2024

ccutrer Aug 20, 2024

jimtng Aug 20, 2024

florian-h05 Aug 20, 2024

jimtng Aug 20, 2024

openhab-bot commented Sep 13, 2024

		*/
		public OnOffValue(@Nullable String onValue, @Nullable String offValue) {

[mqtt] Support multiple, comma-separated values for on/off param of switch/contact channels #17242

[mqtt] Support multiple, comma-separated values for on/off param of switch/contact channels #17242

Conversation

florian-h05 commented Aug 13, 2024

lsiepel commented Aug 13, 2024

florian-h05 commented Aug 13, 2024

ccutrer commented Aug 13, 2024

jimtng commented Aug 20, 2024

florian-h05 commented Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openhab-bot commented Sep 13, 2024

florian-h05 commented Aug 20, 2024 •

edited

Loading