-
Notifications
You must be signed in to change notification settings - Fork 28
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
Comments
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. :-( |
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. |
Here's what I found: 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
|
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 Can you clarify what the multiecho concern is? |
@tfoote What are your feelings on this stuff? If we build a prototype of |
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]:
|
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. |
@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: Here's the relevant message: And an example/support library: @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). |
@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. |
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. |
publishes odometry messages based on localization data
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.
The text was updated successfully, but these errors were encountered: