scan_offset parameter for Ros2 #80
Conversation
|
@chadrockey I just wanted to aks if you could make a statement on roughly when someone could look at this PR? |
|
@YBachmann This is failing all of the tests because it doesn't conform to our style. Please fix the tests to pass, then we can take a look at reviewing it. |
@clalancette The tests are now passing, sorry for not having them fixed earlier. |
| @@ -4,4 +4,5 @@ depthimage_to_laserscan: | |||
| range_min: 0.45 | |||
| range_max: 10.0 | |||
| scan_height: 1 | |||
| scan_offset: 0.5 | |||
There was a problem hiding this comment.
My major question here has to do with whether scan_offset makes sense as a float. Since 0.0 means the top, and 1.0 means the bottom, 0.5 means "in the middle". But what if there are an odd number of scan lines? The user would have no ability to choose exactly which line to use, and instead would just have to take whichever one 0.5 rounded down to.
That said, I'm not all too familiar with this stuff. But would it make sense to let the user pick a specific scan line?
There was a problem hiding this comment.
I understand your concern, however in my opinion a float makes a lot of sense because:
- even if there are an odd number of lines (which is quite uncommon) you can still pick a specific line. The scan_offset value would is
specific_line / image_height. - I don't think it ist even necessary to specify an exact row for the vast majority of usecases. Most people will just roughly look at their depthimages and say "I want to use the top half of my images for the laserscan, so let's set scan-height to
0.25". - As a user I would much rather have the ability to specify that I always want e.g. the center of the image to be used (
scan_offset=0.5), rather than specify a specific row. This way the center will still be used even if I use a different image resolution in the future (e.g. because I scale down my images for performence reasons).
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
|
@clalancette do you have any open points you want to discuss or have changed? PS: I also opened another PR #82 , I would really appreciate if you could take a look at it as well :) |
|
Hello @clalancette , |
Similar to this PR: #28 I added a scan_offset parameter for the ROS2 version.