-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[shelly] Fix UNI support up to three DS18B20 sensors #15530
Conversation
… in order Signed-off-by: Anton Hattendorf <anton@hattendoerfer.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@markus7017: could you please review ? |
if (json.contains("\"ext_temperature\":{\"0\":[{") || json.contains("\"ext_temperature\":{\"1\":[{") | ||
|| json.contains("\"ext_temperature\":{\"2\":[{")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this be sufficient ?
if (json.contains("\"ext_temperature\":{\"")) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't tell.
I assume there was a reason, the JSON string is checked including the array index. So I just added the two other possible cases, but keeping as close to the original condition as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait for @markus7017 feedback
@markus7017 : are you there ? |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/oh4-runs-out-of-memory/148699/66 |
LGTM |
… in order (openhab#15530) Signed-off-by: Anton Hattendorf <anton@hattendoerfer.de>
… in order (openhab#15530) Signed-off-by: Anton Hattendorf <anton@hattendoerfer.de>
… in order (openhab#15530) Signed-off-by: Anton Hattendorf <anton@hattendoerfer.de> Signed-off-by: querdenker2k <querdenker2k@gmx.de>
… in order (openhab#15530) Signed-off-by: Anton Hattendorf <anton@hattendoerfer.de> Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Description
For the Shelly UNI a small fix needs to be applied to the JSON that is received form the device. When two or more temperature sensors are attached, the array is not sorted and the sensor "1" is first, causes the string replacement to fail.
With this patch all tree possible variants are checked, so that the JSON fix is applied for all possible sortings.
Note: The Shelly UNI supports only up to three DS18B20.
Testing
Here is the build artifact:
https://openhab.jfrog.io/artifactory/libs-pullrequest-local/org/openhab/addons/bundles/org.openhab.binding.shelly/4.1.0-SNAPSHOT/org.openhab.binding.shelly-4.1.0-SNAPSHOT.jar
Installed and tested on my local system - Shelly gets
ONLINE
and all three DS18B20 are detected and working.Note: This is my first pull request, I hope I'll got everything correct.