[miio] Elimate several SAT warnings #9289
Conversation
Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>
Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>
Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>
Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>
|
Builderror in Jenkins unrelated to the PR |
...hab.binding.miio/src/main/java/org/openhab/binding/miio/internal/cloud/MiCloudConnector.java
Outdated
Show resolved
Hide resolved
...rg.openhab.binding.miio/src/main/java/org/openhab/binding/miio/internal/robot/RRMapDraw.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>
Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>
marcelrv
added
rebuild
...hab.binding.miio/src/main/java/org/openhab/binding/miio/internal/cloud/MiCloudConnector.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>
marcelrv
added
rebuild
| cmdResponse.get(0).isJsonPrimitive() ? cmdResponse.get(0) | ||
| : new JsonPrimitive(cmdResponse.get(0).toString())); |
There was a problem hiding this comment.
Please cache cmdResponse.get(0) in a local variable.
...b.binding.miio/src/main/java/org/openhab/binding/miio/internal/handler/MiIoBasicHandler.java
Outdated
Show resolved
Hide resolved
| } | ||
| } else { | ||
| errorMsg = "Received message is invalid JSON"; | ||
| } catch (ClassCastException | IllegalStateException e) { |
There was a problem hiding this comment.
What would throw either of these exceptions?
There was a problem hiding this comment.
During my testing I had all sorts of garbage responses that came form the Xiaomi cloud. (so invalid json)
Causing exceptions during the reading and processing of the json responses.
At most places I was already catching these, but seems this one not.
bundles/org.openhab.binding.miio/src/main/java/org/openhab/binding/miio/internal/Utils.java
Outdated
Show resolved
Hide resolved
| return ""; | ||
| } | ||
|
|
||
| } catch (JsonSyntaxException | NullPointerException e) { | ||
| } catch (JsonParseException | IllegalStateException | ClassCastException e) { |
There was a problem hiding this comment.
What would throw a ClassCastException or IllegalStateException here?
There was a problem hiding this comment.
as before comment, the responses of Xiaomi cloud are many times corrupt.
maybe I'm overly caution here to catch all possible errors that can be thrown in this part of the process.
During my testing for this PR I had some JsonParseException & IllegalStateExceptions in the module, not 100% sure anymore if it was in this specific step. I may have added it to multiple steps to avoid the same.
|
@cpmeister are you okay with the changes implemented, or still something not fine for merging? |
|
did you miss my last review? |
Co-authored-by: Connor Petty <mistercpp2000@gmail.com>
Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>
marcelrv
added
rebuild
|
@cpmeister let me know your thoughts on catching the multiple exceptions. I don't want that a exception in the cloud would prevent other parts of the process to stop (e.g. the discovery checks for the device details from the cloud, but if that runs into an exception, I still want the discover to proceed and submit the found device with a less specific details) |
I think we crossed messages.... when I wrote it the review was not in yet, so most likely I wrote is while you were commenting |
cpmeister
added
rebuild
|
Jenkins build error unrelated to the binding/PR |
* [miio] eliminate warnings from mapdraw * [miio] clean warnings from basic handler * [miio] avoid apache commons warning in utils * [miio] eliminate warnings from micloudconnector
* [miio] eliminate warnings from mapdraw * [miio] clean warnings from basic handler * [miio] avoid apache commons warning in utils * [miio] eliminate warnings from micloudconnector Signed-off-by: Marcel Verpaalen <marcel@verpaalen.com>
eliminating sat warnings and minor improvements in the miloudconnector for failed attempts