Skip laserscans with no corresponding tf transform #313

ghost · 2017-02-08T12:57:13Z

fixes https://github.com/ipa320/cob_driver/issues/268

mgruhler

Could you also add the sleep (1s) after the initialization of the transform listener, as proposed in the issue.

Also, please investigate (in simulation) what happens if you set the frequency of this node to something higher than 10 Hz.

mgruhler · 2017-02-08T14:37:24Z

cob_scan_unifier/src/scan_unifier_node.cpp

        ROS_DEBUG("now project to point_cloud");
-        projector_.transformLaserScanToPointCloud("/base_link",vec_laser_struct_.at(i).current_scan_msg, vec_cloud.at(i), listener_);
+        projector_.transformLaserScanToPointCloud("base_link",vec_laser_struct_.at(i).current_scan_msg, vec_cloud.at(i), listener_);


could you please make this a parameter, while you are at it?

ghost · 2017-02-08T16:08:38Z

Frequency was already 100Hz. Tried also with 15Hz and it works. Should I add the param to the YAMLs of all robots?

mgruhler · 2017-02-08T16:17:23Z

@mig-em yes, please do that.

fmessmer · 2017-02-09T08:55:40Z

could we also investigate #248 and #249 while @mig-em is on it?
(At least check and report whether the issues are still an issue 😉)

mgruhler · 2017-02-09T09:10:10Z

@mig-em Please do as commanded ;-)

benmaidel · 2017-02-09T12:22:39Z

cob_scan_unifier/src/scan_unifier_node.cpp

+            vec_laser_struct_.at(i).current_scan_msg.header.stamp, ros::Duration(3.0)))
+            {
+              continue;
+            }


please correct indentation

benmaidel · 2017-02-09T12:55:13Z

@mig-em @ipa-mig isn't 15Hz a bad idea? See your comments here: ipa320/cob_robots#367

I think we should switch to https://github.com/iralabdisco/ira_laser_tools because they seem to have a better implementation. I've already used their implementation once and it worked quite well.

mgruhler · 2017-02-09T13:10:55Z

@ipa-bnm to avoid misunderstandings: @ipa-nhg reported in #268 some problems with a rate higher then 10Hz. This should not be made the default :-)

The comment about the parameter is only to add the new parameter to the yamls...

ghost · 2017-02-10T14:32:44Z

So should I adapt the synchronisation part by using the TimeSynchronizer filter and investigate https://github.com/ipa320/cob_driver/issues/249 or better use the ira_laser_tools?
If I understand it right the rate param will become obsolete by using the filter.

fmessmer · 2017-02-10T14:45:54Z

I'd say we should evaluate ira_laser_tools and - if it solves all our problems - use it!
If not, fix issues in current scan_unifier...

fmessmer · 2017-02-10T14:47:07Z

Well, if ira_laser_tools works for us, we might want to "take over" the package and copy/move it to an ipa320 repo...as it is not released...License is TODO 😉

mgruhler · 2017-02-10T15:56:45Z

if ira_laser_tools is actually working good, "taking it over" would be a good idea. But I'd say we try to get in contact with the developers and see how to proceed. Last commit was Oct. 2015 for that matter...

mgruhler · 2017-02-10T15:57:09Z

@ipa-bnm would this be worth investigating on cob4?

benmaidel · 2017-02-13T10:43:34Z

@ipa-mig yes, we can/should test this on cob4 as soon as a new hardware is available

fmessmer · 2017-02-16T07:28:01Z

Well, would #257 have solved #268 already?

I guess we should keep #248 and #249 out of this PR...as it is separate and needs more testing.
But, I'd like to finalize/accept/close #313, #257 and #268...
@mig-em @ipa-mig @destogl

ghost · 2017-02-22T16:21:04Z

I tried the ira_laser_tools and they seem to work but https://github.com/ipa320/cob_driver/pull/257 and https://github.com/ipa320/cob_driver/issues/268 are not fixed (there is still the "Lookup would require extrapolation into the past." error and they use tf1).

In this screen you see raw3-3 scans (red, green) and the unified scan (yellow)

fmessmer · 2017-02-22T16:40:09Z

Is the following summary correct?
@ipa-mig @mig-em @ipa-bnm

cob_scan_unifier:

no LookupTransform error Skip laserscans with no corresponding tf transform #313 [cob_scan_unifier] LookupError on start up #268
tf2 (tested?) Update scan_unifier to use tf2 #257
has issues merging scans [cob_scan_unifier] cob_scan_unifier is not merging complete scans in simulation #249 (were you able to reproduce [cob_scan_unifier] cob_scan_unifier is not merging complete scans in simulation #249 with your tf-fixes?)

ira_laser_tools

has LookupTransform error
uses tf1
merges scans correctly (both sim and real sensor?)
release issue as we are not maintainer...

benmaidel · 2017-02-23T09:24:17Z

Just for completeness.
There is also the possibility to use a combination of https://github.com/ros-perception/laser_assembler and https://github.com/ros-perception/pointcloud_to_laserscan to achieve a merged scan.

ghost · 2017-02-23T13:13:37Z

Additionally the parameters of the ira_laser_tools should be adapted.
Maybe the LookupTransform error should be fixed in the driver of the scanners, or is there a point in sending out scans without according transforms to base_link?

fmessmer · 2017-03-01T09:53:58Z

cob_scan_unifier/src/scan_unifier_node.cpp

+        if (!listener_.waitForTransform(frame_, vec_laser_struct_.at(i).current_scan_msg.header.frame_id,
+            vec_laser_struct_.at(i).current_scan_msg.header.stamp, ros::Duration(3.0)))
+        {
+          continue;


@ipa-mig @mig-em
Well, here we silently skip a scan that cannot be transformed. Should we at least notify in some way?
e.g. ROS_ERROR_STREAM("Not using scan during unification: " << vec_laser_struct_.at(i).current_scan_msg.header.frame_id,);

fmessmer · 2017-03-01T10:41:12Z

@ipa-mig FYI

As discussed with @mig-em, we will try a message_filter-based implementation before merging this PR

mgruhler · 2017-03-01T10:42:57Z

good idea.

fmessmer · 2017-03-09T08:22:05Z

message filter implementation is in #324
In case #324 works, this PR is obsolete

fmessmer · 2017-03-16T08:21:05Z

https://github.com/ipa320/cob_driver/pull/324 is the proper way to solve #268

Skip laserscans with no corresponding tf transform

573a007

mgruhler suggested changes Feb 8, 2017

View reviewed changes

Added delay and parameter for tf_frame

beddaff

mgruhler approved these changes Feb 9, 2017

View reviewed changes

benmaidel requested changes Feb 9, 2017

View reviewed changes

Fixed identation

6d7f322

benmaidel approved these changes Feb 10, 2017

View reviewed changes

fmessmer requested changes Mar 1, 2017

View reviewed changes

fmessmer closed this Mar 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Skip laserscans with no corresponding tf transform #313

Skip laserscans with no corresponding tf transform #313

ghost commented Feb 8, 2017 •

edited by fmessmer

Loading

mgruhler left a comment

mgruhler Feb 8, 2017

ghost commented Feb 8, 2017

mgruhler commented Feb 8, 2017

fmessmer commented Feb 9, 2017

mgruhler commented Feb 9, 2017

benmaidel Feb 9, 2017

benmaidel commented Feb 9, 2017

mgruhler commented Feb 9, 2017

ghost commented Feb 10, 2017

fmessmer commented Feb 10, 2017

fmessmer commented Feb 10, 2017

mgruhler commented Feb 10, 2017

mgruhler commented Feb 10, 2017

benmaidel commented Feb 13, 2017

fmessmer commented Feb 16, 2017

ghost commented Feb 22, 2017 •

edited by ghost

Loading

fmessmer commented Feb 22, 2017 •

edited

Loading

benmaidel commented Feb 23, 2017

ghost commented Feb 23, 2017

fmessmer Mar 1, 2017

fmessmer commented Mar 1, 2017 •

edited

Loading

mgruhler commented Mar 1, 2017

fmessmer commented Mar 9, 2017

fmessmer commented Mar 16, 2017

Skip laserscans with no corresponding tf transform #313

Skip laserscans with no corresponding tf transform #313

Conversation

ghost commented Feb 8, 2017 • edited by fmessmer Loading

mgruhler left a comment

Choose a reason for hiding this comment

mgruhler Feb 8, 2017

Choose a reason for hiding this comment

ghost commented Feb 8, 2017

mgruhler commented Feb 8, 2017

fmessmer commented Feb 9, 2017

mgruhler commented Feb 9, 2017

benmaidel Feb 9, 2017

Choose a reason for hiding this comment

benmaidel commented Feb 9, 2017

mgruhler commented Feb 9, 2017

ghost commented Feb 10, 2017

fmessmer commented Feb 10, 2017

fmessmer commented Feb 10, 2017

mgruhler commented Feb 10, 2017

mgruhler commented Feb 10, 2017

benmaidel commented Feb 13, 2017

fmessmer commented Feb 16, 2017

ghost commented Feb 22, 2017 • edited by ghost Loading

fmessmer commented Feb 22, 2017 • edited Loading

benmaidel commented Feb 23, 2017

ghost commented Feb 23, 2017

fmessmer Mar 1, 2017

Choose a reason for hiding this comment

fmessmer commented Mar 1, 2017 • edited Loading

mgruhler commented Mar 1, 2017

fmessmer commented Mar 9, 2017

fmessmer commented Mar 16, 2017

ghost commented Feb 8, 2017 •

edited by fmessmer

Loading

ghost commented Feb 22, 2017 •

edited by ghost

Loading

fmessmer commented Feb 22, 2017 •

edited

Loading

fmessmer commented Mar 1, 2017 •

edited

Loading