RFC2217 URI does not work in OH 2.5

[RafalLukawiecki]

Unsure if this is related to FreeBSD or is it a more generic issue, but using the ZWave binding 2.5.5 latest snapshot and OH 2.5.4, which in turn is using nrjavaserial-3.15.0.OH2, I cannot get the binding to recognise a ser2net remote serial by specifying it as "rfc2217://192.168.1.1:3333" etc. The binding reports offline with error:

Status: OFFLINE - COMMUNICATION_ERROR Serial Error: Port rfc2217://192.168.1.1:3333 does not exist

However, that port works when I use socat to link to a local "real" port, although OH2 crashes later due to another issue related to serial port lockfiles, see #1461

I have discussed it to some extent with @cdjackson, see this thread. @cdjackson seems adamant that the issue is not with the binding but with the underlying serial.

Could this issue be related to the 3.15.OH2 version of nrjavaserial? From a cursory glance at their repo I can see that version 3.16 introduced some changes to the user-side of defining the serial port in rfc2217 terms, even if the key support for it came in version 3.10.

Could it be related to the extra Java option -Dgnu.io.rxtx.SerialPorts=/dev/ttyUSB0" that needs to know what serial ports there are? If so, the syntax of that option does not allow for colons, so providing the URI in it does not work, either.

Happy to test any other suggestions.

As per @cdjackson suggestion, linking also to this discussion in case it helps: eclipse-archived/smarthome#5560

[cdjackson]

As I mentioned, I don’t think nrjavaserial is related - I think the rfc2217 is handled separately through a different abstraction. However I’m not really familiar with where this is done so hopefully someone else can help with that. You might also want to link the ESH issue I referenced earlier as well?

[RafalLukawiecki]

Apologies if I have misquoted you, @cdjackson. Please see these two commits in nrjavaserial that mention RFC2217: NeuronRobotics/nrjavaserial@4d1a035 and NeuronRobotics/nrjavaserial@8f54ed9

The first one also mentions the jvser library as the provider of the functionality. Unfortunately, I am not familiar with any of it, nor Java development, so please look at my effort through a prism of your experience.

One way or another, however, RFC2217 URIs do not work in OH 2.5 according to the reports in the above mentioned community thread.

[cdjackson]

Fair point. I'd assumed (incorrectly) that the RFC2217 was done elsewhere and nrjavaserial was just a serial driver, but it seems it also handles the rfc2217. As I said earlier - I've never looked at this code and it doesn't change my earlier statement that this isn't managed in the ZWave binding. As far as I'm aware there is nothing that is needed in the ZWave binding to support RFC2217 - that was meant to have been the point of the serial abstraction in ESH AFAIK.

[RafalLukawiecki]

Thank you, @cdjackson. I think at this stage it would be easier if someone who actually knows that code helped us out. I had a cursory look at the source of the OH serial namespace and I feel I would need to spend hours to understand their hierarchy and logic—probably only to find out what is clearly known to someone else. All I can see so far is that the OH side of things is aware of rfc2217, some of it (like discovery) is not implemented, but the rest seems to be, according to the class interfaces etc. However, I did not yet find the implementations. I am a bit lost in that codebase!

On another note, I am noticing references that a binding that relies on rfc2217 needs to have additional logic to deal with disconnections, or, otherwise, it will need to be manually restarted when the connection breaks. Does your binding take care of disconnections, ie. as if someone unplugged, then plugged back the USB stick already, in which case I assume it would also deal with the remote rfc2217 host going offline and then coming back online.

[cdjackson]

Does your binding take care of disconnections

Yes, the ZWave binding should reconnect if the serial connection is dropped.

[RafalLukawiecki]

I have configured and recompiled openhab-core to use nrjavaserial-5.0.0. Interestingly, I am getting a different error now when I try to connect using the rfc2217 URI:

check_lock_status: device is locked by another application

Perhaps this is a clue that the version of nrjavaserial supplied with OH2.5 does not have the necessary support for rfc2217, while the one that I have used is not compatible with OH own use of locking, which, I believe, was added in an OH fork of nrjavaserial. I cannot find those modifications anywhere, but if someone shows me the repo, branch, or commit, I can try recompiling again with 5.0.2 (just out) and those modifications.

[RafalLukawiecki]

@cdjackson: I have succeeded in connecting over rfc2217 using the newest nrjavaserial (see NeuronRobotics/nrjavaserial#176), but without OpenHAB in the way. Unfortunately, I am still unable to connect to it from the ZWave binding. I realise you wrote that from your perspective it makes no difference what kind of a port is used, but perhaps it does make a difference since rfc2217 ports cannot be listed/enumerated—they are just URIs after all, and conceivably any Internet host could be an rfc2217 port.

I can see this line of code in ZWaveSerialHandler.java:

Optional<SerialPortIdentifier> opt = serialPortManager.getIdentifiers().filter(id -> id.getName().equals(name))
        .findFirst();

I can also see that this trips up when I try things out—perhaps coincidentally to the issue:

java.lang.UnsatisfiedLinkError: Native Library /var/db/openhab2/userdata/tmp/libNRJavaSerial_openhab_0/libNRJavaSerial.so already loaded in another classloader
        at java.lang.ClassLoader.loadLibrary0(ClassLoader.java:1900)
        at java.lang.ClassLoader.loadLibrary(ClassLoader.java:1817)
        at java.lang.Runtime.load0(Runtime.java:809)
        at java.lang.System.load(System.java:1088)
        at gnu.io.NativeResource.loadResource(NativeResource.java:208)
        at gnu.io.NativeResource.inJarLoad(NativeResource.java:98)
        at gnu.io.NativeResource.loadLib(NativeResource.java:119)
        at gnu.io.NativeResource.load(NativeResource.java:86)
        at gnu.io.SerialManager.<init>(SerialManager.java:67)
        at gnu.io.SerialManager.getInstance(SerialManager.java:73)
        at gnu.io.RXTXCommDriver.<clinit>(RXTXCommDriver.java:95)
        at java.lang.Class.forName0(Native Method)
        at java.lang.Class.forName(Class.java:264)
        at gnu.io.CommPortIdentifier.<clinit>(CommPortIdentifier.java:109)
        at org.eclipse.smarthome.io.transport.serial.internal.SerialPortUtil.getPortIdentifiersUsingScan(SerialPortUtil.java:79)
        at org.eclipse.smarthome.io.transport.serial.internal.RxTxPortProvider.getSerialPortIdentifiers(RxTxPortProvider.java:64)
        at org.eclipse.smarthome.io.transport.serial.internal.SerialPortManagerImpl.lambda$0(SerialPortManagerImpl.java:60)
        at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:269)
        at java.util.HashMap$KeySpliterator.tryAdvance(HashMap.java:1577)
        at java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:126)
        at java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:499)
        at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:486)
        at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
        at java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:152)
        at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.util.stream.ReferencePipeline.findFirst(ReferencePipeline.java:531)
        at org.openhab.binding.zwave.handler.ZWaveSerialHandler.getSerialPortIdentifier(ZWaveSerialHandler.java:108)
        at org.openhab.binding.zwave.handler.ZWaveSerialHandler.watchSerialPort(ZWaveSerialHandler.java:118)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
java.lang.ExceptionInInitializerError thrown while loading gnu.io.RXTXCommDriver

I remember someone on the OH community thread pointed out that the implementation of rfc2217 port enumeration is still "TODO". Checking the code, indeed this is the case in RFC2217PortProvider.java:

@Override
public Stream<SerialPortIdentifier> getSerialPortIdentifiers() {
    // TODO implement discovery here. https://github.com/eclipse/smarthome/pull/5560
    return Stream.empty();
}

I suspect it would be impossible to do that without adding some UI to list those ports upfront.

In any case, I wonder if your code being unable to enumerate ports whose name begins "rfc2217://" could be what prevents a connection from being made.

Should I create a new issue in the ZWave binding repo?

[cdjackson]

My understanding is that the ESH/OH serial abstraction should handle all potential namespaces of serial ports / URIs etc. If we had to change code in every binding to make this work, then it would (IMHO) defeat the purpose of the abstraction.

The ZWave binding just does the following -:

            SerialPortIdentifier portIdentifier = getSerialPortIdentifier(portId);
            SerialPort commPort = portIdentifier.open("org.openhab.binding.zwave", 2000);

I've removed logging and error checking from this to make it clearer. portId is passed directly from the user - so you can type in anything you want. Without knowing the code, maybe the TODO you already found above is the issue?

[RafalLukawiecki]

Thank you, @cdjackson. I had another look at both your and core code, and at the call stack (above) and I think the issue is not just enumeration. From what I can see, it looks like your workaround code (for another issue I guess) calls your version of getSerialPortIdentifier around line 118 of your ZWaveSerialHandler.java. What I do not understand is why this call ends up with the RxTxPortProvider.getSerialPortIdentifiers instead of RFC2217PortProvider.getSerialPortIdentifiers—after all, I am using an rfc2217 port.

In any case, even if the call sequence hit RFC2217PortProvider.getSerialPortIdentifiers, it would end up getting an empty result, as that function has not been implemented (that TODO I mentioned above).

All of this makes me believe that no one has managed to make ZWave binding work over RFC2217 ports. Perhaps it is possible to use rfc2217 in OpenHAB but not the way ZWave uses it, as its calls 100% lead to the wrong places. In any cases, if rfc2217 ever worked in OpenHAB it could not have been in the recent history of nrjavaserial, since that library's rfc2217 had unreachable code until the fix I spurned yesterday. I suppose no one has gone to any lengths of trying to test it, either. Unfortunately, it makes me think that there may be bigger deficiencies in the openhab-core implementation of RFC2217 than even the ones that I have identified so far.

I would humbly suggest that ZWave binding ought to consider taking the same approach as RFXCOM binding which just allows the Bridge to be specified as a TCP host. It is simple and it works very well, as I and others have found out. The alternative is to invest much more effort into fixing, and actually coding ("TODO") the RFC2217 support in the OpenHAB core.

I feel I have reached the limit of what I can do, having invested well over 35 hours into the issue. I would love to help, but without more support and engagement on the side of existing binding and core developers I am unable to do that all by myself.

Time for me to start looking for alternative ways forward, I am afraid. Sorry I could not get this done.

[cdjackson]

From what I can see, it looks like your workaround code (for another issue I guess)

To be honest I'm not 100% clear on why this is necessary - I didn't write this.

I would humbly suggest that ZWave binding ought to consider taking the same approach as RFXCOM binding

I am not keen to do this - sorry. If you want to add this, then I'm happy to take a look at it, but IMHO it's the wrong approach. The whole idea of the serial abstraction was so that bindings didn't have to implement things like this - so that ALL bindings have such features provided by the framework. IMHO modifying the binding to work around a bug elsewhere is a hack - we have to do it for 20 or 30 bindings that want this feature. Clearly this is not something that many people use so unfortunately I can't justify spending time on this at the moment (sorry).

Thanks for looking at this though - maybe someone will pick it up...

[wborn]

In this community post I documented my results with testing rfc2217 connections with OH 2.5.4 on Ubuntu:

• it should be possible to get physically connected devices and rfc2217 devices working at the same time
• there are Paper UI limitations that currently make it difficult to use these rfc2217 connections and it's probably best to configure them using .things files
• it's not guaranteed that all operations on normal serial ports also work on rfc2217 connections, thus they are currently not transparent or a drop in replacement for a normal serial port

Some bindings (dsmr, zwave) call enableReceiveThreshold, enableReceiveTimeout on serial ports. These are unsupported operations on the nrjavaserial TelnetSerialPort. Thus the connection cannot be used when there is code that depends on these operations to succeed.

See also: eclipse-archived/smarthome#6280

[cdjackson]

Thanks for the info @wborn. I think that in general the ESH/OH serial abstraction should take account of these issues so that bindings do not need to know about all possible implementations. I'm not sure that many people use this, so it's possibly not worth spending too much time on until someone can be found to work on this.

[wborn]

The current implementation leaves it up to the developer to properly handle the exceptions and determine if certain functionality is required. Usually everyone will implement a generic try/catch for all exceptions when opening connections without thinking about handling exceptions for each individual operation.

Most likely developers will only test with physically connected devices so the issue will only popup once someone starts testing rfc2217 connections with their add-on.

There are at least 4 options:

1. properly implement exception handling in each add-on (current situation, being able to use rfc2217 is hit or miss)
2. catch the exceptions in the serial transport (will usually make the rfc2217 connections work, but the calling code would no longer know if certain mission critical features could be enabled, which may silently cause wrong behavior)
3. remove these methods from the transport (loss of functionality)
4. make the operations supported in the nrjavaserial TelnetSerialPort implementation (may not be possible)

IMHO option 1. still allows for the best/most functionality when exceptions are properly handled. Perhaps we can improve the current situation by reworking the exception handling in existing add-ons and focus on this in code reviews.

[cdjackson]

It's hard to disagree that implementors should handle the exceptions ;) Currently, the ZWave binding does handle this exception - ie it traps it, and treats this as a failure to configure the serial port.

Maybe we could do something different and try and run anyway, but currently the binding is expecting this to succeed so that the binding gets information in a timely manner. In the past we had problems with data getting cached in the serial driver and this caused delays.

There is another option - maybe not nice, but I'll throw it out there. We could add a separate standalone RFC2217 implementation. I've not looked at how the OH serial abstraction works, but I assume it allows different implementations to register, so we could have a separate drop-in service if we think changing nrjavaserial is not possible.