Issue63 #65
Merged
Issue63 #65
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… we might mess up json syntax. (fixes broken json when "hw", which is the last json entry, is empty)
…ey are still empty. This ensures that the message stays static at one point so home assistant doesn't update its internal device structures (leading to race conditions which prevent the picture from showing for example)
|
Thank you for your fix and your detailed problem analysis. We will try to integrate it, Currently it isn't building, but I will check that. |
SciLor
merged commit 111a525
into
toniebox-reverse-engineering:develop
2 of 8 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the none working picture in home assistant. (issue #63 )
The reason why this was not working is some sort of race condition.
The current architecture updates the sw and hw fields of the ha_info class when "mqtt_sendBoxEvent" is called. Here seems to be a problem (which has a root cause I sadly was unable to find due to lack of time). It seems that hw and sw fields are alternating between empty and sane values. This results in 2 things.
If hw is empty it will result in invalid json syntax for the home assistant autodiscovery topic (because hw is the last property in the json and is supposed to "close" json struct properly). I fixed this by allowing to publish empty values inside json which is a valid thing to do imo. This leads to valid json even if "hw" property is (still) empty.
The fact that the json content is changing all the time (because of alternating hw and sw properties) results in the fact that home assistant updates its internal device structures. It seems it unsubscribes from the topics (state and url) as well and resubscribes to them immediately. The fact that teddycloud pushes its state and url topics (for example the picture url) instantly after publishing the auto discovery topic seems to lead to the fact that home assistant misses that state change of the url. I fixed this by only updating sw and hw fields in case they are still empty. Using retained messages on teddy cloud side might fix this too - but I don't like those messages because the can get messy on the broker ;)
I know that the fix for the 2nd problem is not ideal. No problem if you don't consider this feasible to merge - but I use it for myself so I thought I would at least propose it here. :)