Bug in logic in check for duplication of discovery topics

[pbyrne-ms]

Hey Steve, Hope all is well! I upgraded to the latest version of your code today, and have found a little bug I think. I don't fully understand it, but I have managed to kludge a fix for it. I'm unsure if I'm fixing it correctly though, so I won't do a PR..., maybe you can look into it when you have time.

It's in "MQTT send receive", in the function "client.ON_MESSAGE = function(mid, topic, payload)"

This is how i found/fixed it:

client.ON_MESSAGE = function(mid, topic, payload)
  -- Record discovery topics to check for duplication
  local parts = string.split(topic, '/')
  log(parts) -- added by me
  if parts[1] == string.split(mqttDiscoveryTopic, '/')[1] then
    if discovery[parts[3]] ~= nil and discovery[parts[3]] ~= parts[2] then
      discoveryDelete[parts[3]..'/'..parts[2]] = true
    else
      -- check for nil
      log('Part 3 is : ' .. tostring(parts[3]))  -- added by me
      log('Part 2 is : ' .. tostring(parts[2])) -- added by me
        if discovery[parts[3]] ~= nil then -- added by me
            discovery[parts[3]] = parts[2] -- Dictionary of CBus addresses with type as the value
        end -- added by me
    end
  else
    mqttMessages[#mqttMessages + 1] = { topic=topic, payload=payload } -- Queue the MQTT message
  end
end

Sometimes Part 3 can be nil, as homessistant publishes a message on this topic as only "homeassistant/status" and the logic errors out when trying to access the third part as it's not there... I don't really understand why the logic in one of the earlier "IFs" doesn't catch this... something to do with the order or precedence of lua maybe..

I kludged it by checking it again for nil in the ELSE block...and it does work and allow the code to run.... but I suspect i'm missing the bigger picture, I might be breaking something else you intended...

Paddy

[autoSteve]

I see the issue. Weird it doesn't fail too on my install, as the original logic is flawed.

What this code does is to ensure that old discovery topics get cleaned up should the type change (e.g. "light" changes to a "switch").

Try this:

client.ON_MESSAGE = function(mid, topic, payload)
  -- Record discovery topics to check for duplication
  local parts = string.split(topic, '/')
  if parts[1] == string.split(mqttDiscoveryTopic, '/')[1] then
    if discovery[parts[3]] ~= nil then
      if discovery[parts[3]] ~= parts[2] then
        discoveryDelete[parts[3]..'/'..parts[2]] = true
      else
        discovery[parts[3]] = parts[2] -- Dictionary of CBus addresses with type as the value
      end
    end
  else
    mqttMessages[#mqttMessages + 1] = { topic=topic, payload=payload } -- Queue the MQTT message
  end
end

[pbyrne-ms]

thanks for the explanation, makes sense to me now! That code snippet works perfectly now, thanks