openhab / org.openhab.binding.zigbee

openHAB binding for ZigBee
Eclipse Public License 2.0
73 stars 111 forks source link

USB discovery does not seem to honour persistent device names #307

Closed puzzle-star closed 2 years ago

puzzle-star commented 5 years ago

Since "Support discovery of USB dongles #178" USB devices are automatically discovered and some discovery participants allow for auto detection of some devices (i.e. ZigBee Telegesis dongles).

Notwithstanding, USB discovery does not seem to honour persistent device names, and discovered device names may not be consistent between reboots. This is a follow-up on "https://github.com/openhab/org.openhab.binding.zigbee/issues/178#issuecomment-441360319".

@hsudbrock, could you please have a look at this and give your thoughts on whether this would need a fix?

I have seen that the discovery of the USB devices, and naming the different dongles via udev, do not seem to interface very well (at least for me).

The discovery participant is returning the /dev/ttyUSBx (for instance) and using the discovered device name as representationProperty. But this name might change between reboots, so normally, when using different dongles, we symlink to it (i.e. /dev/telegesis-1 -> /dev/ttyUSB0) via udev rules.

With the USB discovery, if for some reason the devname changes between reboots, the dongle becomes unusable. On the other side, if after discovering it I manually change the serial port of the device to the persistent name symlink, it will keep on appearing as new device in the inbox (yes, I can hide it, but it is annoying and maybe misleading for current users).

Changing the representationProperty to the serial number (or maybe vip_pid_serial) might prevent it from appearing again in the inbox (not sure), but would still require to manually change the serial port to the symlink.

Would it make sense to change (this is ESH, not OH):

Path devicePath = Paths.get(devDirectory).resolve(serialPortName);`

To some thing like (this is pseudocode, just to check if this would make sense to you):

Path devicePath = resolveDevicePath(devDirectory, serialPortName);
...
Path resolveDevicePath(String devDirectory, String serialPortName) {
    Path devicePath = Paths.get(devDirectory).resolve(serialPortName);
    File devDir = new File(devDirectory);
    for (File dev : devDir.listFiles()) {
       Path devPath = dev.toPath();
       if (Files.isSymbolicLink(devPath) && Files.isSameFile(devPath, devicePath)) return devPath;
    }
    return devicePath;
}

I know it is not the most efficient, as it traverses the the full dev directory looking for symlinks to the discovered path, but allows to honour the different dongle naming persistence.

Pedro

hsudbrock commented 5 years ago

Hi Pedro, thanks for bringing this up! I agree that using the serial port as representation property is not a good idea, because this property does not represent device (for the reason you describe). So, I don't have anything against the representation property being changed to the serial number. (Side note: As far as I recall, some dongles don't provide their serial number via USB, this needs to be taken into account when changing this.)

I have not done anything about it so far, because ESH automatically fixes the configuration when the serial port changes: The serial number of the dongle is encoded in the discovery result's thing UID; in consequence, a dongle will always have the same thing UID in the discovery, regardless of the serial port; when ESH detects a discovery result with the same thing UID as an existing thing, ESH triggers a configuration update of the thing using the configuration provided in the discovery result; as a result, the serial port property is adjusted in the thing's configuration.

Due to this config update mechanism of ESH, I don't think that a scan for symlinks that you propose should be necessary.

With the USB discovery, if for some reason the devname changes between reboots, the dongle becomes unusable.

I wonder why the ESH mechanism I sketched above does not work. Could it be due to #233, which Chris fixed in #273, and that is not yet included in your installation?

puzzle-star commented 5 years ago

Hi Henning,

My installation is usually at SNAPSHOT version (well, from time to time: now on 2.4.0-M6), but as I installed the dongle quite ago and I had issues I manually created it (so the think ID is not the serial number), and points to the symlinked device.

This means the config update mechansim is never invoked. I am now manually changing everything (json, xml, etc) to set the dongle thing ID to the same one the USB participant is discovering, will check what happens, and let you know.

puzzle-star commented 5 years ago

Hi @hsudbrock ,

Checked and working OK, but it has been a bit more painful than expected.

I have forced a change in the USB name, and it has been correctly detected by the participant. The configuration of the dongle has been updated accordingly...

But the new ttyUSB1 was not in my "-Dgnu.io.rxtx.SerialPorts=...", so it has not worked at all, with the side effect of losing my network state (@cdjackson, is it possible that if the dongle fails to initialize for some reason, the network state is saved with no node, forcing a full rediscovery on next succesful boot?)

So unless "gnu.io.rxtx.SerialPorts" is preloaded with all possible options for device names, it will not work as expected.

Henning, would it then make sense to look for the symlinks as an enhancement?

Also, I am surprised about this: looking whether the list of ports could be automatically set-up, I had a look at the nrjavaserial code, and to my surprise autodetection for Linux (and other OSes) is in place, so I just did a quick test, and:

~ #> java -cp /usr/share/openhab2/runtime/system/com/neuronrobotics/nrjavaserial/3.14.0/nrjavaserial-3.14.0.jar:. TestSerialPort
Available port: /dev/ttyAMA0
Available port: /dev/ttyUSB0

~ #> java -Dgnu.io.rxtx.SerialPorts=/dev/telegesis-1 -cp /usr/share/openhab2/runtime/system/com/neuronrobotics/nrjavaserial/3.14.0/nrjavaserial-3.14.0.jar:. TestSerialPort
Available port: /dev/telegesis-1
Available port: /dev/ttyAMA0
Available port: /dev/ttyUSB0

Is there somewhere in OH / ESH where this is overriden? It looks like the defined list should not be necessary, but it is not working without it.

johanneskropf commented 5 years ago

I think this is connected to this issue. I have four USB devices attached to an Raspberry Pi 3B+. One of them is a Zigbee Ember dongle. I have Symlinks for the devices. After every Reboot the status of the Dongle is unknown until I manually change it back to the Symlink as that is reset at every restart. As soon as I try to start Zigbee discovery this happens:

2019-01-24 11:21:26.456 [ERROR] [ding.zigbee.handler.ZigBeeSerialPort] - Serial Error: Port /dev/ttyUSB0 does not exist.

2019-01-24 11:21:26.461 [ERROR] [zigbee.dongle.ember.ZigBeeDongleEzsp] - Unable to open Ember serial port

==> /var/log/openhab2/events.log <==

2019-01-24 11:21:26.475 [hingStatusInfoChangedEvent] - 'zigbee:coordinator_ember:013C17D4' changed from UNKNOWN to OFFLINE (COMMUNICATION_ERROR)

2019-01-24 11:21:26.491 [hingStatusInfoChangedEvent] - 'zigbee:coordinator_ember:013C17D4' changed from OFFLINE (COMMUNICATION_ERROR) to OFFLINE: Failed to open communications port

After this the Serial Port is reset once again but this time if i try to change it back to the Symlink this happens:

2019-01-24 11:26:48.008 [ERROR] [st.core.internal.thing.ThingResource] - Exception during HTTP PUT request for update config at 'things/zigbee:coordinator_ember:013C17D4/config'

I have to restart to get it back online but the problem when I start discovery persists. I already tried all the fixes. It could be related to this issue with the enocean binding too https://github.com/fruggy83/openocean/issues/57

puzzle-star commented 5 years ago

Hi @johanneskropf,

Did you add /dev/ttyUSB0 (and the equivalent for the other dongles), to your "gnu.io.rxtx.SerialPorts"?

The "Serial Error: Port /dev/ttyUSB0 does not exist." normally happens because of this.

johanneskropf commented 5 years ago

@puzzle-star this shouldn't be necessary as there are Symlinks for each of the three Dongles. But yes I have tried this too, but if I add /dev/ttyUSB0 it doesn't work at all. Too my understanding the Ember Stick is not gonna be on the same tty every boot anyway. That's why there are Symlinks in the first place. Those are correct too. And I am in the right groups with the right permissions and the right users. It works with the zwave and Enocean Stick. Although the Zwave Binding is the only one which respects the Serial port Symlink through a reboot. I have to reenter the Symlink for the Enocean after every reboot but than it works, but the discovery progress does something strange too, see the link in my previous post. The problem with the Zigbee Binding I have is that once I started the discovery the Stick goes offline and there is no way to bring it back but a restart. I kind of think that maybe the Zigbee Ember and Enocean USB300 just don't like being on the same pi or that the bindings for them have some issue with each other.

puzzle-star commented 5 years ago

Hi @johanneskropf,

It was not necessary before the USB discovery automatically changed the serial port name when discovering a recognized dongle.

Now, as the USB discovery is changing the name of the serial port to the non-symlink one, it has to be listed in "gnu.io.rxtx.SerialPorts" or it won't work.

@hsudbrock, if the representation property is the serial port, what happens if the serial port name is changed after a reboot, or by reconnecting the dongle? This is why, at least the Linux guys, like myself :), usually give a persistent name to the serial ports, and hence the symlinks. This used to be also the recommended approach when using more than one dongle or in situations where the serial port could change.

Do you think it would be worth either changing the representation property to the serial number of the device, or otherwise allowing the USB discovery to check for the symlinks?

If not I think we will find these issues recurrently, and will be hard to fix if the representation property is the serial port name.

hsudbrock commented 5 years ago

Hi @puzzle-star;

you ask:

Do you think it would be worth either changing the representation property to the serial number of the device, or otherwise allowing the USB discovery to check for the symlinks?

I have nothing against this, see my answer when you opened this issue :)

Hi Pedro, thanks for bringing this up! I agree that using the serial port as representation property is not a good idea, because this property does not represent device (for the reason you describe). So, I don't have anything against the representation property being changed to the serial number. (Side note: As far as I recall, some dongles don't provide their serial number via USB, this needs to be taken into account when changing this.)

puzzle-star commented 5 years ago

Hi @hsudbrock,

I recall, but this is on ESH, and I do not have a full dev environment to make changes on ESH.

I can give it a try to change it myself, but is this the right moment or better wait till ESH is integrated back onto OH?

Also my "IDE" and "compiler" tools at the moment are "vi", "javac" and "jar". Struggling with maven since last week to create a p2 bundle... Is command line building of ESH possible, or do I need to setup again Eclipse IDE?

johanneskropf commented 5 years ago

So I would have to list ttyUSB1 etc too in Dgnu.io in case the Zigbee dongle is on one of these? After a reboot it was now on ttyUSB0 and it stays online when I go into discovery. But why does it reset the Serial Port of the Enocean binding when going into discovery? The Enocean Stick is on ttyUSB1. Right now I can't discover devices but that is probably a different problem.

hsudbrock commented 5 years ago

I recall, but this is on ESH, and I do not have a full dev environment to make changes on ESH.

No, the representation property is set by the discovery participant (e.g., here for Ember in the ZigBee binding: https://github.com/openhab/org.openhab.binding.zigbee/blob/master/org.openhab.binding.zigbee.ember/src/main/java/org/openhab/binding/zigbee/ember/internal/discovery/ZigBeeEmberUsbSerialDiscoveryParticipant.java).

cdjackson commented 4 years ago

Does anyone have a concrete solution to this problem. If not, I would propose to remove the USB discovery.

hsudbrock commented 4 years ago

I have ideas for a solution, but, unfortunately, I do not get to experiment how well they work in the foreseeable future :/

Hence, @cdjackson, I agree that maybe it's best to remove the USB discovery for now; I can still create a PR for re-adding a USB discovery which works with persistent device names at some other time, when I have more time for working on that.

puzzle-star commented 4 years ago

I have ideas for a solution, but, unfortunately, I do not get to experiment how well they work in the foreseeable future :/

Hi @hsudbrock , at the beginning of the issue (in one of the comments, I posted some code that fixed it.

https://github.com/openhab/org.openhab.binding.zigbee/issues/307#issue-383982884

I tried in my setup and it worked, but it was several months ago and I have no longer a development environment setup to create a PR (and I could not before the end of Christmas)...

Pedro

cdjackson commented 2 years ago

USB discovery was removed so closing this