arachnetech / homebridge-mqttthing

A plugin for Homebridge allowing the integration of many different accessory types using MQTT.
Apache License 2.0
466 stars 104 forks source link

any plans to bump mqtt npm dependency? #437

Open bahamas10 opened 3 years ago

bahamas10 commented 3 years ago

hello! I'm very interested in getting this plugin setup with my zwavejs2mqtt setup. I tried setting this up but noticed, while it looked like everything was working, mqttthing wouldn't connect to my mqtt broker at all (mosquitto v2.0.10 running behind a TLS proxy).

I did some debugging - i pulled in the same version of mqtt that this plugin used and attempted to manually invoke the sub.js file that accompanies that repo:

Install:

git clone git@github.com:mqttjs/MQTT.js.git
cd MQTT.js
git checkout v2.18.8
npm i
cd bin

Test:

./sub.js -v -h mqtts://my-internal-host -p 8883 -t 'zwavejs2mqtt/#' -u zwavejs2mqtt -P foo

I then manually published some messages to that topic and saw that none of them were seen by ./sub.js. In fact, I could see from my proxy logs and the mosquitto logs that no connection was ever made. No error was ever seen, the ./sub.js command just seemed to hand indefinitely.

I then pulled the next major release - v3.0.0.

git checkout v3.0.0
npm i

Then test again:

./sub.js -v -h mqtts://my-internal-host -p 8883 -t 'zwavejs2mqtt/#' -u zwavejs2mqtt -P foo

Same issue - no messages were ever seen.

Finally, i decided to pull the latest master code:

git checkout master
npm i

And then test again. Note that the usage of ./sub.js has changed with this version. The protocol is now specified as -l and is omitted from the -h argument:

./sub.js -v -h my-internal-host -p 8883 -t 'zwavejs2mqtt/#' -u zwavejs2mqtt -P foo -l mqtts
zwavejs2mqtt/_CLIENTS/ZWAVE_GATEWAY-zwavejs2mqtt/version {"value":"3.5.0","time":1620070610616}
zwavejs2mqtt/_CLIENTS/ZWAVE_GATEWAY-zwavejs2mqtt/status {"value":true,"time":1620070610616}
zwavejs2mqtt/Laundry_Room/Temp_Outdoor_Switch/37/0/currentValue false

The latest version (v4.x.x) seems to be the only one that works for me. I'm not sure exactly why this is.

Note that I tested this TWICE: once with node v14.16.1 and again with node v12.21.0 - both yielding the same results.


So my question is - is upgrading to a later version of mqtt possible for this plugin? I'll take a look into doing this myself manually to test, but I figured it would be best to write down and document everything I have seen while doing this.

bahamas10 commented 3 years ago

I've dug deeper into this issue. This appears to be due to my TLS proxy requiring SNI support, which was added to MQTT.js in this commit: f6534c2d8348afadc91c4d6c636447430be4642b

I manually got this to work with this one-line change:

diff --git a/lib/connect/tls.js b/lib/connect/tls.js
index eda78be..419cedd 100644
--- a/lib/connect/tls.js
+++ b/lib/connect/tls.js
@@ -5,6 +5,7 @@ function buildBuilder (mqttClient, opts) {
   var connection
   opts.port = opts.port || 8883
   opts.host = opts.hostname || opts.host || 'localhost'
+  opts.servername = opts.host

   opts.rejectUnauthorized = opts.rejectUnauthorized !== false

In /opt/local/lib/node_modules/homebridge-mqttthing/node_modules/mqtt/lib/connect/tls.js


Though this fixes my issue manually, I will leave this open as upgrading the mqtt dep may still be beneficial.