Freebox binding: fix initialization and add thing properties #1127

lolodomo · 2016-07-15T19:17:26Z

Signed-off-by: Laurent Garnier lg.hc@free.fr

lolodomo · 2016-07-15T19:21:38Z

This PR implements #1113.
IAnd it fixed initialization process that was failing in recent snapshots.

kaikreuzer · 2016-07-16T09:37:19Z

addons/binding/org.openhab.binding.freebox/ESH-INF/thing/thing-types.xml

@@ -34,6 +34,15 @@
 			<channel id="bytes_down" typeId="bytes_down" />
 		</channels>

+		<properties>
+			<property name="vendor">Freebox SAS</property>
+			<property name="serialNumber"></property>


I do not like this - properties should not have an empty string as value. Either they are set or not.

You mean I should only declare properties for which I already know the value ?
The others would just be added dynamically at runtime ?

Yes, exactly.

Ok, will do the change.

lolodomo · 2016-07-16T22:52:03Z

I suppressed the empty properties from thing-types.xml.
And I updated the documentation (I forgot to do it in my initial PR).

kaikreuzer · 2016-07-17T12:17:17Z

...ing.freebox/src/main/java/org/openhab/binding/freebox/discovery/FreeboxDiscoveryService.java

@@ -99,6 +100,9 @@ public void onDataFetched(ThingUID bridge, LanHostsConfig hostsConfig) {
                    if (thingUID != null) {
                        logger.trace("Adding new Freebox Network Device {} to inbox", thingUID);
                        Map<String, Object> properties = new HashMap<>(1);
+                        if (!hostConfig.getVendorName().isEmpty()) {


The vendor is always set, because you define it in the thing type, don't you?
Otherwise, I would expect that hostConfig.getVendorName() can be null, which would result in an NPE. Or am I missing something?

No the vendor property I set in thing-types.xml is for the bridge thing. Here I set the vendor property on other things (network devices).

I will check again but I think getVendorName(l never returns null.

kaikreuzer · 2016-07-17T12:19:54Z

@clinique As this contains quite some changes, could you please review this PR?

kaikreuzer · 2016-07-17T12:24:44Z

...x/src/main/java/org/openhab/binding/freebox/discovery/FreeboxServerDiscoveryParticipant.java

@@ -71,12 +72,15 @@ public DiscoveryResult createResult(ServiceInfo service) {
        if (thingUID != null && ip != null) {
            logger.info("Created a DiscoveryResult for Freebox Server {} on IP {}", thingUID, ip);
            Map<String, Object> properties = new HashMap<>(1);
-            properties.put(FreeboxServerConfiguration.IP_ADDRESS, ip);
+            properties.put(FreeboxServerConfiguration.IP_ADDRESS, ip + ":" + service.getPort());


@clinique All other bindings have the IP and port as separate config parameters - could that be applied for freebox as well?

In fact this parameter contains a FQDN, it can be 192.168.0.254:80 but it can be mafreebox.free.fr.
Maybe the name of the parameter could have been different.

Yes, I think the property name is then misleading, it would propably be better to name it FQDN

lolodomo · 2016-07-23T06:35:01Z

My PR is now updated and take into account your comments.

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

lolodomo · 2016-07-23T06:55:03Z

The build seems to fail on compilation of harmony hub binding.

08:51:48.556  |  [ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:0.24.0:compile (default-compile) on project org.openhab.binding.harmonyhub: Compilation failure: Compilation failure:
08:51:48.556  |  [ERROR] /scratch/jenkins/workspace/PR-openHAB2-Addons/addons/binding/org.openhab.binding.harmonyhub/src/main/java/org/openhab/binding/harmonyhub/handler/HarmonyHubHandler.java:[274]
08:51:48.556  |  [ERROR] BridgeBuilder thingBuilder = BridgeBuilder.create(getThing().getUID())
08:51:48.556  |  [ERROR] .withConfiguration(getThing().getConfiguration()).withProperties(getThing().getProperties());
08:51:48.556  |  [ERROR] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
08:51:48.557  |  [ERROR] Type mismatch: cannot convert from ThingBuilder to BridgeBuilder
08:51:48.557  |  [ERROR] 1 problem (1 error)

This PR only impacts the Freebox binding so the build was certainly broken by another PR ?

kaikreuzer · 2016-07-23T07:21:31Z

The harmony error should be fixed by https://github.com/openhab/openhab2-addons/pull/1146

lolodomo · 2016-07-23T07:26:01Z

Ok. I can force a new push to trigger a new build of this PR i you wish ?

* master: (44 commits) ZWave fix Barrier Operator converter (openhab#1155) Added Groovy Eclipse compiler to master pom. (openhab#1141) added HD Powerview binding to build Initial contribution of HD PowerView binding (openhab#660) ZWave database update (openhab#1153) added Onkyo binding to build initial version of a Onkyo binding (openhab#768) Fix initialization and add thing properties (openhab#1127) ZWave: Add extra debugging when updating properties. (openhab#1147) ZWave database update (openhab#1148) fixed compilation error due to changed ESH API (openhab#1146) ZWave database updates (openhab#1145) Add example things definition (openhab#1144) ignore commands other than ON/OFF applied code formatter Meteostick binding (openhab#455) Fix for multiple tags and HomeKit (openhab#1138) ZWave database update (openhab#1136) Fix wording and typos. Alphabetise supported channel list. (openhab#1134) ZWave database updates (openhab#1132) ... Signed-off-by: Chris Jackson <chris@cd-jackson.com> # Conflicts: # addons/binding/org.openhab.binding.zwave/src/main/java/org/openhab/binding/zwave/handler/ZWaveThingHandler.java

* master: (257 commits) Delete pom.xml.orig Update README.md Remove ZWave binding (openhab#1169) Update README.md (openhab#1168) ZWave check that serial port is open before sending data (openhab#1167) ZWave change polling to use BigDecimal rather than Integer (openhab#1166) ZWave fix processing of alarm supported report (openhab#1165) ZWave add network update method (openhab#1162) Zwave association refactor and consolidation (openhab#893) Update README.md (openhab#1159) ZWave database update (openhab#1161) ZWave fix Barrier Operator converter (openhab#1155) Added Groovy Eclipse compiler to master pom. (openhab#1141) added HD Powerview binding to build Initial contribution of HD PowerView binding (openhab#660) ZWave database update (openhab#1153) added Onkyo binding to build initial version of a Onkyo binding (openhab#768) Fix initialization and add thing properties (openhab#1127) ZWave: Add extra debugging when updating properties. (openhab#1147) ... # Conflicts: # addons/binding/pom.xml

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Mockito 3 does not introduce any breaking API changes, but now requires Java 8 over Java 6 for Mockito 2. Signed-off-by: Wouter Born <github@maindrain.net>

lolodomo changed the title ~~Fix initialization and add thing properties~~ Freebox binding: fix initialization and add thing properties Jul 15, 2016

kaikreuzer reviewed Jul 16, 2016
View reviewed changes

kaikreuzer reviewed Jul 17, 2016
View reviewed changes

kaikreuzer added awaiting feedback Awaiting feedback from the pull request author enhancement An enhancement or new feature for an existing add-on labels Jul 20, 2016

Fix initialization and add thing properties

b24dc59

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

kaikreuzer removed the awaiting feedback Awaiting feedback from the pull request author label Jul 25, 2016

kaikreuzer merged commit 57d7d1e into openhab:master Jul 25, 2016

kaikreuzer added this to the 2.0.0.b4 milestone Jul 25, 2016

lolodomo deleted the freebox_properties branch July 25, 2016 09:03

lolodomo mentioned this pull request Jul 25, 2016

Freebox binding: use properties #1113

Closed

lolodomo mentioned this pull request Aug 1, 2016

Freebox binding: bridge thing remaining sometimes in INITIALIZING state #1170

Closed

doubled-ca pushed a commit to doubled-ca/openhab-addons that referenced this pull request Aug 14, 2016

Fix initialization and add thing properties (openhab#1127)

e3a001a

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Freebox binding: fix initialization and add thing properties #1127

Freebox binding: fix initialization and add thing properties #1127

lolodomo commented Jul 15, 2016

lolodomo commented Jul 15, 2016

kaikreuzer Jul 16, 2016

lolodomo Jul 16, 2016

kaikreuzer Jul 16, 2016

lolodomo Jul 16, 2016

lolodomo commented Jul 16, 2016

kaikreuzer Jul 17, 2016

lolodomo Jul 17, 2016

kaikreuzer commented Jul 17, 2016

kaikreuzer Jul 17, 2016

lolodomo Jul 17, 2016

clinique Jul 18, 2016

lolodomo commented Jul 23, 2016

lolodomo commented Jul 23, 2016 •

edited

Loading

kaikreuzer commented Jul 23, 2016

lolodomo commented Jul 23, 2016 •

edited

Loading

Freebox binding: fix initialization and add thing properties #1127

Freebox binding: fix initialization and add thing properties #1127

Conversation

lolodomo commented Jul 15, 2016

lolodomo commented Jul 15, 2016

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

lolodomo commented Jul 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaikreuzer commented Jul 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lolodomo commented Jul 23, 2016

lolodomo commented Jul 23, 2016 • edited Loading

kaikreuzer commented Jul 23, 2016

lolodomo commented Jul 23, 2016 • edited Loading

lolodomo commented Jul 23, 2016 •

edited

Loading

lolodomo commented Jul 23, 2016 •

edited

Loading