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

Use EasyMile's fork of lms1xx. #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

esteve
Copy link

@esteve esteve commented Aug 3, 2017

This PR replaces the lms1xx library with EasyMile's (https://github.com/EasyMile/lms1xx), the API is very similar, and includes the following improvements:

  • support for timeouts
  • fixes several underflow issues
  • crossplatform compatibility (via Boost.asio, instead of the Sockets API)

I've added it as an external CMake project, but I don't know if it'd be better as a separate Bloom package released into ROS and have the LMS1xx ROS driver depend on it or just copying the source files into this repository.

One semi-drawback of using this library is that it depends on C++11. Kinetic added C++11 as a requirement for compilers, but Indigo is still at C++03. I'm not sure what the most viable path is, but I guess it'd mean creating a new branch for Kinetic and onwards.

@mikepurvis
Copy link
Contributor

Thanks!

Given that this is a non-core package, I think I'd be fine to simply add the flag for c++11 in this package's build. If you only need language features and not the new standard library stuff, GCC 4.8.2 (Ubuntu Trusty) almost certainly has you covered: https://gcc.gnu.org/gcc-4.8/cxx0x_status.html

The status of bloom as far as submodules is still a bit uncertain (ros-infrastructure/bloom#217), but releasing the library separately and having to patch in the package.xml file is a pretty big pain too. Unless you feel strongly, I'd be inclined to just copy in the source and have a readme note in the parent directory explaining when and where it came from.

@esteve
Copy link
Author

esteve commented Aug 11, 2017

@mikepurvis sorry for the slow response. Yeah, I used CMake's ExternalProject_Add instead of git submodules because of bloom. In the worst case, I can just copy the library here, but I'd rather not duplicate the code to avoid keeping both in sync.

It seems that Travis is missing catkin_pkg, let me see if I can fix that and I'll submit another pull request:

https://travis-ci.org/clearpathrobotics/LMS1xx/jobs/260560497

...
-- Using Python nosetests: /usr/bin/nosetests-2.7
ImportError: "from catkin_pkg.package import parse_package" failed: No module named catkin_pkg.package
Make sure that you have installed "catkin_pkg", it is up to date and on the PYTHONPATH.
CMake Error at /opt/ros/indigo/share/catkin/cmake/safe_execute_process.cmake:11 (message):
  execute_process(/opt/python/2.7.13/bin/python
  "/opt/ros/indigo/share/catkin/cmake/parse_package_xml.py"
  "/opt/ros/indigo/share/catkin/cmake/../package.xml"
  "/home/travis/catkin_ws/build/catkin/catkin_generated/version/package.cmake")
  returned error code 1
Call Stack (most recent call first):
  /opt/ros/indigo/share/catkin/cmake/catkin_package_xml.cmake:63 (safe_execute_process)
  /opt/ros/indigo/share/catkin/cmake/all.cmake:151 (_catkin_package_xml)
  /opt/ros/indigo/share/catkin/cmake/catkinConfig.cmake:20 (include)
  CMakeLists.txt:52 (find_package)
-- Configuring incomplete, errors occurred!
See also "/home/travis/catkin_ws/build/CMakeFiles/CMakeOutput.log".
See also "/home/travis/catkin_ws/build/CMakeFiles/CMakeError.log".
Invoking "cmake" failed
The command "catkin_make" exited with 1.
...

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

Successfully merging this pull request may close these issues.

2 participants