Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mqtt.homeassistant] Crash when discovery topic updates with slightly different payload #13517

Closed
ssalonen opened this issue Oct 8, 2022 · 0 comments · Fixed by #13518
Closed
Labels
bug An unexpected problem or unintended behavior of an add-on

Comments

@ssalonen
Copy link
Contributor

ssalonen commented Oct 8, 2022

Similar to #9711 #12295, perhaps same root cause.

Expected Behavior

No errors in logs

Current Behavior

When discovery topic updates with slightly different JSON payload, the existing homeassistant thing crashes.

openhab.log uncatched error, potentially with bad side-effects

java.lang.IllegalArgumentException: Duplicate channels mqtt:homeassistant_REDACTED:MQTTBroker:Ruuvi_REDACTED:REDACTED_2Dmov#sensor
        at org.openhab.core.thing.util.ThingHelper.ensureUniqueChannels(ThingHelper.java:152) ~[?:?]
        at org.openhab.core.thing.util.ThingHelper.ensureUniqueChannels(ThingHelper.java:144) ~[?:?]
        at org.openhab.core.thing.util.ThingHelper.addChannelsToThing(ThingHelper.java:105) ~[?:?]
        at org.openhab.binding.mqtt.homeassistant.internal.handler.HomeAssistantThingHandler.accept(HomeAssistantThingHandler.java:292) ~[?:?]

Potential side effect is unsubscribe of channels

This is done at

if (discovered.getConfigHash() != known.getConfigHash()) {
// Don't wait for the future to complete. We are also not interested in failures.
// The component will be replaced in a moment.
known.stop();

/**
* Unsubscribes from all state channels of the component.
*
* @return A future that completes as soon as all subscriptions removals have been performed. Completes
* exceptionally on errors.
*/
public CompletableFuture<@Nullable Void> stop() {
return channels.values().parallelStream().map(ComponentChannel::stop).collect(FutureCollector.allOf());
}

since the crash happens after that, at

And indeed,

Possible Solution

PR to follow

Steps to Reproduce (for Bugs)

  1. Set up bluetooth sensor, e.g. ruuvi tag, https://ruuvi.com/
  2. Set up two instances of OpenMQTTGateway, listening in bluetooth advertisements and publishing data to MQTT. Have different gateway names for the Gateways.
  3. Reboot the first OpenMQTTGateway , then the second
  4. Observe errors in the logs

openhab.log error

java.lang.IllegalArgumentException: Duplicate channels mqtt:homeassistant_SNIP
        at org.openhab.core.thing.util.ThingHelper.ensureUniqueChannels(ThingHelper.java:152) ~[?:?]
        at org.openhab.core.thing.util.ThingHelper.ensureUniqueChannels(ThingHelper.java:144) ~[?:?]
        at org.openhab.core.thing.util.ThingHelper.addChannelsToThing(ThingHelper.java:105) ~[?:?]

OpenMQTTGateway publishes discovery information similar to below

{
  "stat_t": "+/+/BTtoMQTT/REDACTED",
  "name": "RuuviTag_RAWv2-mov",
  "uniq_id": "REDACTED-mov",
  "val_tpl": "{{ value_json.mov}}",
  "state_class": "measurement",
  "device": {
    "identifiers": [
      "REDACTED"
    ],
    "connections": [
      [
        "mac",
        "REDACTED"
      ]
    ],
    "manufacturer": "Ruuvi",
    "model": "RuuviTag_RAWv2",
    "name": "RuuviTag",
    "via_device": "OpenMQTTGateway_1"
  }
}

NOTE: Depending which sensor was restarted last, the via_device contains different value. This field is not even affecting discovery process, it is purely additional metadata!

This change of value causes the binding to crash.

Context

I am listening bluetooth sensors using multiple ESP32's (with OpenMQTTGateway) around my house. Some bluetooth sensors are in the coverage of two ESP32s.

This setup is causing error on openHAB side.

Your Environment

openHAB 3.3.0 stable release

@ssalonen ssalonen added the bug An unexpected problem or unintended behavior of an add-on label Oct 8, 2022
ssalonen added a commit to ssalonen/openhab2-addons that referenced this issue Oct 8, 2022
Fixes openhab#13517
Possibly resolves openhab#9711 and openhab#12295 as well.

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
@ssalonen ssalonen changed the title [mqtt.homeassistant] Crash when discovery topic updates [mqtt.homeassistant] Crash when discovery topic updates with slightly different payload Oct 8, 2022
lolodomo pushed a commit that referenced this issue Nov 12, 2022
…topics update with content (#13518)

* [mqtt.homeassistant] Fix for discovery topics that update with content

Fixes #13517
Possibly resolves #9711 and #12295 as well.

* [mqtt.homeassistant] Sort channels before changing thing
* [mqtt.homeassistant] logging + removed unnecessary synchronization
* Resolve bunch of warnings in homeassistant bundle
* [mqtt.homeassistant] Handling null warnings and unnecessary null checks
* [mqtt.homeassistant] Removing unnecessary null checks

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
Co-Authored-by: @antroids github handle
andrasU pushed a commit to andrasU/openhab-addons that referenced this issue Dec 24, 2022
…topics update with content (openhab#13518)

* [mqtt.homeassistant] Fix for discovery topics that update with content

Fixes openhab#13517
Possibly resolves openhab#9711 and openhab#12295 as well.

* [mqtt.homeassistant] Sort channels before changing thing
* [mqtt.homeassistant] logging + removed unnecessary synchronization
* Resolve bunch of warnings in homeassistant bundle
* [mqtt.homeassistant] Handling null warnings and unnecessary null checks
* [mqtt.homeassistant] Removing unnecessary null checks

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
Co-Authored-by: @antroids github handle
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
borazslo pushed a commit to borazslo/openhab-mideaac-addon that referenced this issue Jan 8, 2023
…topics update with content (openhab#13518)

* [mqtt.homeassistant] Fix for discovery topics that update with content

Fixes openhab#13517
Possibly resolves openhab#9711 and openhab#12295 as well.

* [mqtt.homeassistant] Sort channels before changing thing
* [mqtt.homeassistant] logging + removed unnecessary synchronization
* Resolve bunch of warnings in homeassistant bundle
* [mqtt.homeassistant] Handling null warnings and unnecessary null checks
* [mqtt.homeassistant] Removing unnecessary null checks

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
Co-Authored-by: @antroids github handle
psmedley pushed a commit to psmedley/openhab-addons that referenced this issue Feb 23, 2023
…topics update with content (openhab#13518)

* [mqtt.homeassistant] Fix for discovery topics that update with content

Fixes openhab#13517
Possibly resolves openhab#9711 and openhab#12295 as well.

* [mqtt.homeassistant] Sort channels before changing thing
* [mqtt.homeassistant] logging + removed unnecessary synchronization
* Resolve bunch of warnings in homeassistant bundle
* [mqtt.homeassistant] Handling null warnings and unnecessary null checks
* [mqtt.homeassistant] Removing unnecessary null checks

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
Co-Authored-by: @antroids github handle
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this issue Feb 28, 2023
…topics update with content (openhab#13518)

* [mqtt.homeassistant] Fix for discovery topics that update with content

Fixes openhab#13517
Possibly resolves openhab#9711 and openhab#12295 as well.

* [mqtt.homeassistant] Sort channels before changing thing
* [mqtt.homeassistant] logging + removed unnecessary synchronization
* Resolve bunch of warnings in homeassistant bundle
* [mqtt.homeassistant] Handling null warnings and unnecessary null checks
* [mqtt.homeassistant] Removing unnecessary null checks

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
Co-Authored-by: @antroids github handle
andrasU pushed a commit to andrasU/openhab-addons that referenced this issue Jan 6, 2024
…topics update with content (openhab#13518)

* [mqtt.homeassistant] Fix for discovery topics that update with content

Fixes openhab#13517
Possibly resolves openhab#9711 and openhab#12295 as well.

* [mqtt.homeassistant] Sort channels before changing thing
* [mqtt.homeassistant] logging + removed unnecessary synchronization
* Resolve bunch of warnings in homeassistant bundle
* [mqtt.homeassistant] Handling null warnings and unnecessary null checks
* [mqtt.homeassistant] Removing unnecessary null checks

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
Co-Authored-by: @antroids github handle
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
1 participant