Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update nrjavaserial #101

Closed
cvanorman opened this issue Dec 22, 2016 · 21 comments · Fixed by #104
Closed

Update nrjavaserial #101

cvanorman opened this issue Dec 22, 2016 · 21 comments · Fixed by #104

Comments

@cvanorman
Copy link
Contributor

OH currently uses nrjavaserial-3.9.3 in the serial transport. I've had issues getting serial communications to work properly (in particular with the UPB binding) and these issues go away with the latest version (3.12.1).

Is there any particular issues or concerns with updating this library?

cvanorman added a commit to cvanorman/openhab-core that referenced this issue Dec 23, 2016
Signed-off-by: Chris Van Orman <vanorman.chris@gmail.com>
@watou
Copy link

watou commented Dec 23, 2016

Related issue #29 (comment)

@cvanorman
Copy link
Contributor Author

Ok, well, obviously I didn't look very hard. I've read through a bit of that and it seems the primary issue is jar size (as they've include commons-net and commons-lang3). I can't commit to maintaining a fork, but I could certainly produce a slim jar of 3.12.1 by removing those. This assumes that those libraries are available in OH somewhere else, though. If we need to remove the dependency entirely, that would require some more investigation into how they are using it.

The Arm64 v8hf support is not functional in 3.9.3 from what I understand, so I'm presuming that is not a blocker.

@watou
Copy link

watou commented Dec 23, 2016

Thank you for working on this @cvanorman!

@ThomDietrich
Copy link
Member

ThomDietrich commented Dec 26, 2016

This change introduced a new issue with users zwave-connection @kai @cvanorman

https://community.openhab.org/t/oh2b5-and-dev-ttyacm0-z-wave/18783/10

Finding this thread, I installed liblockdev1 and finally it works! So my guess would be that the latest nrjavaserial needs the liblockdev1 as a dependency (at least on raspberry pi 2)?

Either the dependency can be eliminated or the package needs to be installed with openhab2...
However not everyone is successful with the simple installation of the package. I would suggest to revert the latest commit and look for a better solution.

@kaikreuzer kaikreuzer reopened this Dec 26, 2016
@kaikreuzer
Copy link
Member

Probably related to this change: NeuronRobotics/nrjavaserial#65

And there is another problem report about it: NeuronRobotics/nrjavaserial#60 (comment)

@cvanorman & @watou What do you suggest to do? What issues were solved by the upgrade?

@cvanorman
Copy link
Contributor Author

Well, for me, I had trouble getting it to allow any reading of serial input on Windows. I see there are a number of other open issues under OH about it:

Unfortunately, I don't see any easy solution given the current state of the nrjavaserial project. There appears to be a significant amount of maintenance required to update the native code portion to handle the variety of platforms being used (mostly around file locking). The addition of liblockdev was an attempt to fix this, but obviously broke it for others.

It may be time for OH to consider alternatives to nrjavaserial. I believe Java 8 has added some device io functionality.

@kaikreuzer
Copy link
Member

I reverted the commit again.
Regarding other options: There isn't really anything better, I already did a lot of research in the past.
All solutions require native parts (which thus need to be compiled) and there is no project being really actively maintained.

Java 8 does not bring anything new, I assume you were referring to https://wiki.openjdk.java.net/display/dio/Main. But this is not part of SE 8 (only ME 8) and I cannot see much recent activity on this either...

@kaikreuzer kaikreuzer reopened this Dec 27, 2016
@cvanorman
Copy link
Contributor Author

Yes, I spent some time after that comment and I'm not seeing much that looks promising.

The locking mechanism in nrjavaserial (at least for linux) could probably be changed to a no-op fairly easily (with a compiler flag). This would not be too inconsistent with common behaviour in linux and would be easier than attempting to establish a locking mechanism that would work across multiple linux platforms.

It would have to be done on a fork (which could also trim it down from a fat jar). I have time to do this right now, but I can't guarantee time to provide long term maintenance of the library.

@cvanorman
Copy link
Contributor Author

Of course, you could also just accept that it doesn't work and those that want to use an updated version of the library can uninstall the openhab-transport-serial bundle and drop the nrjavaserial 3.12.1 in their addons folder.

@kaikreuzer
Copy link
Member

It would have to be done on a fork (which could also trim it down from a fat jar). I have time to do this right now, but I can't guarantee time to provide long term maintenance of the library.

This would be terrific!

@splatch
Copy link
Contributor

splatch commented Dec 27, 2016

There are other options for serial port access. Sadly rxtx seems to be abandoned a while ago and all work is done in forks. There are other options, which are actively maintained such jSerialComm (LGPL) or purejavacomm (BSD). Did anyone experiment with these?

@cvanorman
Copy link
Contributor Author

I looked at them and the easier path is still nrjavaserial. jSerialComm suffers from the same native issues as nrjavaserial, and purejavacomm does not seem to have the wide array of platform support that is available in nrjavaserial.

@cvanorman
Copy link
Contributor Author

I've forked nrjavaserial and made a few changes to it:

  • Removed all references to liblockdev.
  • Recompiled the linux natives with the DISABLE_LOCKFILES define (ARM, x86_64, x86_32).
  • Removed the rfc2217 package entirely.
  • Removed the apache-commons dependencies.

The jar is fully OSGi compliant (and works like a charm just dropping it into the addons folder). What would work best for including it?

@kaikreuzer
Copy link
Member

Hey, cool! I have forked your fork to https://github.com/openhab/nrjavaserial, so that we have the source of what is used in openHAB available under the openHAB org at github :-)

If you could upload the jar here, I would make it available on https://bintray.com/openhab/mvn and include it in the target platform (while removing io.transport.serial at the same time).

@cvanorman
Copy link
Contributor Author

File attached.

nrjavaserial-3.12.0-OH.zip

@kaikreuzer
Copy link
Member

Many thanks, @cvanorman!
The jar is now available at https://bintray.com/openhab/mvn/nrjavaserial
It is included in this p2 repo: https://bintray.com/openhab/p2/openhab-deps-repo/1.0.10#files/openhab-deps-repo/1.0.10
which is picked up by the openhab.target by openhab/openhab-distro#363.

The io.transport.serial bundle has been removed and the nrjavaserial bundle is instead included through #104.

@maggu2810
Copy link
Contributor

@kaikreuzer Are the issue reporting deactivated by intention (of the nrjavaserial repo)?
I assume the readme should be changed as there are still references to the original repository (e.g. git clone https://github.com/NeuronRobotics/nrjavaserial.git)

I assume there will be further changes from time to time. Or do you want to keep the work as small as possible and so use the openHAB issue tracker only (so nobody see the repo as a new maintained fork)?

What about the serial implementation of the Kura project?

@kaikreuzer
Copy link
Member

I didn't create the repo as a fork that is supposed to be an actively maintained version of nrjavaserial. I still hope that the original project isn't dead and will be updated again some time. The repo at openHAB is just for tracking the manual changes that have been done to the original version, so that it can be easily re-applied on any future updates.

What about the serial implementation of the Kura project?

It isn't using gnu.io, so that isn't really an option for openHAB (as all code would need to be refactored). And afaik, it is also a pretty old and neglected library...

@ThomDietrich
Copy link
Member

@cvanorman is liblockdev* still needed with the changes you've added? Check https://community.openhab.org/t/nrjavaserial-upgrade-causes-serial-error-port-dev-ttyacm0-does-not-exist/18783/51

@cvanorman
Copy link
Contributor Author

No, the latest version (3.12.0-OH) disables file locking on linux platforms.

There is also #111 which would affect Debian Wheezy users.

@msteigenberger
Copy link

msteigenberger commented Apr 13, 2017

Hi,
I'd need RFC 2217 for my binding. Can we add rfc2217 classes again without including the dependencies?
I've created a PR for this:
openhab/nrjavaserial#1

Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants