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.generic] Clear availability subscriptions on stop #8317

Merged
merged 2 commits into from Aug 27, 2020

Conversation

bodiroga
Copy link

@bodiroga bodiroga commented Aug 19, 2020

When Home Assistant and MQTT generic things were disposed (and stopped), parent class (AbstractMQTTThingHandler) stop() method was not called and, thus, availability subscriptions were not removed. This made the Thing stay offline in some cases, as described in #8316.

The AbstractMQTTThingHandler's stop() method has also been simplified calling clearAllAvailabilityTopics();. @jochen314, what do you think about this last change? I don't see any problem emptying the availabilityStates map of AbstractMQTTThingHandler, because the initialize() method of both GenericThingHandler and HomeAssistantThingHandler takes care of creating the map again.

Fixes #8316

Review request: @jochen314, @J-N-K, @fwolter, @cpmeister, @Hilbrand.

Signed-off-by: Aitor Iturrioz riturrioz@gmail.com

Signed-off-by: Aitor Iturrioz <riturrioz@gmail.com>
@bodiroga bodiroga changed the title [mqtt.homeassistant] Clear availability subscriptions on dispose/stop [mqtt.generic] Clear availability subscriptions on stop Aug 20, 2020
@J-N-K
Copy link
Member

J-N-K commented Aug 22, 2020

@jochen314 I would like to hear your thoughts.

@jochen314
Copy link
Contributor

I guess, I missed that. The method was empty before...

Could we add the call to super.stop() also to HomieThingHandler, while we are doing this?

@Hilbrand Hilbrand added the bug An unexpected problem or unintended behavior of an add-on label Aug 27, 2020
Signed-off-by: Aitor Iturrioz <riturrioz@gmail.com>
@bodiroga
Copy link
Author

Could we add the call to super.stop() also to HomieThingHandler, while we are doing this?

Sure, I didn't add it because the Homie binding doesn't make use of the availability topic feature, but it is way better to keep all three handlers synchronized.

Done! 👍

Copy link
Contributor

@jochen314 jochen314 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Hilbrand Hilbrand merged commit 9187f9b into openhab:2.5.x Aug 27, 2020
@Hilbrand Hilbrand added this to the 2.5.9 milestone Aug 27, 2020
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
Closes openhab#8316

Signed-off-by: Aitor Iturrioz <riturrioz@gmail.com>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
Closes openhab#8316

Signed-off-by: Aitor Iturrioz <riturrioz@gmail.com>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
Closes openhab#8316

Signed-off-by: Aitor Iturrioz <riturrioz@gmail.com>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
Closes openhab#8316

Signed-off-by: Aitor Iturrioz <riturrioz@gmail.com>
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020
Closes openhab#8316

Signed-off-by: Aitor Iturrioz <riturrioz@gmail.com>
Signed-off-by: Daan Meijer <daan@studioseptember.nl>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Sep 12, 2020
Closes openhab#8316

Signed-off-by: Aitor Iturrioz <riturrioz@gmail.com>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
Closes openhab#8316

Signed-off-by: Aitor Iturrioz <riturrioz@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
Development

Successfully merging this pull request may close these issues.

[mqtt.homeassistant] Thing doesn't come online after editing or disabling-enabling it
4 participants