[homewizard] Add current, voltage and failure channels #16995

lsiepel · 2024-07-04T20:56:30Z

Add active current channels (L1,L2,L3 + sum of all)
Add active voltage channels (L1,L2,L3 + generic)
Add power failure count channels (any + long)
Add tests and minor refactoring
Changed gas timestamp now respects openHAB configured timezone

Resolves: #16988

Signed-off-by: Leo Siepel <[email protected]>

lsiepel · 2024-08-24T11:04:51Z

Would be nice if we can get this merged, then it would be easier to move #13495 forward.

Signed-off-by: Leo Siepel <[email protected]>

lsiepel · 2024-08-28T19:09:32Z

Got a little bit messy because of the merge conflicts. But all should be ok now. Ready for a review. Was tested by community and unit tests.

jlaur

Thanks for the new channels! I have added a few comments.

.../src/main/java/org/openhab/binding/homewizard/internal/handler/HomeWizardP1MeterHandler.java

jlaur · 2024-09-28T11:15:08Z

...inding.homewizard/src/main/java/org/openhab/binding/homewizard/internal/dto/DataPayload.java

+        try {
+            return ZonedDateTime.of(year, month, day, hours, minutes, seconds, 0, ZoneId.systemDefault());
+        } catch (DateTimeException e) {
+            LoggerFactory.getLogger(DataPayload.class).warn("Unable to parse Gas timestamp: {}", gasTimestamp);


I'm not sure we should log from a DTO get method. Perhaps this could be logged in the handler instead (even if this would require a getter for the raw value to be logged)?

Moved logging to the handler.

jlaur · 2024-09-28T11:16:08Z

bundles/org.openhab.binding.homewizard/src/main/resources/OH-INF/i18n/homewizard.properties

+thing-type.homewizard.energy_socket.label = HomeWizard Energysocket
+thing-type.homewizard.energy_socket.description = This thing provides the measurement data that is available through the http interface of a HomeWizard Energysocket.


This seems duplicated from the next two lines?

Result of duplice thing declaration, fixed.

jlaur · 2024-09-28T11:16:36Z

bundles/org.openhab.binding.homewizard/src/main/resources/OH-INF/i18n/homewizard.properties

+thing-type.config.homewizard.energy_socket.ipAddress.label = Network Address
+thing-type.config.homewizard.energy_socket.ipAddress.description = The IP or host name of the Energysocket.


Duplicated.

Result of duplice thing declaration, fixed.

jlaur · 2024-09-28T11:16:47Z

bundles/org.openhab.binding.homewizard/src/main/resources/OH-INF/i18n/homewizard.properties

+thing-type.config.homewizard.energy_socket.refreshDelay.label = Refresh Interval
+thing-type.config.homewizard.energy_socket.refreshDelay.description = The refresh interval in seconds for polling the Energysocket.


Duplicated.

Result of duplice thing declaration, fixed.

jlaur · 2024-09-28T11:20:35Z

bundles/org.openhab.binding.homewizard/src/main/resources/OH-INF/i18n/homewizard.properties

+thing-type.config.homewizard.watermeter.ipAddress.label = Network Address
+thing-type.config.homewizard.watermeter.ipAddress.description = The IP or host name of the Watermeter.
+thing-type.config.homewizard.watermeter.refreshDelay.label = Refresh Interval
+thing-type.config.homewizard.watermeter.refreshDelay.description = The refresh interval in seconds for polling the Watermeter.


Duplicated.

Result of duplice thing declaration, fixed.

jlaur · 2024-09-28T11:27:14Z

bundles/org.openhab.binding.homewizard/src/main/resources/OH-INF/thing/thing-types.xml

+				<label>Active Voltage L3</label>
+			</channel>
+
+			<channel id="any_power_failures" typeId="power_failures"/>


The word "any" gives me the impression that it's a boolean value, but actually it's the count of power failures (long or short), if I understand it correctly. Perhaps the name could be rephrased to better express this? Perhaps power_failure_count and long_power_failure_count, respectively? Or simply power_failures and long_power_failures (which is already consistent with channel below)?

yes, thanks, that is much better. When building up a model from the API, one tend to re-use those conventions.

jlaur · 2024-09-28T11:29:19Z

bundles/org.openhab.binding.homewizard/src/main/resources/OH-INF/thing/thing-types.xml

@@ -98,6 +131,68 @@

 	</thing-type>

+	<thing-type id="energy_socket">


This thing type is already defined in line 78?

Merge conflicts leftover. Fixed.

jlaur · 2024-09-28T11:30:03Z

bundles/org.openhab.binding.homewizard/src/main/resources/OH-INF/thing/thing-types.xml

+
+	</thing-type>
+
+	<thing-type id="watermeter">


This thing type is already defined in line 109?

Merge conflicts leftover. Fixed.

bundles/org.openhab.binding.homewizard/src/main/resources/OH-INF/update/instructions.xml

Signed-off-by: Leo Siepel <[email protected]>

jlaur · 2024-09-28T21:44:59Z

.../src/main/java/org/openhab/binding/homewizard/internal/handler/HomeWizardP1MeterHandler.java

     */
-    public HomeWizardP1MeterHandler(Thing thing) {
+    public HomeWizardP1MeterHandler(Thing thing, ZoneId zoneId) {


It would be better to store TimeZoneProvider here in order to call timeZoneProvider.getTimeZone() at the moment when creating ZonedDateTimes. This way changes in time-zone will automatically have effect immediately.

Yep, totally true.

bundles/org.openhab.binding.homewizard/src/main/resources/OH-INF/thing/thing-types.xml

Signed-off-by: Leo Siepel <[email protected]>

jlaur

Last two comments, otherwise LGTM.

bundles/org.openhab.binding.homewizard/README.md

jlaur · 2024-09-29T18:36:36Z

.../src/main/java/org/openhab/binding/homewizard/internal/handler/HomeWizardP1MeterHandler.java

+            logger.warn("Unable to parse Gas timestamp: {}", e.getMessage());
+            gasTimestamp = null;
+        }
+        if (gasTimestamp != null) {


Just checking, it shouldn't be cleared (UNDEF) otherwise?

I would prefer to leave it as close to the original behaviour as possible.

Co-authored-by: Jacob Laursen <[email protected]> Signed-off-by: lsiepel <[email protected]>

lsiepel added 2 commits July 4, 2024 22:36

Add channels

6a18acd

Signed-off-by: Leo Siepel <[email protected]>

Adjust ordering

0f1a4c4

Signed-off-by: Leo Siepel <[email protected]>

lsiepel added enhancement An enhancement or new feature for an existing add-on additional testing preferred The change works for the pull request author. A test from someone else is preferred though. labels Jul 4, 2024

lsiepel requested a review from wborn July 4, 2024 20:56

lsiepel added 3 commits July 7, 2024 21:14

Add two powerfailure channels

0f500b2

Signed-off-by: Leo Siepel <[email protected]>

Merge branch 'openhab:main' into homewizard-endpoint

5710ace

Spotless

61d834d

Signed-off-by: Leo Siepel <[email protected]>

lsiepel changed the title ~~[homewizard] Add current and voltage channels~~ [homewizard] Add current, voltage and failure channels Jul 8, 2024

lsiepel added 2 commits July 12, 2024 21:43

Add tests

902411c

Signed-off-by: Leo Siepel <[email protected]>

Final adjustments

3c89661

Signed-off-by: Leo Siepel <[email protected]>

lsiepel removed the additional testing preferred The change works for the pull request author. A test from someone else is preferred though. label Jul 12, 2024

lsiepel requested review from a team and removed request for wborn July 12, 2024 19:55

Different solution to extend/mock handler

1b73f1c

Signed-off-by: Leo Siepel <[email protected]>

lsiepel added 2 commits August 28, 2024 17:12

Merge remote-tracking branch 'upstream/main' into homewizard-endpoint

c72d7aa

Corrections after merge

9a8b656

Signed-off-by: Leo Siepel <[email protected]>

lsiepel marked this pull request as draft August 28, 2024 15:45

lsiepel added 2 commits August 28, 2024 20:59

Final adjustments after merge

50d8412

Signed-off-by: Leo Siepel <[email protected]>

Fix visibility

b6482c6

Signed-off-by: Leo Siepel <[email protected]>

lsiepel marked this pull request as ready for review August 28, 2024 19:09

lsiepel added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Aug 28, 2024

Merge branch 'main' into homewizard-endpoint

44f9717

jlaur requested changes Sep 28, 2024

View reviewed changes

Review comments

bb0bfab

Signed-off-by: Leo Siepel <[email protected]>

lsiepel requested a review from jlaur September 28, 2024 13:25

Spotless

0c394b5

Signed-off-by: Leo Siepel <[email protected]>

jlaur reviewed Sep 28, 2024

View reviewed changes

bundles/org.openhab.binding.homewizard/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved

lsiepel added 2 commits September 29, 2024 14:01

Adjust label - review comment

02d62ab

Signed-off-by: Leo Siepel <[email protected]>

Adjust timezoneprovider

f68bf70

Signed-off-by: Leo Siepel <[email protected]>

jlaur reviewed Sep 29, 2024

View reviewed changes

Update bundles/org.openhab.binding.homewizard/README.md

402b683

Co-authored-by: Jacob Laursen <[email protected]> Signed-off-by: lsiepel <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[homewizard] Add current, voltage and failure channels #16995

[homewizard] Add current, voltage and failure channels #16995

lsiepel commented Jul 4, 2024 •

edited

Loading

lsiepel commented Aug 24, 2024

lsiepel commented Aug 28, 2024

jlaur left a comment

jlaur Sep 28, 2024

lsiepel Sep 28, 2024

jlaur Sep 28, 2024

lsiepel Sep 28, 2024

jlaur Sep 28, 2024

lsiepel Sep 28, 2024

jlaur Sep 28, 2024

lsiepel Sep 28, 2024

jlaur Sep 28, 2024

lsiepel Sep 28, 2024

jlaur Sep 28, 2024

lsiepel Sep 28, 2024

jlaur Sep 28, 2024

lsiepel Sep 28, 2024

jlaur Sep 28, 2024

lsiepel Sep 28, 2024

jlaur Sep 28, 2024

lsiepel Sep 29, 2024

jlaur left a comment

jlaur Sep 29, 2024

lsiepel Sep 30, 2024

		thing-type.homewizard.energy_socket.label = HomeWizard Energysocket
		thing-type.homewizard.energy_socket.description = This thing provides the measurement data that is available through the http interface of a HomeWizard Energysocket.

		thing-type.config.homewizard.energy_socket.ipAddress.label = Network Address
		thing-type.config.homewizard.energy_socket.ipAddress.description = The IP or host name of the Energysocket.

		@@ -98,6 +131,68 @@

		</thing-type>

		<thing-type id="energy_socket">

[homewizard] Add current, voltage and failure channels #16995

Are you sure you want to change the base?

[homewizard] Add current, voltage and failure channels #16995

Conversation

lsiepel commented Jul 4, 2024 • edited Loading

lsiepel commented Aug 24, 2024

lsiepel commented Aug 28, 2024

jlaur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlaur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lsiepel commented Jul 4, 2024 •

edited

Loading