[knx] Add integration tests #15727

holgerfriedrich · 2023-10-09T22:25:03Z

Add integration tests which include decoding and encoding KNX frames, and the handling by Calimero library as well. This ensures that the KNX wire format is handled properly.
Add itests for all main DPT numbers supported by Calimero.
Add a mechanism to check for new missing DPT main numbers when new Calimero release is integrated.

The idea of this PR is basically to check the whole way from bytes on the KNX bus through Calimero, into the plugin, and back again via Calimero down to the bus again. The implementation overwrites KNXNetworkLink to implement a test interface. Calimero is then set up on top.

The benefit of this approach is that we now cover large parts of the handling of KNX frame in the Calimero library, without the need of setting up real itests including bus communication to mimic the KNX communication.

PR is ready for review, tests for more DPT subtypes can be added later.

J-N-K · 2023-10-10T10:29:30Z

Maybe we can test both at once and return the send frame as incoming frame from the bus?

holgerfriedrich · 2023-10-14T05:26:08Z

Maybe we can test both at once and return the send frame as incoming frame from the bus?

Exactly. Now a CEMI frame is generated from the data, which is used to send an indication to all listeners. A ProcessCommunicator impl gets the data and supplies it to a dummy ProcessListener, where the test can grab the information. This is close to the way we supply data from KNX to ValueDecoder.decode().

Concept is working, test for 251.600 is there.

PR needs some generalization and other data types to be covered.

holgerfriedrich · 2023-10-15T11:53:37Z

Generalization to other DPTs is done, tests for different DPTs have been added.
Extending the tests to more DPTs is not scope of this PR.

holgerfriedrich · 2023-10-22T07:20:44Z

Cleaned up the commit history, as this should not be squashed during merge.
Ready for review.

* Add integration tests which include decoding and encoding KNX frames, and the handling by Calimero library as well. This ensures that the KNX wire format is handled properly. * Add itests for DPT1 and color DPTs 251.600 and 232.600. Signed-off-by: Holger Friedrich <[email protected]>

- Change default type of DPT14 to QuantityType - Allow for parsing DPT20, DPT21, DPT22 as DecimalType in addition to String - Add DPTs for color transitions, mapped to String: DPT243.600, DPT249.600, DPT250.600, DPT252.600, DPT253.600, DPT254.600 - Adapt handling of DST flag in DPT19.001 Signed-off-by: Holger Friedrich <[email protected]>

holgerfriedrich · 2023-11-03T21:53:27Z

Fixed a few typos in the code comments. Ready for review.

holgerfriedrich · 2023-12-10T15:39:15Z

@J-N-K can I ask you to have a short look into this PR?
Maybe you can remember why we had the code handling "no-day" for DPT 19.001. I removed this in 534f593 as I could not reproduce this (neither for German nor English locale, and the source code of Calimero does not contain it as well).

J-N-K

In general looks good an very useful.

bundles/org.openhab.binding.knx/doc/dpt.txt

...org.openhab.binding.knx/src/main/java/org/openhab/binding/knx/internal/dpt/ValueDecoder.java

...openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/itests/Back2BackTest.java

J-N-K · 2023-12-10T16:27:42Z

...openhab.binding.knx/src/test/java/org/openhab/binding/knx/internal/itests/Back2BackTest.java

+import tuwien.auto.calimero.process.ProcessCommunicatorImpl;
+
+/**
+ * Integration test to check conversion from raw KNX frame data to OH data types and back.


This is not an integration test in the sense we usually use it in the openHAB context. We call a test an integration test if it starts the whole framework and executes tests in a running application.

I see that as well. Looking from the perspective of the binding code, it is more than a unit test, as we involve the full Calimero stack. How would you call it? Back2Back? E2E? Though, E2E is typically considered even more as integration test....

@J-N-K Do you have a proposal? Or should we merge as it is?

…ing/knx/internal/dpt/ValueDecoder.java Co-authored-by: J-N-K <[email protected]> Signed-off-by: Holger Friedrich <[email protected]>

holgerfriedrich · 2023-12-16T10:49:06Z

@kaikreuzer I would like to see this in the next milestone build if possible. It should be merged as 2 separate commits to keep the records. I think we have only one open point above: can we call this an integration test or shall I change the naming to something different.... WDYT?

kaikreuzer · 2023-12-16T11:39:21Z

I'll leave merging this to @J-N-K as he already did the initial review.

J-N-K

I would say that the benefit of having these tests outweigh any potential confusion due to the naming. Since no-one seems to have a better proposal, we should merge as-is.

holgerfriedrich · 2023-12-16T11:53:36Z

thank you!

* [knx] Add integration tests * [knx] Adapt handling of DPTs Signed-off-by: Holger Friedrich <[email protected]> Signed-off-by: Jørgen Austvik <[email protected]>

* [knx] Add integration tests * [knx] Adapt handling of DPTs Signed-off-by: Holger Friedrich <[email protected]>

holgerfriedrich added work in progress A PR that is not yet ready to be merged test labels Oct 9, 2023

holgerfriedrich requested a review from kaikreuzer as a code owner October 9, 2023 22:25

holgerfriedrich mentioned this pull request Oct 9, 2023

[knx] Bugfix problem with DPT 251.600 #15723

Merged

holgerfriedrich force-pushed the pr-knx-tests branch from b8a26ca to b94edca Compare October 14, 2023 05:16

holgerfriedrich force-pushed the pr-knx-tests branch from b94edca to 2b0cc15 Compare October 15, 2023 11:47

holgerfriedrich removed the work in progress A PR that is not yet ready to be merged label Oct 15, 2023

holgerfriedrich changed the title ~~[knx] Enhance tests~~ [knx] Add integration tests Oct 15, 2023

holgerfriedrich requested a review from J-N-K October 15, 2023 11:52

holgerfriedrich force-pushed the pr-knx-tests branch 4 times, most recently from 73527c2 to f36d2ed Compare October 22, 2023 07:19

holgerfriedrich force-pushed the pr-knx-tests branch from f36d2ed to dfc8997 Compare October 22, 2023 10:27

holgerfriedrich added the enhancement An enhancement or new feature for an existing add-on label Oct 22, 2023

holgerfriedrich force-pushed the pr-knx-tests branch from dfc8997 to 95ea7c6 Compare November 3, 2023 21:04

holgerfriedrich added 2 commits November 3, 2023 22:47

holgerfriedrich force-pushed the pr-knx-tests branch from 95ea7c6 to 534f593 Compare November 3, 2023 21:47

J-N-K requested changes Dec 10, 2023

View reviewed changes

Update bundles/org.openhab.binding.knx/src/main/java/org/openhab/bind…

2f31324

…ing/knx/internal/dpt/ValueDecoder.java Co-authored-by: J-N-K <[email protected]> Signed-off-by: Holger Friedrich <[email protected]>

holgerfriedrich requested a review from J-N-K December 11, 2023 18:25

J-N-K approved these changes Dec 16, 2023

View reviewed changes

J-N-K added this to the 4.1 milestone Dec 16, 2023

J-N-K merged commit 4ba325d into openhab:main Dec 16, 2023
3 checks passed

holgerfriedrich deleted the pr-knx-tests branch December 16, 2023 11:53

joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024

[knx] Add integration tests (openhab#15727)

6130333

* [knx] Add integration tests * [knx] Adapt handling of DPTs Signed-off-by: Holger Friedrich <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[knx] Add integration tests #15727

[knx] Add integration tests #15727

holgerfriedrich commented Oct 9, 2023 •

edited

Loading

J-N-K commented Oct 10, 2023

holgerfriedrich commented Oct 14, 2023

holgerfriedrich commented Oct 15, 2023

holgerfriedrich commented Oct 22, 2023

holgerfriedrich commented Nov 3, 2023

holgerfriedrich commented Dec 10, 2023

J-N-K left a comment

J-N-K Dec 10, 2023

holgerfriedrich Dec 10, 2023

holgerfriedrich Dec 15, 2023

holgerfriedrich commented Dec 16, 2023

kaikreuzer commented Dec 16, 2023

J-N-K left a comment

holgerfriedrich commented Dec 16, 2023 •

edited

Loading

[knx] Add integration tests #15727

[knx] Add integration tests #15727

Conversation

holgerfriedrich commented Oct 9, 2023 • edited Loading

J-N-K commented Oct 10, 2023

holgerfriedrich commented Oct 14, 2023

holgerfriedrich commented Oct 15, 2023

holgerfriedrich commented Oct 22, 2023

holgerfriedrich commented Nov 3, 2023

holgerfriedrich commented Dec 10, 2023

J-N-K left a comment

Choose a reason for hiding this comment

J-N-K Dec 10, 2023

Choose a reason for hiding this comment

holgerfriedrich Dec 10, 2023

Choose a reason for hiding this comment

holgerfriedrich Dec 15, 2023

Choose a reason for hiding this comment

holgerfriedrich commented Dec 16, 2023

kaikreuzer commented Dec 16, 2023

J-N-K left a comment

Choose a reason for hiding this comment

holgerfriedrich commented Dec 16, 2023 • edited Loading

holgerfriedrich commented Oct 9, 2023 •

edited

Loading

holgerfriedrich commented Dec 16, 2023 •

edited

Loading