TheAgentK / tuya-mqtt

Nodejs-Script to combine tuyaapi and openhab via mqtt
MIT License
173 stars 80 forks source link

Use MQTT message payload rather than topic to set switch state #7

Closed tsightler closed 5 years ago

tsightler commented 5 years ago

Is your feature request related to a problem? Please describe. The 2.4 OpenHAB MQTT bindings have entries for only a single state topic, and a single command topic. This makes sense as it's normal for the command channel to act on the message payload rather than on the topic itself. The current tuya-mqtt is not easily usable with the 2.4 since it requires two command topics and completely ignores the message payload.

Describe the solution you'd like Instead of using separate topics for on/off, use a single topic and act on the message payload. For the case of example, if using the mosquitto_pub cmdline tool today I have to run: mosquitto_pub -t '<tuyaAPI-type>/<tuyaAPI-id>/<tuyaAPI-key>/<tuyaAPI-ip>/command/on' -m '' mosquitto_pub -t '<tuyaAPI-type>/<tuyaAPI-id>/<tuyaAPI-key>/<tuyaAPI-ip>/command/off' -m ''

I can literally put anything I want in the message field but it ignores it, only the topic matters. I'm an MQTT newbie, but this seems unusual and not how other devices work. Most have a command topic, and you can publish any number of commands there via the message like this:

mosquitto_pub -t '<tuyaAPI-type>/<tuyaAPI-id>/<tuyaAPI-key>/<tuyaAPI-ip>/command' -m 'on' mosquitto_pub -t '<tuyaAPI-type>/<tuyaAPI-id>/<tuyaAPI-key>/<tuyaAPI-ip>/command' -m 'off'

This is what the OpenHAB 2.4 MQTT bindings expect.

I have tested the following patch, which maintains the existing behavior, so it shouldn't break existing bindings, but, if you including only the /command topic, it instead acts on the message. Simple replace:

        if (exec == "command") {
            var status = topic[6];
            device.onoff(status);
        }

With the following:

        if (exec == "command") {
            var status = topic[6];
            if ( status == null ) {
                device.onoff(message);
            } else {
                device.onoff(status);
            }
        }

I've tested this small change with both the only 1.X bindings configured as they worked previously, and the new 2.4 using a single command topic and both seem to work properly.

Describe alternatives you've considered I could keep using the 1.X bindings, which worked fine, but can't be configured via Paper UI (not a huge deal). With the 2.4 bindings I had bound a status only thing (left command topic blank) and used a rule which directly called publishMQTT to the different topics based on the received ON/OFF command. This also worked fine but required a rule for every device, which seemed ugly and also couldn't be configured via the Paper UI. With the fix above, just add an MQTT Generic Thing with the proper state and command topics, link it to an item, and you're done.

TheAgentK commented 5 years ago

Great solution! I had already seen the problem, but no time to fix it.

If you want to create a pull request for it, I would be very grateful. Otherwise it can take a while until I integrate it, unfortunately I don't have time for this project at the moment.

TheAgentK commented 5 years ago

fixed