4am-robotics / cob_driver

The cob_driver stack includes packages that provide access to the Care-O-bot hardware over ROS messages and services. E.g. for mobile base, arm, camera_sensors, scanners, etc...
www.care-o-bot.org
Apache License 2.0
102 stars 164 forks source link

[cob_scan_unifier] Update rate #248

Closed fmessmer closed 7 years ago

fmessmer commented 8 years ago

Some discussion history from https://github.com/ipa320/cob_robots/pull/367#issuecomment-157902641

@ipa-fxm: On a side note: @ipa-fmw thought that 100Hz might be faster than the actual scanner publish rate....Should the rate be > reduced? @ipa-bnm What do you think?

@ipa-mig: @ipa-fxm @ipa-fmw @ipa-bnm Yes, the rate of 100Hz is (considerably) faster than the scanner publish rate (which is 12Hz for the S300, afaik). However, this is on purpose to send out the combined scan asap. It is not deterministic in which order the single scan topics come in and it is, in the worst case, almost one full scan rate cycle delay. This is due to the scan_unifier waiting until it received information from every scan before it combines one. This frequency was chosen to be high to check the subscriber callbacks frequently to introduce as little latency between the original scans and the combined one as possible. However, it could still be beneficial to change this frequency...

Some hints that might help reduce the update rate while guaranteeing the desired behaviour:

Some questions to decide on best implementations:


@ipa-bnm could you also create a new issue for the assembling bug that you told me about? so that it is documented!

mgruhler commented 8 years ago

@ipa-bnm btw, you told me once about another scan unifier. Where is this again?

@ipa-fxm for some mapping algorithms, it is important to have a unified scan of all scans. The problem is actually not in the algorithm itself, but in the, sometimes very, bad calibration of the laser scanners. this results in a "rotational jumping" which produces bad maps. this is why this has been developed in the first place.

For localization, I don't need this at all.

Thus, it is actually hard to say, what would be okay. I cannot really tell ;-)

Where else is the scan_unifier required currently?

benmaidel commented 8 years ago

The scan unifier is also used by the led driver (match scan to led ring) and the tablet app (distance view feature on teleop fragment). I'm not aware of any other nodes.

I think a guaranteed publish rate for the assembled scan is not that important than a complete merged scan. So ipa-fxm's proposals (use message_filters and ros::Timers) are the right way to go.

The other scan unifier i found is here: https://github.com/iralabdisco/ira_laser_tools This one offers more features like, point cloud publisher and frame transformer. It uses the LaserProjection and pcl packages to merge the scans. And it also waits until all scans received once.

@ipa-fxm: i copied the assembling bug issue from cob_navigation to cob_driver

fmessmer commented 7 years ago

https://github.com/ipa320/cob_driver/pull/324 uses a callback based approach, thus we do not have a separate publish rate...closing