Smarteon / loxone-java

Java implementation of the Loxone™ communication protocol (Web Socket)
BSD 3-Clause "New" or "Revised" License
15 stars 10 forks source link

First draft for state retention #195

Closed TCke83 closed 1 year ago

TCke83 commented 1 year ago

New pull request with same changes + added tests.

The LoxoneState class needs some help for testing, i'm still a newbie with the used mocking framework and with kotlin code.

codecov[bot] commented 1 year ago

Codecov Report

Base: 79.57% // Head: 79.44% // Decreases project coverage by -0.14% :warning:

Coverage data is based on head (3afcc79) compared to base (0fd541f). Patch coverage: 76.85% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #195 +/- ## ============================================ - Coverage 79.57% 79.44% -0.14% - Complexity 723 766 +43 ============================================ Files 100 108 +8 Lines 2120 2228 +108 Branches 176 181 +5 ============================================ + Hits 1687 1770 +83 - Misses 341 364 +23 - Partials 92 94 +2 ``` | [Impacted Files](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon) | Coverage Δ | | |---|---|---| | [src/main/java/cz/smarteon/loxone/app/Control.java](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon#diff-c3JjL21haW4vamF2YS9jei9zbWFydGVvbi9sb3hvbmUvYXBwL0NvbnRyb2wuamF2YQ==) | `43.75% <0.00%> (-6.25%)` | :arrow_down: | | [...n/java/cz/smarteon/loxone/message/LoxoneEvent.java](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon#diff-c3JjL21haW4vamF2YS9jei9zbWFydGVvbi9sb3hvbmUvbWVzc2FnZS9Mb3hvbmVFdmVudC5qYXZh) | `100.00% <ø> (ø)` | | | [...ain/java/cz/smarteon/loxone/message/TextEvent.java](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon#diff-c3JjL21haW4vamF2YS9jei9zbWFydGVvbi9sb3hvbmUvbWVzc2FnZS9UZXh0RXZlbnQuamF2YQ==) | `85.71% <ø> (ø)` | | | [...in/java/cz/smarteon/loxone/message/ValueEvent.java](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon#diff-c3JjL21haW4vamF2YS9jei9zbWFydGVvbi9sb3hvbmUvbWVzc2FnZS9WYWx1ZUV2ZW50LmphdmE=) | `80.00% <ø> (ø)` | | | [.../smarteon/loxone/app/state/events/LockedEvent.java](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon#diff-c3JjL21haW4vamF2YS9jei9zbWFydGVvbi9sb3hvbmUvYXBwL3N0YXRlL2V2ZW50cy9Mb2NrZWRFdmVudC5qYXZh) | `50.00% <50.00%> (ø)` | | | [src/main/java/cz/smarteon/loxone/Loxone.java](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon#diff-c3JjL21haW4vamF2YS9jei9zbWFydGVvbi9sb3hvbmUvTG94b25lLmphdmE=) | `85.71% <66.66%> (-0.65%)` | :arrow_down: | | [...java/cz/smarteon/loxone/app/state/LoxoneState.java](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon#diff-c3JjL21haW4vamF2YS9jei9zbWFydGVvbi9sb3hvbmUvYXBwL3N0YXRlL0xveG9uZVN0YXRlLmphdmE=) | `69.44% <69.44%> (ø)` | | | [.../smarteon/loxone/app/state/SwitchControlState.java](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon#diff-c3JjL21haW4vamF2YS9jei9zbWFydGVvbi9sb3hvbmUvYXBwL3N0YXRlL1N3aXRjaENvbnRyb2xTdGF0ZS5qYXZh) | `76.47% <76.47%> (ø)` | | | [...marteon/loxone/app/state/LockableControlState.java](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon#diff-c3JjL21haW4vamF2YS9jei9zbWFydGVvbi9sb3hvbmUvYXBwL3N0YXRlL0xvY2thYmxlQ29udHJvbFN0YXRlLmphdmE=) | `84.21% <84.21%> (ø)` | | | [...rteon/loxone/app/state/AnalogInfoControlState.java](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon#diff-c3JjL21haW4vamF2YS9jei9zbWFydGVvbi9sb3hvbmUvYXBwL3N0YXRlL0FuYWxvZ0luZm9Db250cm9sU3RhdGUuamF2YQ==) | `100.00% <100.00%> (ø)` | | | ... and [6 more](https://codecov.io/gh/Smarteon/loxone-java/pull/195?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smarteon)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

jimirocks commented 1 year ago

Hi, that's a lot of code. It will take a while to review. But well, thanks for contribution.

TCke83 commented 1 year ago

It's the same code change as PR #174, but it took me a while to find the time to write the tests

jimirocks commented 1 year ago

So we put some guidance for testing here: https://github.com/TCke83/loxone-java/pull/1/files please take a look, we can discuss further...

TCke83 commented 1 year ago

Added a few tests for LoxoneState class

jimirocks commented 1 year ago

Hi @TCke83 do you plan to work more on this yet, or should we do final walkthrough? I briefly checked the overall coverage and it would be nice to test also th exceptional flows in LoxoneState - do you nedd help with that?

Once you feel it shoudl be finalized, please just reorganize the commits - one commit with lombok refactorings and everything else could be in the second one.

TCke83 commented 1 year ago

I've added a test for the constructor check. The exception flow in the initializeState() method is covered in test 'controlStates should be compatible'. I don't really see a way to test the initializeState() method further, if you think this should be done, some help would be welcome.

jimirocks commented 1 year ago

Please take a look at https://github.com/TCke83/loxone-java/pull/2 I finished the changes my colleague proposed as guidance. Please take a look and take notes for the future. The exceptions in LoxoneState are still not covered, but it's probably OK for now - can be added in future development if needed. Once you accept my changes, squash it into single commit and I am ready to merge and release.

TCke83 commented 1 year ago

I've looked at the test and merged & squashed them in the pull request. Thanks for the help and guidance.