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

Support LMS1xx series devices #3

Open
mikepurvis opened this issue Oct 30, 2013 · 10 comments
Open

Support LMS1xx series devices #3

mikepurvis opened this issue Oct 30, 2013 · 10 comments

Comments

@mikepurvis
Copy link
Member

There's support in the underlying sicktoolbox library; I believe it's just a matter of bringing it out as a node.

If there aren't significant concerns, someone from Clearpath can produce a PR for this in the next little while.

@chadrockey
Copy link
Member

I think this has been done under names like "sicktoolbox2" or something similar.

I don't have test hardware, so I can't merge substantial patches. :-(

@mikepurvis
Copy link
Member Author

There's a catkinized version of the old RCPRG driver here: https://github.com/clubcapra/LMS1xx

We'd really like to get support for 100-series in this driver, though. Would you consider a patch which tested against one or more pcap files?

Alternatively, we could arrange for you to SSH to a computer at CPR with the hardware attached, for the purposes of verification.

@chadrockey
Copy link
Member

Here's what I found:
https://github.com/rhuitl/laser_drivers

I don't think sicktoolbox can be easily extended to support multiecho.

I'm also not sure about mixing serial and Ethernet again. In not sure
users easily understand the configuration of urg_node.
On Oct 30, 2013 8:55 AM, "Mike Purvis" [email protected] wrote:

There's a catkinized version of the old RCPRG driver here:
https://github.com/clubcapra/LMS1xx

We'd really like to get support for 100-series in this driver, though.
Would you consider a patch which tested against one or more pcap files?

Alternatively, we could arrange for you to SSH to a computer at CPR with
the hardware attached, for the purposes of verification.


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-27402940
.

@mikepurvis
Copy link
Member Author

Hmm, that one looks somewhat abandoned, too—certainly never got catkinized or released.

In any case, what I'm hearing is that we'd be best served to build (and release) our own version of sicktoolbox_wrapper, with a focus on the 100-series devices, eg sicktoolbox_wrapper_lms1xx.

Can you clarify what the multiecho concern is?

@mikepurvis
Copy link
Member Author

@tfoote What are your feelings on this stuff? If we build a prototype of sicktoolbox_wrapper_lms1xx package to support these devices, would you consider having it in the ros-drivers org? Any opinions on naming?

@tfoote
Copy link

tfoote commented Oct 30, 2013

Is this not something we can merge back and just have one driver?

On Wed, Oct 30, 2013 at 12:45 PM, Mike Purvis [email protected]:

@tfoote https://github.com/tfoote What are your feelings on this stuff?
If we build a prototype of sicktoolbox_wrapper_lms1xx package to support
these devices, would you consider having it in the ros-drivers org? Any
opinions on naming?


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-27432203
.

@mikepurvis
Copy link
Member Author

That would be my preference, certainly.

@chadrockey, would you consider dual-maintainership, where you're responsible for the sicklms/sickld binaries, and we'd create sources for and agree to maintain new sicklms_1xx/sickld_1xx binaries?

Depending how similar they looked, we could look at a merger down the line, rather than trying to hack in support to the existing driver upfront.

@chadrockey
Copy link
Member

@mikepurvis The 1xx and 5xx series use the same comms, so you should be able to support both. (One is just a subset of the other).


@mikepurvis The multiecho is the multiple returns per beam feature of the 1xx and 5xx. For instance, you get multiple distance measurements per angular increment. This allows you to see through partially obstructed materials, sometimes get a return from glass, easier measurements from partially reflective materials. I've added support for Hokuyos but I haven't been able to get ahold of a 1xx and a 5xx to investigate whether to patch, fork, or wrap a new library for SICK. Here's a diagram from SICK showing multiechoes:
http://www.robotsinsearch.com/sites/default/files/images/lms500-20000-pro/SICK-LMS500-multi-Echo.png

Here's the relevant message:
http://docs.ros.org/api/sensor_msgs/html/msg/MultiEchoLaserScan.html

And an example/support library:
https://github.com/ros-perception/laser_proc/blob/hydro-devel/src/LaserPublisher.cpp


@tfoote @mikepurvis Sicktoolbox is pretty legacy and stable. I would prefer not to risk introducing unknown bugs (doubly so since I do not have any of the various lasers to test 291-S05, 291-S16, Pioneer 291, 501, 511, 151, and so on. I would prefer a new repo for the newer lasers that fully support a multiecho API and REP 138 (http://ros.org/reps/rep-0138.html). This would preserve anyone who's been using this package for years and doesn't expect a lot of change. Not to mention, I've tagged this as maintained, rather than developed so users shouldn't expect large changes.


@mikepurvis For practical purposes I've been curating the ros-drivers org. The process for being included is to just send an announcement to the ros-drivers-sig and I'll set up a repo (or fork an existing one).

https://groups.google.com/forum/#!forum/ros-sig-drivers

@chadrockey
Copy link
Member

@mikepurvis I would prefer new laser development to be forked. That's the approach I did with Hokuyos.

Hokuyo_node is the older driver compatible with USB lasers only. Intended to be maintained through the lifespan of the PR2 (whatever that may be at this point).

urg_node is the newer driver compatible with USB and ethernet lasers and introduced the API changes. Intended for those seeking newer features (multi-echo) or newer lasers that have firmwares that still may change.

@mikepurvis
Copy link
Member Author

Hmm. I think was under the impression that the sicklms node in this repo was already 138 compliant.

We haven't got the bandwidth to embark on a new codebase for this, especially if multi-echo would be defining feature and is not something already supported in the underlying sicktoolbox library.

My preference would still be to add a new node to this package, with simply equivalent functionality for 1xx as is available for 2xx, via sicktoolbox.

dmiklic referenced this issue in larics/sicktoolbox_wrapper May 16, 2017
publishes odometry messages based on localization data
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

No branches or pull requests

3 participants