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

[cob_sick_s300] fix fields parameter and handling. #318

Open
4 tasks
mgruhler opened this issue Feb 13, 2017 · 2 comments
Open
4 tasks

[cob_sick_s300] fix fields parameter and handling. #318

mgruhler opened this issue Feb 13, 2017 · 2 comments
Assignees

Comments

@mgruhler
Copy link
Contributor

mgruhler commented Feb 13, 2017

Currently, there is was (commented out in #317) a warning about using the fields parameter, as this is the way to go.

"No params for the Sick S300 fieldset were specified --> will using default, but it's deprecated now, please adjust parameters!!!"

However, this is not properly implemented. The driver only supports one field, which defeates the purpose of this parameter. Also, this parameter is nowhere documented.

EDIT answering @ipa-fxm's question below

To be set in the laser yamls (e.g. here)
Old config (without fields param):

"New" config (with fields param and quite some trial-and-error):

fields:
  "1":
    start_angle: -0.75
    stop_angle: 0.0
    scale: 1.0
  "2":
    start_angle: 0.0
    stop_angle: 0.75
    scale: 1.0

What needs to be done:

  • check that the max scan fields is 5 in this file.
  • check the numbering in the params in this file
  • this getScan function seems to only handle the last / one field. However, all configured fields come in one telegram.
  • the retrieved fields (which can be arbitrary ranges within [-135°, 135°] without overlaps but with gaps) need to be combined correctly to in the publishLaserScan function which is not done as well.

All in all, my guess is this feature has been implemented without any tests, has never been used, and probably is even unnecessary.

Implementing this correctly would lead to a full driver covering much of the possibilities of the scanner. However, in ROS, this is imo not required. We can easily get the fields we want without having to configure the scanner by using the cob_scan_filter (or, prefereably, the laser_filters).

@fmessmer
Copy link
Contributor

fmessmer commented Mar 9, 2017

Can someone post what the old config looked like (and where it needs to be set) and what the new config looks like (and where it needs to be set)?

Also, I'd appreciate a few detailed bullet points about what needs to be done implementation-wise to fully implement this feature (and where)

@mgruhler
Copy link
Contributor Author

mgruhler commented Mar 9, 2017

@ipa-fxm see edit above.

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