stjohnjohnson / smartthings-mqtt-bridge

Bridge between SmartThings and MQTT
https://hub.docker.com/r/stjohnjohnson/smartthings-mqtt-bridge/
MIT License
363 stars 242 forks source link

Device Network ID collision with another DTH #199

Open jezzaaa opened 5 years ago

jezzaaa commented 5 years ago

I struggled for days to find out why nothing was coming out of my SmartThings hub into the Bridge. Finally I noticed an error in the live logging that referred to a Device Network ID (DNID), hinting that it wasn't unique.

Turns out both cast-web-api and smartthings-mqtt-bridge both use the MAC address of where they run, as the DNID. As I had installed cast-web-api first, I suppose it got all the messages destined for smartthings-mqtt-bridge.

I hacked the DTH to prefix the MAC with "MQTT", to make it more likely to be unique. Immediately, I started getting MQTT messages through.

Now I just have to work out why I can't get any traffic going the other way...

jmbinette commented 5 years ago

@jezzaaa Di you find a way to solve this ?

I have exactly the same problem and running both in Docker

jezzaaa commented 5 years ago

Yes, I hacked the DTH code to prefix the MAC with "MQTT", to make it more likely to be unique.

But I gave up on trying to use MQTT.

jmbinette commented 5 years ago

MQTT working for me, let me know if I can help

Could you point me the exact places you had to modify DTH?

I thought it was the way ST hub was communicating, not related to code : "Yes, the SmartThings local incoming calls work by whitelisting by MAC address. And outgoing calls work by IP"

jezzaaa commented 5 years ago

I can do most of what I want without MQTT, so there isn't an urgent need. But it'd be nice - when I get a bit of time.

My diff looks like the below:

<     // Setting Network Device Id
<     def hex = "$settings.mac".toUpperCase().replaceAll(':', '')
<     if (device.deviceNetworkId != "$hex") {
<         device.deviceNetworkId = "$hex"
<         log.debug "Device Network Id set to ${device.deviceNetworkId}"
---
>     def hex = ""
>     if (device.deviceNetworkId == NULL || device.deviceNetworkId == "") {
>         // Setting Network Device Id
>         //hex = "MQTT"+$settings.mac.toUpperCase().replaceAll(':', '')
>         hex = ipToHex(settings.ip+":"+settings.port)
>         log.debug "debug:"+hex
>         if (device.deviceNetworkId != "$hex") {
>             device.deviceNetworkId = "$hex"
>             log.debug "Device Network Id set to $hex -> ${device.deviceNetworkId}"
>         }

I originally just prefixed the right-hand-side of the assignment to hex with the string "MQTT". This is now commented out and replaced - not because it didn't work but because it didn't make sense to me to use the MAC address at all.

The MAC address is unique to the device, but not to the services running on the device. Instead, it makes sense to use something that's unique to the service (and doesn't have to be unique to the site because it will always operate in the local network).

So the new code gets the IP address and port number of the listening socket, converts them to hex and joints them together with a colon. In this way, a different service listening on a different port will have a different device ID even when it's running on the same device (thus same MAC).

The call to ipToHex() refers to a function I added, but didn't include in the above diff, for clarity. It looks like this:

// accepts a IP:PORT string and converts it to a hex identifier
// for setting the SmartThings deviceNetworkId
// NOTE: does not handle octet less than 0x10 = 16!
private String ipToHex(String ip) {
  def parts   = ip.tokenize(':')
  def address = parts[0]
  def port            = parts[1]
  def octets  = address.tokenize('.')
  def hex             = ""

  log.debug "converting ipToHex"
  octets.each {
    hex = hex + Integer.toHexString(it as Integer).toUpperCase()
  }
  hex = hex + ":" + Integer.toHexString(port as Integer).toUpperCase().padLeft(4,'0')
  return hex
}
jmbinette commented 5 years ago

Great ! Thank you I will def try that !

@stjohnjohnson could we integrate this kind of mod ? I think it would help many people