[178] Support Discovery of USB dongles (for some Telegesis and one Ember dongle) #179

hsudbrock · 2018-04-12T11:36:15Z

Please note: This PR depends on the recently added USB-Serial discovery from Eclipse SmartHome (see eclipse-archived/smarthome#5315). In consequence, the build of this PR will fail until the new bundles for this discovery are contained in the stable p2 repository for Eclipse SmartHome, and this PR should not be merged before this has happened. I will add a comment to this PR once this has happened. For building the PR successfully now, replace the p2 repository URL in the ZigBee binding parent pom from https://openhab.jfrog.io/openhab/eclipse-smarthome-stable with http://download.eclipse.org/smarthome/updates-nightly.

This PR adds two discovery participants for the USB-serial discovery of Eclipse SmartHome, one for the telegesis bundle, and one for the ember bundle. These discovery participants can discover three Telegesis dongles (from the Qivicon brand) and one Ember dongle (from Bitron Video brand).

Discovery of USB dongles is held rather simple - the discovery participants check whether USB vendor and product ID match the vendor and product ID of a known ZigBee dongle. If yes, a discovery result is created using the dongle's serial number in the thing ID, and configuring the baud rate to a hard-coded baud rate with which the dongle is known to function.

I would be happy about any feedback on this PR!

Fixes #178.

cdjackson · 2018-04-12T12:41:54Z

Thanks Henning.

One question without having looked at the code (and given I probably can't test this as I think it only works on Linux) -:

and configuring the baud rate to a hard-coded baud rate with which the dongle is known to function.

I don't think there's any way to know this for sure - at least not without doing some sort of active scan? I guess we can't be sure it will function, so presumably the user can always change the baud rate to the value that they know works?

hsudbrock · 2018-04-12T14:35:01Z

I don't think there's any way to know this for sure - at least not without doing some sort of active scan?

Good question. I have tried this on different hardwares and Linuxes, and the baud rate with which the dongles worked was always the same. But there might be some device out there where a different baud rate is needed.

I guess we can't be sure it will function, so presumably the user can always change the baud rate to the value that they know works?

If the user adds the discovered device from the inbox, the thing will be configured with that baud rate, but the user should still be able to change the baud rate afterwards if the Eclipse SmartHome-based system permits this (as, e.g., via Paper UI). I have not tried this, though; I will do a test if the ZigBee binding actually supports a change of the baud rate after the thing has been created, and report the result as comment to this PR.

cdjackson · 2018-04-12T15:05:12Z

But there might be some device out there where a different baud rate is needed.

Yes, for sure this is the case. The Ember dongle will definitely require this as there are 2 common baud rates used, and other configuration parameters will also need to change. Namely, the flow control settings also change - I expect it's about 50/50 for the different configurations that are commonly used.

tomhoefer · 2018-04-12T18:23:42Z

...penhab/binding/zigbee/ember/internal/discovery/ZigBeeEmberUsbSerialDiscoveryParticipant.java

+
+    private static final Set<ThingTypeUID> SUPPORTED_THING_TYPES = new HashSet<>(asList(THING_TYPE_EMBER));
+
+    public static final int SILICON_LABS_USB_VENDOR_ID = 0x10c4;


just a minor comment - such constants have been documented for the Telegesis dongle

I commented only those constants for the Telegesis dongle where I found documentation useful (i.e., those constants where there are multiple different dongles which might or might not have the same product ID - having in mind that there are two dongles that look alike but have different product IDs, and two dongles that don't look alike but have the same product ID, which I wanted to clarify using those constants). Here, I don't think that additional documentation is needed.

tomhoefer · 2018-04-12T18:26:08Z

...inding/zigbee/telegesis/internal/discovery/ZigBeeTelegesisUsbSerialDiscoveryParticipant.java

+     * is used for the Telegesis ZigBee dongle model ETRX3USB+8M, as well as for one version of the QIVICON ZigBee stick
+     * (which is also produced by Telegesis).
+     */
+    public static final int QIVICON_DONGLE_V1_USB_PRODUCT_ID = 0x8293;


I am a bit confused - the black Telegesis dongle is also supported, isn´t it? If yes shouldnt we rename the prefix to Telegesis?

You are right - we could add TELEGESIS to the constant name (as this product ID is used both by one version of the Qivicon dongle, and by the dongle with Telegesis label). I will add this with my next commit to this PR.

tomhoefer · 2018-04-12T18:27:06Z

in general this PR looks good to me - just two additional comments

one question regarding the baud rate - if we use the "wrong" baud rate per default for a specific dongle this would always result in the situation that the bridge thing will not be online after discovery - is there an easy possibility to auto-configure this?

cdjackson

Looks fine - thanks. I can't easily test as I'm not running Linux, but I'm sure you've got that under control ;)

cdjackson · 2018-04-12T20:22:51Z

...penhab/binding/zigbee/ember/internal/discovery/ZigBeeEmberUsbSerialDiscoveryParticipant.java

+                    .withLabel(createBitronVideoDongleLabel(deviceInformation))
+                    .withRepresentationProperty(CONFIGURATION_PORT)
+                    .withProperty(CONFIGURATION_PORT, deviceInformation.getSerialPort())
+                    .withProperty(CONFIGURATION_BAUD, BITRON_VIDEO_2010_10_BAUD_RATE).build();


It may also be worth setting the flow control property as well (Should be XON/OFF for this version).

Ah, I wasn't aware of that option yet, thanks! I will add this with my next commit to this PR (I tested this setting with the Ember dongle, and it worked fine so far).

@cdjackson Do you have any infos on flow control for the telegesis dongles? I have looked at https://www.silabs.com/documents/public/reference-manuals/TG-PM-0518-ETRX357USB.pdf which mentions that nRTS and nCTS lines are connected to the USB bridge chip - from which I (as a layman) would deduce that hardware flow control could be used - or am I mistaken completely here?

I tested this setting with the Ember dongle, and it worked fine so far

I think the lower speed that is used in the XON/OFF flow control means that it's not a big issue this way around when running on a reasonably fast computer. But the other way around (ie using XON/OFF instead of CTS/RTS) can completely stop the dongle from working. This is what is used when the Ember runs at 115k2.

which mentions that nRTS and nCTS lines are connected to the USB bridge chip - from which I (as a layman) would deduce that hardware flow control could be used - or am I mistaken completely here?

I would agree with your assessment. Currently I think I don't use flow control. I think I read somewhere that it is optional, but the Telegesis runs at such a slow speed (19k2) that it shouldn't matter I think (except on a really slow computer!)

Thanks for explaining! I wil thenl leave it as is for now (i.e., use the default flow control setting for the telegesis discovery result, which is 'no flow control'). We can still change this if issues arise somewhere.

hsudbrock · 2018-04-13T12:09:48Z

I have tested changing the baud rate of a ZigBee coordinator after initial configuration using Paper UI - and it did not work in my test. It appears to me that this is not possible, because the method ZigBeeCoordinatorHandler#handleConfigurationUpdate ignores some configuration parameters (e.g., the serial port, the baud rate, the zigbee channel, the pan id, and several others).

As far as I see it, this would need to be changed before being able to change baud rate (and other config parameters) after initial thing creation. Or do I overlook something, @cdjackson ?

cdjackson · 2018-04-13T12:35:20Z

I would have thought that the handler should be reinitialised if the serial port config changes, but this doesn't sound like it's implemented. I'll add that separately.

hsudbrock · 2018-04-13T14:33:31Z

I woulsd have thought that the handler should be reinitialised if the serial port config changes, but this doesn't sound like it's implemented. I'll add that separately.

Thanks, @cdjackson !

hsudbrock · 2018-04-16T11:13:59Z

The USB-Serial discovery has now been deployed to https://openhab.jfrog.io/openhab/eclipse-smarthome-stable; this is, hence, no longer an impediment for merging this PR.

hsudbrock · 2018-04-17T07:00:25Z

I rebased this PR to be able to use the newly introduced flow control constants from #184; I added a commit for using these constants.

hsudbrock · 2018-04-19T12:23:03Z

Before merging this PR, #186 should be fixed (as otherwise one cannot change the baud rate of a thing created from a discovery result, and the baud rate from the discovery result might not always match the needed one as indicated by @cdjackson above).

cdjackson · 2018-04-19T18:17:03Z

You can change the baud rate etc - you ‘just’ need to restart the bundle (not nice I know, but it should work).

cdjackson · 2018-04-20T09:51:38Z

Looks like this needs rebasing then I guess it can be merged now that #187 is merged.

dongle) Adds discovery participants for the USB-serial discovery of Eclipse SmartHome, able to discover three Telegesis dongles (from the Qivicon brand) and one Ember dongle (from Bitron Video brand). Bug: openhab#178 Signed-off-by: Henning Sudbrock <[email protected]>

* Rename constant for USB product ID for one Qivicon/Telgesis USB dongle. * Add flow control configuration for Ember discovery result (to XONOFF)

participant Signed-off-by: Henning Sudbrock <[email protected]>

hsudbrock · 2018-04-21T10:39:57Z

I have rebased the PR, so this can be merged now.

cdjackson · 2018-04-21T10:59:31Z

Thanks @hsudbrock - @tomhoefer I guess you will merge?

tomhoefer · 2018-04-21T12:38:56Z

Thanks @cdjackson - I have no preference who is going to merge actually a PR

I will merge this PR now

kaikreuzer · 2018-04-21T19:37:45Z

Looks as if this broke the build: https://openhab.ci.cloudbees.com/job/openHAB-Distribution/1261/console
I guess we need an additional dependency in https://github.com/openhab/openhab-distro/blob/master/features/addons/src/main/feature/feature.xml#L6.

cdjackson · 2018-04-21T20:07:25Z

I thought this was meant to be part of ESH, and therefore included in OH core? Is it just that OH hasn’t been updated since this was merged into ESH?

hsudbrock · 2018-04-22T05:17:19Z

@kaikreuzer @cdjackson I think the reason is that - while the USB-Serial discovery is a part of ESH - it is not in the ESH-Karaf-feature esh-base, but in the ESH-Karaf-feature esh-config-discovery-usbserial; so the build breaks since I did not add the second feature into https://github.com/openhab/openhab-distro/blob/master/features/addons/src/main/feature/feature.xml.

I will create a PR now in the openhab-distro git repository to add this.

hsudbrock · 2018-04-22T05:59:21Z

@kaikreuzer @cdjackson I added a PR to the openhab-distro repository: openhab/openhab-distro#686
With this PR, feature verification for openhab-distro does not produce an error anymore (at least on my machine).

tomhoefer suggested changes Apr 12, 2018

View reviewed changes

cdjackson reviewed Apr 12, 2018

View reviewed changes

hsudbrock mentioned this pull request Apr 16, 2018

Introduce constants for the configuration values for flow control #183

Merged

hsudbrock force-pushed the 178-support-discovery-of-usb-dongles branch from 7350647 to 6d27491 Compare April 17, 2018 06:59

tomhoefer approved these changes Apr 20, 2018

View reviewed changes

hsudbrock added 3 commits April 20, 2018 22:33

Address review comments by @cdjackson and @tomhoefer.

e2ef71a

* Rename constant for USB product ID for one Qivicon/Telgesis USB dongle. * Add flow control configuration for Ember discovery result (to XONOFF)

Use constant for specifying flow control in ember USB discovery

cd8c7a2

participant Signed-off-by: Henning Sudbrock <[email protected]>

hsudbrock force-pushed the 178-support-discovery-of-usb-dongles branch from 6d27491 to cd8c7a2 Compare April 20, 2018 20:34

tomhoefer merged commit 4ba4617 into openhab:master Apr 21, 2018

hsudbrock mentioned this pull request Apr 22, 2018

Include feature dependency to ESH USB-Serial discovery for ZigBee binding openhab/openhab-distro#685

Closed

cdjackson added this to the 2.3 milestone May 7, 2018

hsudbrock mentioned this pull request May 9, 2018

Support discovery of USB dongles openhab/org.openhab.binding.zwave#900

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[178] Support Discovery of USB dongles (for some Telegesis and one Ember dongle) #179

[178] Support Discovery of USB dongles (for some Telegesis and one Ember dongle) #179

hsudbrock commented Apr 12, 2018 •

edited

Loading

cdjackson commented Apr 12, 2018

hsudbrock commented Apr 12, 2018 •

edited

Loading

cdjackson commented Apr 12, 2018

tomhoefer Apr 12, 2018

hsudbrock Apr 13, 2018

tomhoefer Apr 18, 2018

tomhoefer Apr 12, 2018

hsudbrock Apr 13, 2018

tomhoefer commented Apr 12, 2018 •

edited

Loading

cdjackson left a comment

cdjackson Apr 12, 2018

hsudbrock Apr 13, 2018

cdjackson Apr 13, 2018

hsudbrock Apr 13, 2018

hsudbrock commented Apr 13, 2018

cdjackson commented Apr 13, 2018

hsudbrock commented Apr 13, 2018

hsudbrock commented Apr 16, 2018

hsudbrock commented Apr 17, 2018

hsudbrock commented Apr 19, 2018

cdjackson commented Apr 19, 2018 via email

cdjackson commented Apr 20, 2018

hsudbrock commented Apr 21, 2018

cdjackson commented Apr 21, 2018

tomhoefer commented Apr 21, 2018

kaikreuzer commented Apr 21, 2018

cdjackson commented Apr 21, 2018 via email

hsudbrock commented Apr 22, 2018 •

edited

Loading

hsudbrock commented Apr 22, 2018


		private static final Set<ThingTypeUID> SUPPORTED_THING_TYPES = new HashSet<>(asList(THING_TYPE_EMBER));

		public static final int SILICON_LABS_USB_VENDOR_ID = 0x10c4;

[178] Support Discovery of USB dongles (for some Telegesis and one Ember dongle) #179

[178] Support Discovery of USB dongles (for some Telegesis and one Ember dongle) #179

Conversation

hsudbrock commented Apr 12, 2018 • edited Loading

cdjackson commented Apr 12, 2018

hsudbrock commented Apr 12, 2018 • edited Loading

cdjackson commented Apr 12, 2018

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

tomhoefer commented Apr 12, 2018 • edited Loading

cdjackson 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

hsudbrock commented Apr 13, 2018

cdjackson commented Apr 13, 2018

hsudbrock commented Apr 13, 2018

hsudbrock commented Apr 16, 2018

hsudbrock commented Apr 17, 2018

hsudbrock commented Apr 19, 2018

cdjackson commented Apr 19, 2018 via email

cdjackson commented Apr 20, 2018

hsudbrock commented Apr 21, 2018

cdjackson commented Apr 21, 2018

tomhoefer commented Apr 21, 2018

kaikreuzer commented Apr 21, 2018

cdjackson commented Apr 21, 2018 via email

hsudbrock commented Apr 22, 2018 • edited Loading

hsudbrock commented Apr 22, 2018

hsudbrock commented Apr 12, 2018 •

edited

Loading

hsudbrock commented Apr 12, 2018 •

edited

Loading

tomhoefer commented Apr 12, 2018 •

edited

Loading

hsudbrock commented Apr 22, 2018 •

edited

Loading