-
-
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]Enhancements and bug fixes #6764
Conversation
…hs new PR Signed-off-by: markus7017 <markus7017@gmail.com>
Travis tests have failedHey @markus7017, 2nd BuildExpand here
|
Signed-off-by: markus7017 <markus7017@gmail.com>
Travis tests were successfulHey @markus7017, |
PR #6728 will solve most of the Travis warnings |
Signed-off-by: markus7017 <markus7017@gmail.com>
Travis tests were successfulHey @markus7017, |
Signed-off-by: markus7017 <markus7017@gmail.com>
Travis tests were successfulHey @markus7017, |
@kaikreuzer Who will be the 2nd reviewer? |
Travis tests were successfulHey @markus7017, |
Signed-off-by: Markus Michels <markus7017@gmail.com>
Well, just delete your local 2.5.x branch, rename shelly-snapshot-2.5.x to 2.5.x and do a force push. That should bring this PR to the code revision you have on your shelly-snapshot-2.5.x branch. |
Alternatively do a cherry pick of the new commits from shelly-snapshot-2.5.x to 2.5.x and do a normal push. |
marked as advanced; Typos in properties corrected Signed-off-by: Markus Michels <markus7017@gmail.com>
Travis tests have failedHey @markus7017, 1st BuildExpand here
### 2nd Build Expand here
|
Signed-off-by: Markus Michels <markus7017@gmail.com>
Travis tests were successfulHey @markus7017, |
@kaikreuzer Ok, should be ready to continue. Last change is setting status UNKNOWN on initialization and this shows up here and in my 2.5.x branch. |
* @param status Shelly device status | ||
* @return true: one or more inputs were updated | ||
*/ | ||
@SuppressWarnings({ "null", "unused" }) |
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.
Why do you have to suppress unused? Can't you simply remove unused variables?
Also, please try to avoid null suppressions and rather refactor the code to be fully null-safe.
Ok, if you address this in a future PR, no blocker wrt merging now.
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.
because Eclipse sometimes just report BS
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.
example: For this statement Eclipse reports a redundant null check
Strong thingName = properties.get(PROPERTY_SERVICE_NAME) != null ? properties.get(PROPERTY_SERVICE_NAME).toLowerCase() : thingType;
how to resolved those warnings?
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.
Which is correct, because theoretically another thread could meanwhile remove the property before you call the get.
This should solve those issues:
String thingName = properties.get(PROPERTY_SERVICE_NAME);
thingName = thingName!=null ? thingName.toLowerCase() : thingType;
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.
nope, now it's "thingName!=null"
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.
Then probably this way:
String serviceName = properties.get(PROPERTY_SERVICE_NAME);
String thingName = serviceName!=null ? serviceName.toLowerCase() : thingType;
That's anyhow cleaner than my previous suggestion.
bundles/org.openhab.binding.shelly/src/main/resources/ESH-INF/config/config.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.shelly/src/main/resources/ESH-INF/config/config.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.shelly/src/main/resources/ESH-INF/config/config.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.shelly/src/main/resources/ESH-INF/thing/channels.xml
Outdated
Show resolved
Hide resolved
Signed-off-by: Markus Michels <markus7017@gmail.com>
Signed-off-by: Markus Michels <markus7017@gmail.com>
@kaikreuzer done |
Travis tests were successfulHey @markus7017, |
Travis tests were successfulHey @markus7017, |
Signed-off-by: markus7017 <markus7017@gmail.com>
Signed-off-by: markus7017 <markus7017@gmail.com> Signed-off-by: leluna <hengrui.jiang@googlemail.com>
Signed-off-by: markus7017 <markus7017@gmail.com> Signed-off-by: Hans-Reiner Hoffmann <hans-reiner.hoffmann@gmx.de>
Signed-off-by: markus7017 <markus7017@gmail.com>
Signed-off-by: markus7017 <markus7017@gmail.com>
Signed-off-by: markus7017 <markus7017@gmail.com>
Signed-off-by: markus7017 <markus7017@gmail.com>
Signed-off-by: markus7017 <markus7017@gmail.com>
Signed-off-by: markus7017 <markus7017@gmail.com> Signed-off-by: Daan Meijer <daan@studioseptember.nl>
This PR includes all changes from #6592 and replaces it. PR #6592 has been closed.
Signed-off-by: markus7017 markus7017@gmail.com