[senechome] Avoid null pointer errors #12512

thopiekar · 2022-03-23T20:38:26Z

Looks like the new generation of SenecIES power storage systems, which are using LiPo technology, provide additional data in the JSON array.
This data is not available in the old generation, which lead to missing data, when asking for a key and in further null pointer exceptions when going deeper in the data structure.

Therefore making a trivial check to detect the new generation of devices and skip the troubling section entirely.
CON: The section of items would not be updated. A to-do is of course to split the old and new generation into separate models of devices.

Fixes #11606

Also-By: Erwin Guib eguib@web.de
Signed-off-by: Thomas Karl Pietrowski thopiekar@gmail.com
Signed-off-by: Korbinian Probst kp.droid.dev@gmail.com

thopiekar · 2022-03-23T20:40:30Z

NOTE: Needs to be tested. Theoretically, it should work but coded all with my best gut feeling!

I'm currently on a milestone release and getting this error:

2022-03-23 21:16:13.646 [WARN ] [internal.service.FeaturesServiceImpl] - Can't load features repository mvn:org.apache.karaf.features/framework/4.3.2/xml/features
java.lang.RuntimeException: Error resolving artifact org.apache.karaf.features:framework:xml:features:4.3.2: [Could not find artifact org.apache.karaf.features:framework:xml:features:4.3.2 in openhab (https://openhab.jfrog.io/openhab/libs-milestone/)] : mvn:org.apache.karaf.features/framework/4.3.2/xml/features
        at org.apache.karaf.features.internal.service.RepositoryImpl.load(RepositoryImpl.java:121) ~[bundleFile:?]
        at org.apache.karaf.features.internal.service.RepositoryImpl.<init>(RepositoryImpl.java:51) ~[bundleFile:?]

No idea how to solve this and test my own work 😞

jlaur

Thanks - just a single comment with some advise to be sure problem is fixed and improve reliability. Perhaps it would also be worth checking if there are any null annotation warnings, didn't try to run a build myself.

...binding.senechome/src/main/java/org/openhab/binding/senechome/internal/SenecHomeHandler.java

thopiekar · 2022-03-25T17:27:18Z

See: #12512 (comment)

thopiekar · 2022-03-27T21:10:41Z

I managed to get it working just now. I had to wipe userdata/{cache,tmp}.
@jlaur Your suggestion doesn't work since there is always something inside response.battery.
There is a class that describes the expected dataset and when deserializing the received one, the ones that have not been sent will be omitted.
Therefore response.battery != null, but its children are.

I'm going to update the PR once I'm done with testing.

jlaur · 2022-03-27T21:24:48Z

Your suggestion doesn't work since there is always something inside response.battery.

I understand, but that was only part of the suggestion. Please see #12512 (comment).

thopiekar · 2022-03-27T22:06:45Z

@eguib Want to take a look at this PR? 👀

jlaur

Thanks for fixing this.

jlaur · 2022-03-28T05:05:18Z

@eguib Want to take a look at this PR? 👀

Let me know when you are ready for having it merged. :)

...binding.senechome/src/main/java/org/openhab/binding/senechome/internal/SenecHomeHandler.java

jlaur

See #12512 (comment)

thopiekar · 2022-03-29T17:00:20Z

@jlaur We are ready to go! Feel free to take a last look and, if everything is fine, push the merge button 😄

jlaur

LGTM

jlaur · 2022-03-29T18:45:58Z

@jlaur We are ready to go! Feel free to take a last look and, if everything is fine, push the merge button 😄

One last thing: Can you shorten the commit message: https://www.openhab.org/docs/developer/contributing.html#conventions

And fix the co-signers:
https://www.openhab.org/docs/developer/contributing.html#sign-your-work

KorbinianP

Thank you for the patch

thopiekar · 2022-04-03T19:00:10Z

Ok, corrected the signed-of tags. Made @eguib "also-by".
Just don't see how the commit message is too long.. The title is perfectly fine (max 50 letters) and I read about no limit on the commit description.

>>> len("[senechome] Avoid null pointer errors")
37

Looks like the new generation of SenecIES power storage systems, which are using LiPo technology, provide additional data in the JSON array. This data is not available in the old generation, which lead to missing data, when asking for a key and in further null pointer exceptions when going deeper in the data structure. Therefore making a trivial check to detect the new generation of devices and skip the troubling section entirely. CON: The section of items would not be updated. A to-do is of course to split the old and new generation into separate models of devices. Fixes openhab#11606 Also-By: Erwin Guib <eguib@web.de> Signed-off-by: Thomas Karl Pietrowski <thopiekar@gmail.com> Signed-off-by: Korbinian Probst <kp.droid.dev@gmail.com>

jlaur · 2022-04-03T19:11:27Z

Ok, corrected the signed-of tags. Made @eguib "also-by".
Just don't see how the commit message is too long.. The title is perfectly fine (max 50 letters) and I read about no limit on the commit description.

It's fine! I might have mixed up something previously. It's ready to go, thanks for the fix. I can tweak the sign-off further when squash-merging as I believe there should be only one sign-off, so I would move Korbinian to "Also-By". Please confirm. After that I'll merge.

thopiekar · 2022-04-03T19:13:16Z

Well, @KorbinianP didn't take a look at the problem in-depth, but reviewed @eguib 's and my solution. Therefore I would keep the sense and keep him "signed-off-by".

thopiekar · 2022-04-03T19:14:38Z

.. otherwise someone might get questions, look into the commit message and contact him about why the heck something has been done as is. I mean yeah, he knows the case now, but logically I would keep the commit as it is.

jlaur · 2022-04-03T19:24:01Z

Well, @KorbinianP didn't take a look at the problem in-depth, but reviewed @eguib 's and my solution. Therefore I would keep the sense and keep him "signed-off-by".

For such small commit I guess the usual approach would be simply letting the committer sign-off the commit. But since you insist, I have to consult other maintainers, since I'm not fully aware of the rules. @openhab/add-ons-maintainers - is it allowed to have more than one "Signed-off-by" in commit message? The DCO check passes, but it seems inconsistent being able to mix "Signed-off-by" and "Also-by" this way - it could even obsolete "Also-by" if you could just choose to have multiple "Signed-off by" instead?

thopiekar · 2022-04-03T19:29:40Z

I'm not sure about the rules in detail here, too, just following my logic and my gut feeling.
If there is anything I need to correct still let me know! 👍

KorbinianP · 2022-04-04T05:16:26Z

Hello, how do I sign off if I don't commit anything? Or how can I help to get this merged?

wborn · 2022-04-04T10:02:26Z

The person creating the PR adds the "sign off" commit message and ensures that other contributors also certify the DCO. The other contributors are then added in the "Also-By" line:

See: https://www.openhab.org/docs/developer/contributing.html#sign-your-work

If your commit contains code from others as well, please ensure that they certify the DCO as well and add them with an "Also-By" line to your commit message:

Also-by: Ted Nerd <ted.nerd@email.com>
Also-by: Sue Walker <sue.walker@email.com>
Signed-off-by: Joe Smith <joe.smith@email.com>

The "DCO check" only checks that the last line of a commit message is Signed-off-by: and it matches the author e-mail of the commit. It doesn't check any Also-by lines or if there are multiple Signed-off-by lines.

So when all contributors certify the Developer Certificate of Origin (DCO), the PR can be squashed with the following lines at the end of the commit message:

Also-By: Erwin Guib <eguib@web.de>
Also-By: Korbinian Probst <kp.droid.dev@gmail.com>
Signed-off-by: Thomas Karl Pietrowski <thopiekar@gmail.com>

KorbinianP · 2022-04-04T13:19:06Z

Cool, thank you all!

thopiekar · 2022-04-05T17:48:01Z

Thank you for the explanation @wborn

Looks like the new generation of SenecIES power storage systems, which are using LiPo technology, provide additional data in the JSON array. This data is not available in the old generation, which lead to missing data, when asking for a key and in further null pointer exceptions when going deeper in the data structure. Therefore making a trivial check to detect the new generation of devices and skip the troubling section entirely. CON: The section of items would not be updated. A to-do is of course to split the old and new generation into separate models of devices. Fixes openhab#11606 Also-By: Erwin Guib <eguib@web.de> Also-By: Korbinian Probst <kp.droid.dev@gmail.com> Signed-off-by: Thomas Karl Pietrowski <thopiekar@gmail.com> Signed-off-by: Nick Waterton <n.waterton@outlook.com>

Looks like the new generation of SenecIES power storage systems, which are using LiPo technology, provide additional data in the JSON array. This data is not available in the old generation, which lead to missing data, when asking for a key and in further null pointer exceptions when going deeper in the data structure. Therefore making a trivial check to detect the new generation of devices and skip the troubling section entirely. CON: The section of items would not be updated. A to-do is of course to split the old and new generation into separate models of devices. Fixes openhab#11606 Also-By: Erwin Guib <eguib@web.de> Also-By: Korbinian Probst <kp.droid.dev@gmail.com> Signed-off-by: Thomas Karl Pietrowski <thopiekar@gmail.com>

Looks like the new generation of SenecIES power storage systems, which are using LiPo technology, provide additional data in the JSON array. This data is not available in the old generation, which lead to missing data, when asking for a key and in further null pointer exceptions when going deeper in the data structure. Therefore making a trivial check to detect the new generation of devices and skip the troubling section entirely. CON: The section of items would not be updated. A to-do is of course to split the old and new generation into separate models of devices. Fixes openhab#11606 Also-By: Erwin Guib <eguib@web.de> Also-By: Korbinian Probst <kp.droid.dev@gmail.com> Signed-off-by: Thomas Karl Pietrowski <thopiekar@gmail.com> Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>

Looks like the new generation of SenecIES power storage systems, which are using LiPo technology, provide additional data in the JSON array. This data is not available in the old generation, which lead to missing data, when asking for a key and in further null pointer exceptions when going deeper in the data structure. Therefore making a trivial check to detect the new generation of devices and skip the troubling section entirely. CON: The section of items would not be updated. A to-do is of course to split the old and new generation into separate models of devices. Fixes openhab#11606 Also-By: Erwin Guib <eguib@web.de> Also-By: Korbinian Probst <kp.droid.dev@gmail.com> Signed-off-by: Thomas Karl Pietrowski <thopiekar@gmail.com>

thopiekar requested a review from vctender as a code owner March 23, 2022 20:38

jlaur changed the title ~~senechome: Avoid null pointer errors~~ [senechome] Avoid null pointer errors Mar 23, 2022

jlaur requested changes Mar 23, 2022

View reviewed changes

...binding.senechome/src/main/java/org/openhab/binding/senechome/internal/SenecHomeHandler.java Show resolved Hide resolved

jlaur added the bug An unexpected problem or unintended behavior of an add-on label Mar 23, 2022

thopiekar force-pushed the feature/senechome_avoid-null-pointer-errors branch from c02b912 to 18ebdc5 Compare March 24, 2022 20:45

jlaur approved these changes Mar 28, 2022

View reviewed changes

KorbinianP mentioned this pull request Mar 28, 2022

[senechome] null pointer errors with SENEC.Home 8.0 Pb #11606

Closed

KorbinianP suggested changes Mar 28, 2022

View reviewed changes

...binding.senechome/src/main/java/org/openhab/binding/senechome/internal/SenecHomeHandler.java Show resolved Hide resolved

jlaur requested changes Mar 29, 2022

View reviewed changes

thopiekar force-pushed the feature/senechome_avoid-null-pointer-errors branch from 18ebdc5 to e91e5fb Compare March 29, 2022 16:55

thopiekar force-pushed the feature/senechome_avoid-null-pointer-errors branch from e91e5fb to 42c25df Compare March 29, 2022 17:01

thopiekar requested a review from jlaur March 29, 2022 17:08

jlaur approved these changes Mar 29, 2022

View reviewed changes

KorbinianP approved these changes Mar 30, 2022

View reviewed changes

thopiekar force-pushed the feature/senechome_avoid-null-pointer-errors branch from 42c25df to c570d78 Compare April 3, 2022 19:00

thopiekar force-pushed the feature/senechome_avoid-null-pointer-errors branch from c570d78 to 07069ba Compare April 3, 2022 19:02

jlaur merged commit 2a2389a into openhab:main Apr 4, 2022

jlaur added this to the 3.3 milestone Apr 4, 2022

thopiekar deleted the feature/senechome_avoid-null-pointer-errors branch April 5, 2022 17:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[senechome] Avoid null pointer errors #12512

[senechome] Avoid null pointer errors #12512

thopiekar commented Mar 23, 2022 •

edited

Loading

thopiekar commented Mar 23, 2022 •

edited

Loading

jlaur left a comment

thopiekar commented Mar 25, 2022

thopiekar commented Mar 27, 2022 •

edited

Loading

jlaur commented Mar 27, 2022

thopiekar commented Mar 27, 2022

jlaur left a comment

jlaur commented Mar 28, 2022

jlaur left a comment

thopiekar commented Mar 29, 2022

jlaur left a comment

jlaur commented Mar 29, 2022

KorbinianP left a comment

thopiekar commented Apr 3, 2022

jlaur commented Apr 3, 2022

thopiekar commented Apr 3, 2022

thopiekar commented Apr 3, 2022

jlaur commented Apr 3, 2022

thopiekar commented Apr 3, 2022

KorbinianP commented Apr 4, 2022 •

edited

Loading

wborn commented Apr 4, 2022

KorbinianP commented Apr 4, 2022

thopiekar commented Apr 5, 2022

[senechome] Avoid null pointer errors #12512

[senechome] Avoid null pointer errors #12512

Conversation

thopiekar commented Mar 23, 2022 • edited Loading

thopiekar commented Mar 23, 2022 • edited Loading

jlaur left a comment

Choose a reason for hiding this comment

thopiekar commented Mar 25, 2022

thopiekar commented Mar 27, 2022 • edited Loading

jlaur commented Mar 27, 2022

thopiekar commented Mar 27, 2022

jlaur left a comment

Choose a reason for hiding this comment

jlaur commented Mar 28, 2022

jlaur left a comment

Choose a reason for hiding this comment

thopiekar commented Mar 29, 2022

jlaur left a comment

Choose a reason for hiding this comment

jlaur commented Mar 29, 2022

KorbinianP left a comment

Choose a reason for hiding this comment

thopiekar commented Apr 3, 2022

jlaur commented Apr 3, 2022

thopiekar commented Apr 3, 2022

thopiekar commented Apr 3, 2022

jlaur commented Apr 3, 2022

thopiekar commented Apr 3, 2022

KorbinianP commented Apr 4, 2022 • edited Loading

wborn commented Apr 4, 2022

KorbinianP commented Apr 4, 2022

thopiekar commented Apr 5, 2022

thopiekar commented Mar 23, 2022 •

edited

Loading

thopiekar commented Mar 23, 2022 •

edited

Loading

thopiekar commented Mar 27, 2022 •

edited

Loading

KorbinianP commented Apr 4, 2022 •

edited

Loading