[siemenshvac] Initial contribution - interface (OZW672) #14263

lo92fr · 2023-01-22T19:28:05Z

Title

Add a binding for siemens Hvac interface (OZW672)

Description

This pull request is about adding a binding for siemens Hvac interface OZW672 (https://hit.sbt.siemens.com/RWD/app.aspx?rc=FR&lang=fr&module=Catalog&action=ShowProduct&key=BPZ:OZW672.01)

The OZW672 is a ethernet bridge for the LSB bus of Hvac device.
It implements a web interface that enable reading and setting of differents hvac parameters.
OZW672 also support a REST api.

This binding use this REST api to integrate OZW672 into openhab.

Testing

The binding support autodiscovery of the OZW672 interface using upnp.
After a few minutes, you should see the OZW672 bridge into your Inbox.
If not, power off and up OZW672.
You can also add the bridge manually.

After adding the bridge, you will have to go to bridge parameters, setting the password for the OZW672, and then restart openhab.
On restart, OZW672 will do autodiscovery phase from you installation during initialization.
This can be a little long : ~2 to 5 minutes depending of your installation.

After OZW672 bridge come up again, go back again to the "things" page of openhab.
Click on the add button, then click on the "siemensHvac" binding, and on the next page click on the "scan" button.
After a few second, openhab should list all you devices connect to the LSB bus.
If you have a simple home Hvac installation, it will generally find only one device.
Most common name for this device are something like "RVS41xxx.yyy".
This device represents the main controler inside your Hvac device.

After you add this RVS to your system, you can go into it, and Channels tabs will expose all your hvac settings.
Structure of the Channels should be the same as the one on the LCD screen of your Hvac device.
You may have a lot more of items that on your LCD screen because the device exposte all items, even the one for "specialist" guys.

lo92fr · 2023-01-22T19:51:42Z

Hello,

Not sure to understand the error message on the build.
The compilation is working on my side, if you can give me some advice about this error.

lsiepel · 2023-01-23T20:31:48Z

Hope the rebuild helps, don't see anything obvious wrong with the POM etc. You might want to look at the readme.md and the i18n files as they seem to be just the placeholders.

clinique

Partial review

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

...ain/java/org/openhab/binding/siemenshvac/internal/Metadata/SiemensHvacMetadataDataPoint.java

bundles/org.openhab.binding.siemenshvac/src/main/resources/OH-INF/binding/binding.xml

...ain/java/org/openhab/binding/siemenshvac/internal/type/SiemensHvacThingTypeProviderImpl.java

...n/java/org/openhab/binding/siemenshvac/internal/type/SiemensHvacChannelTypeProviderImpl.java

...a/org/openhab/binding/siemenshvac/internal/type/SiemensHvacChannelGroupTypeProviderImpl.java

...c/main/java/org/openhab/binding/siemenshvac/internal/network/SiemensHvacRequestListener.java

clinique

Partial review.

lo92fr · 2023-01-24T18:22:59Z

Hello Gael,

Thanks for your review.
I've just updated the pull request with the requested modifications.

Laurent.

lo92fr · 2023-01-24T18:57:50Z

I also manage to fix the build.
Was missing a reference in bom/openhab-addons/pom.xml.

Laurent.

clinique

Second review

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

...c/main/java/org/openhab/binding/siemenshvac/internal/Metadata/RuntimeTypeAdapterFactory.java

...ain/java/org/openhab/binding/siemenshvac/internal/Metadata/SiemensHvacMetadataDataPoint.java

...src/main/java/org/openhab/binding/siemenshvac/internal/network/SiemensHvacConnectorImpl.java

...a/org/openhab/binding/siemenshvac/internal/type/SiemensHvacChannelGroupTypeProviderImpl.java

...n/java/org/openhab/binding/siemenshvac/internal/type/SiemensHvacChannelTypeProviderImpl.java

.../org.openhab.binding.siemenshvac/src/main/resources/OH-INF/i18n/siemenshvac_xx_XX.properties

freichen · 2023-04-04T03:01:02Z

@lo92fr Amazing binding - thank you ! I tested it with an OZW672.01 connected to a RVP350 and the only issue I found was that Enumeration channels are auto-discovered as Number - for example:

lo92fr · 2023-04-16T07:31:05Z

hannels are auto-discovered as Number - for example:

Hello Freichen,

Not sure what are the good way to fix this.
Do you have some guidlines, or some other example binding I can look to fix this.

lo92fr · 2023-04-16T07:32:18Z

Hello Gaël,

@clinique : I'm not sure what are the current state of this pull request.
Do you wait more change from my side ?

larnalcegid · 2023-04-28T16:17:38Z

Hello,

Can some one can give an update about the integration of this pull request in main stream.

Thanks,
Laurent.

larnalcegid · 2023-04-28T16:18:56Z

@lo92fr Amazing binding - thank you ! I tested it with an OZW672.01 connected to a RVP350 and the only issue I found was that Enumeration channels are auto-discovered as Number - for example:

I've commit a fix for this, should be working as excepted now.

larnalcegid · 2023-05-18T05:41:56Z

Hello Gaël, Hello Freichen,

Any update on this ?
Let me know if any more changed is need.

Laurent.

Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr · 2024-05-14T11:00:52Z

Hello Jacob (@jlaur ),

I commit a bunch of fixes this morning about your last review.
I also see your question about screenshoot f the discovered things.
So I've put some screenshoot and explanation on your comment.
Tell me if it's sufficient, or if you're need more explanation.

Laurent.

Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr · 2024-05-14T12:37:16Z

Freichen

Hello @freichen,

I don't know if you see my last message (#14263 (comment)).
Do you have the time to verify if removing the json fix the issue ?

Thanks,
Laurent.

Signed-off-by: Laurent ARNAL <laurent@clae.net>

jlaur · 2024-05-14T13:36:42Z

...inding.siemenshvac/src/main/java/org/openhab/binding/siemenshvac/internal/type/UidUtils.java

+            result = result.replaceAll("\\p{M}", "");
+        }
+
+        result = result.replaceAll("[^a-zA-Z0-9_]", "_").toLowerCase();


I might have made a mistake in a previous suggestion based on the logic being replaced rather than the naming convention:

Suggested change

result = result.replaceAll("[^a-zA-Z0-9_]", "_").toLowerCase();

result = result.replaceAll("[^a-zA-Z0-9-]", "-").toLowerCase();

(not sure if "-" needs to be escaped, i.e. "[^a-zA-Z0-9\-]")

jlaur · 2024-05-15T07:31:01Z

bundles/org.openhab.binding.siemenshvac/src/main/resources/OH-INF/thing/ozw672.xml

+	xsi:schemaLocation="https://openhab.org/schemas/thing-description/v1.0.0 https://openhab.org/schemas/thing-description-1.0.0.xsd">
+
+	<!-- Bridge Thing Type -->
+	<bridge-type id="ozw672">


I was thinking about #14263 (comment) on my way to work and to provide some additional context. I then came also to think about the bridge type id. If the binding also works with OZW772.x, or might be brought to work with that as well, probably this id is too specialized. I'm thinking about two possible scenarios:

OZW772.x is very different code-wise and a new bridge type would have to be introduced.

OZW772.x is very similar code-wise, and it can be tweaked to work with the same bridge type, perhaps with some different configuration.

If actually it is expected to be similar, it would be strange to create a bridge type ozw672 for a OZW772.

My proposal in this case would be to rename to a more generic name, perhaps the common denominator "ozw"?

I'm not really sure on that, as I don't have documentation about Ozw772, and don't have access to this physical device to do some test. But I except that the code should be identical between the two devices.

Today, in the binding, the only specific part is in bridge initialization / discovery where we test the modelName, and initialize an OZW672. Notes that in current configuration, we will not currently support Ozw772 because we don't have an SiemensHvacOZW772BridgeThingHandler override the SiemensHvacBridgeBaseThingHandler, and so we have no way to init the addon for this device.

So I think we can go two way :

Scenario 1: Keep like this for first version of the addon.

Scenario 2: extends the discovering and bridge initialization to handle OZW772, adding an SiemensHvacOZW772BridgeThingHandler override, and see if someone has the device to do some testing.

It sounds like you would expect OZW772 to be similar rather than different. The implementation and support can always be changed, but the modelling cannot without breaking backwards compatibility.

So I would propose to keep the current implementation, but generalize the bridge type id: ozw672 → ozw. I think in the long run it will then be easier to add support for OZW772 with less duplication (i.e. separate bridge definitions, handlers, etc.). It won't hurt to also rename SiemensHvacOZW772BridgeThingHandler etc., but strictly it's not necessary as it can always be refactored without breaking anything.

...src/main/java/org/openhab/binding/siemenshvac/internal/network/SiemensHvacConnectorImpl.java

...java/org/openhab/binding/siemenshvac/internal/handler/SiemensHvacBridgeBaseThingHandler.java

jlaur · 2024-05-15T10:17:30Z

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

+offline.waiting-bridge-initialization = Waiting bridge initialization, reading metadata in background
+
+offline.baseurl-mandatory = baseUrl is mandatory on configuration !
+offline.error-gateway-init = Error occurs during gateway initialization: %s


Suggested change

offline.error-gateway-init = Error occurs during gateway initialization: %s

offline.error-gateway-init = Error occurred during gateway initialization: {0}

fix on next commit

One space too much slipped in between the words "occurred" and "during".

fix on next commit

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

...vac/src/main/java/org/openhab/binding/siemenshvac/internal/network/SiemensHvacConnector.java

.../java/org/openhab/binding/siemenshvac/internal/metadata/SiemensHvacMetadataRegistryImpl.java

jlaur · 2024-05-15T21:35:44Z

...main/java/org/openhab/binding/siemenshvac/internal/converter/type/DateTimeTypeConverter.java

+                    }
+
+                    LocalDateTime parsedDate = LocalDateTime.parse(value.getAsString(),
+                            formatterBuilder.toFormatter(Locale.FRENCH));


Is this product only for the French market, or why do you need hardcoded French Locale here? Can you configure the OZW webserver language - if yes, how will such a change affect this method?

No it's a mistake, the product is international.

And you're right, changing the language on gateway side will influenced the result.
Mainly on formatter like this one "EEEE, d. LLLL yyyy HH:mm[:ss]" where the first part EEEE will be localized regarding the language setting of the gateway.

I take a look on how i can Handle this a better way

I've modify the code of TypeConverter to add the Local information as a parameter of fromBinding function.
This information is take from the MetadataRegistry which already contains the current user used to connect the API.
I handle there the language retrivial, and locale initialization.

Current gateway is supporting a bunch of different language: English,Deutsch,Francais,Italiano,Nederlands,Polski,Cesky,Magyar,Espanol,Dansk,Svenska,Suomi,Portugues,Russkij,Turkce,Slovensky;

I will certainly need to do more test to verify that formatter are ok for all this language.

Wow, that's a lot of languages to support. This reminds me of writing an integration for the old Miele@home Homebus API where essential information was provided as localized strings. It was a tremendous work to support that for the 4-5 languages supported by the gateway.

You might want to add to the README that depending on the configured language, there might be some issues which can then be reported (I suppose). You might want to prioritize to fully test and support at least a few of those languages besides French, for example English and German (considering the home of both Siemens and openHAB 😄).

lo92fr · 2024-05-31T08:26:29Z

Hello @jlaur,

I do a first pass on your review of May 15.
Sorry for the delay, but I was off for two weeks because of familly affair.

I still have to review the point about unit handling (your comment "For UoM support you would need to add dimension, i.e.").
I will try to fix this this morning.

Laurent.

Signed-off-by: Laurent ARNAL <laurent@clae.net>

fix locale initialization handling review dimension initialization for NumberType Fix TimeOfDay converter Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr · 2024-06-02T09:50:20Z

Hello @jlaur,

I push the modification to handle UoM this morning.
After doing some more in depth test, I also realize that dimension initialization in MetadataRegistry was not properly handle.
I also fix this, and a few other things.

The addon now support the following dimensions on number:
Number:Temperature
Number:ElectricPotential
Number:Dimensionless
Number:Time

Laurent.

Signed-off-by: Laurent ARNAL <laurent@clae.net>

jlaur · 2024-06-02T19:28:23Z

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

+
+Mainly, it will first discover the gateway.
+Currently test was done with the OZW672.x series.
+No test was conduct using the OZW772.x series, the code will currently not handing initialization of OZW772 gateway.


Suggested change

No test was conduct using the OZW772.x series, the code will currently not handing initialization of OZW772 gateway.

No test was conducted using the OZW772.x series, the code will currently not handle initialization of an OZW772 gateway.

fix on next commit

jlaur · 2024-06-02T19:29:04Z

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

+Mainly, it will first discover the gateway.
+Currently test was done with the OZW672.x series.
+No test was conduct using the OZW772.x series, the code will currently not handing initialization of OZW772 gateway.
+You can request support to the community forum is you have the gateway model and want it to be supported.


Suggested change

You can request support to the community forum is you have the gateway model and want it to be supported.

You can request support in the community forum, if you have the gateway model and want it to be supported.

fix on next commit

jlaur · 2024-06-02T19:30:52Z

Sorry for the delay, but I was off for two weeks because of familly affair.

No worries, family is first priority. I will try to catch up in the next days.

I push the modification to handle UoM this morning.

Awesome!

Signed-off-by: Laurent ARNAL <laurent@clae.net>

fix initialization code to support OZW772 Signed-off-by: Laurent ARNAL <laurent@clae.net>

Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr requested a review from a team as a code owner January 22, 2023 19:28

lo92fr force-pushed the 14262-siemensHvac branch 2 times, most recently from dc1076f to 737f2f3 Compare January 22, 2023 19:41

lsiepel changed the title ~~Add a binding for siemens Hvac interface (OZW672)~~ [siemenshvac] Initial contribution - interface (OZW672) Jan 22, 2023

lsiepel added new binding If someone has started to work on a binding. For a new binding PR. rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 22, 2023

lsiepel linked an issue Jan 24, 2023 that may be closed by this pull request

Add a binding for siemens Hvac interface (OZW672) #14262

Open

clinique requested changes Jan 24, 2023

View reviewed changes

lo92fr force-pushed the 14262-siemensHvac branch 2 times, most recently from bc727ff to 66445a4 Compare January 24, 2023 18:50

clinique requested changes Jan 25, 2023

View reviewed changes

lo92fr force-pushed the 14262-siemensHvac branch from 2b83f89 to 0c8f02f Compare February 4, 2023 13:57

lo92fr force-pushed the 14262-siemensHvac branch from 0c8f02f to da5d4b8 Compare February 11, 2023 17:04

lo92fr requested a review from clinique February 11, 2023 17:07

lo92fr force-pushed the 14262-siemensHvac branch from 81d5e7d to 992ebe1 Compare April 16, 2023 11:00

larnalcegid approved these changes Apr 21, 2023

View reviewed changes

larnalcegid approved these changes Apr 28, 2023

View reviewed changes

fix code violations

5c1d1a6

Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr added 2 commits May 14, 2024 13:08

review upnp discovery

907373f

Signed-off-by: Laurent ARNAL <laurent@clae.net>

fix autodiscovery

b0957b7

Signed-off-by: Laurent ARNAL <laurent@clae.net>

change json cache file location

088cf37

Signed-off-by: Laurent ARNAL <laurent@clae.net>

jlaur reviewed May 14, 2024

View reviewed changes

jlaur reviewed May 15, 2024

View reviewed changes

...src/main/java/org/openhab/binding/siemenshvac/internal/network/SiemensHvacConnectorImpl.java Outdated Show resolved Hide resolved

jlaur reviewed May 15, 2024

View reviewed changes

...java/org/openhab/binding/siemenshvac/internal/handler/SiemensHvacBridgeBaseThingHandler.java Outdated Show resolved Hide resolved

jlaur reviewed May 15, 2024

View reviewed changes

bundles/org.openhab.binding.siemenshvac/src/main/resources/OH-INF/i18n/siemenshvac.properties Outdated Show resolved Hide resolved

jlaur reviewed May 15, 2024

View reviewed changes

...vac/src/main/java/org/openhab/binding/siemenshvac/internal/network/SiemensHvacConnector.java Outdated Show resolved Hide resolved

jlaur reviewed May 15, 2024

View reviewed changes

.../java/org/openhab/binding/siemenshvac/internal/metadata/SiemensHvacMetadataRegistryImpl.java Outdated Show resolved Hide resolved

jlaur reviewed May 15, 2024

View reviewed changes

lo92fr requested a review from jlaur May 31, 2024 08:26

lo92fr added 2 commits May 31, 2024 10:39

first pass on May 15 review from Jacob

defe62a

Signed-off-by: Laurent ARNAL <laurent@clae.net>

fix some imprecision on gateway model handle by the addon

06ce157

Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr force-pushed the 14262-siemensHvac branch from b1eea2e to 06ce157 Compare May 31, 2024 08:39

Add support for UoM

7424dee

fix locale initialization handling review dimension initialization for NumberType Fix TimeOfDay converter Signed-off-by: Laurent ARNAL <laurent@clae.net>

lo92fr added 2 commits June 2, 2024 12:01

review style checker warnings

76d5caa

Signed-off-by: Laurent ARNAL <laurent@clae.net>

fix typo

9b52226

Signed-off-by: Laurent ARNAL <laurent@clae.net>

jlaur reviewed Jun 2, 2024

View reviewed changes

lo92fr added 3 commits June 3, 2024 09:07

few typo fixes on last jlaur review

a5455ef

Signed-off-by: Laurent ARNAL <laurent@clae.net>

generalized bridge naming

8f78df2

fix initialization code to support OZW772 Signed-off-by: Laurent ARNAL <laurent@clae.net>

spotless:²apply

6bb1165

Signed-off-by: Laurent ARNAL <laurent@clae.net>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[siemenshvac] Initial contribution - interface (OZW672) #14263

[siemenshvac] Initial contribution - interface (OZW672) #14263

lo92fr commented Jan 22, 2023

lo92fr commented Jan 22, 2023

lsiepel commented Jan 23, 2023

clinique left a comment

clinique left a comment

lo92fr commented Jan 24, 2023

lo92fr commented Jan 24, 2023

clinique left a comment

freichen commented Apr 4, 2023

lo92fr commented Apr 16, 2023

lo92fr commented Apr 16, 2023

larnalcegid commented Apr 28, 2023

larnalcegid commented Apr 28, 2023

larnalcegid commented May 18, 2023

lo92fr commented May 14, 2024

lo92fr commented May 14, 2024

jlaur May 14, 2024 •

edited

jlaur May 15, 2024

lo92fr May 31, 2024

jlaur Jun 2, 2024

jlaur May 15, 2024

lo92fr May 31, 2024

jlaur Jun 2, 2024

lo92fr Jun 3, 2024

jlaur May 15, 2024

lo92fr May 31, 2024

lo92fr May 31, 2024

jlaur Jun 2, 2024

lo92fr commented May 31, 2024

lo92fr commented Jun 2, 2024

jlaur Jun 2, 2024

lo92fr Jun 3, 2024

jlaur Jun 2, 2024

lo92fr Jun 3, 2024

jlaur commented Jun 2, 2024

	result = result.replaceAll("[^a-zA-Z0-9_]", "_").toLowerCase();
	result = result.replaceAll("[^a-zA-Z0-9-]", "-").toLowerCase();

	offline.error-gateway-init = Error occurs during gateway initialization: %s
	offline.error-gateway-init = Error occurred during gateway initialization: {0}

	No test was conduct using the OZW772.x series, the code will currently not handing initialization of OZW772 gateway.
	No test was conducted using the OZW772.x series, the code will currently not handle initialization of an OZW772 gateway.

	You can request support to the community forum is you have the gateway model and want it to be supported.
	You can request support in the community forum, if you have the gateway model and want it to be supported.

[siemenshvac] Initial contribution - interface (OZW672) #14263

Are you sure you want to change the base?

[siemenshvac] Initial contribution - interface (OZW672) #14263

Conversation

lo92fr commented Jan 22, 2023

Title

Description

Testing

lo92fr commented Jan 22, 2023

lsiepel commented Jan 23, 2023

clinique left a comment

Choose a reason for hiding this comment

clinique left a comment

Choose a reason for hiding this comment

lo92fr commented Jan 24, 2023

lo92fr commented Jan 24, 2023

clinique left a comment

Choose a reason for hiding this comment

freichen commented Apr 4, 2023

lo92fr commented Apr 16, 2023

lo92fr commented Apr 16, 2023

larnalcegid commented Apr 28, 2023

larnalcegid commented Apr 28, 2023

larnalcegid commented May 18, 2023

lo92fr commented May 14, 2024

lo92fr commented May 14, 2024

jlaur May 14, 2024 • edited

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

lo92fr commented May 31, 2024

lo92fr commented Jun 2, 2024

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 commented Jun 2, 2024

jlaur May 14, 2024 •

edited