Port to tf2 and enable using static broadcaster #28
Conversation
paulbovbel
force-pushed
the
tf2-static
branch
2 times, most recently
from
2285b13 to
6e744fb
Compare
|
This is a great enhancement!. Do you have some numbers of |
|
@adolfo-rt, the reduction is linear with the number of fixed joints. From what I remember, each fixed joint eats up approx 5-10 KB/s, at the default publish rate (50 Hz). |
|
Upstream changes to geometry_experimental have been merged and released, ping for review. |
I just checked, and our REEM robot has +100 joints, of which almost 40% are fixed. So the bandwidth improvements should be significant in such a case. Good work!. |
|
Glad to share the bandwidth reduction - this was a low hanging fruit, and I'm surprised it never happened during the tf2 transition. |
|
Ping @isucan, can you please review? |
|
This looks good to me. The only concern I have is whether this should be merged to jade, not indigo. The improvements are great, but I worry about the ABI change, given this is such a low level package. What do you think @wjwwood ? |
|
Either way, in jade I would advocate for making 'use_static' the default behaviour. |
|
As long as the default behavior is not changing in Indigo, that's probably fine. I'm a bit wary to change the default behavior in Jade, but since it should be a transparent change I'd probably go ahead with it. |
|
@isucan, is this okay to merge as is? I can write a separate pull to jade-devel with this behaviour as default. |
|
Yes, I think we can merge this, but there seem to be some conflicts (related to another change that got pushed). We should also keep the jade branch in sync, and add the change to the default you mentioned (in jade). |
|
I rebased on https://github.com/ros/robot_state_publisher/tree/tf2-static @paulbovbel if that looks correct to you, you can either rebase your branch on the upstream branch, or we can just open a new PR with the rebased branch if you don't have time. I will also open a separate PR that targets jade. |
paulbovbel
force-pushed
the
tf2-static
branch
from
8e4e18e to
727b848
Compare
|
thanks @jacquelinekay @isucan, this PR is ready to go |
Port to tf2 and enable using static broadcaster
|
|
||
| private: | ||
| void addChildren(const KDL::SegmentMap::const_iterator segment); | ||
|
|
||
|
|
||
| std::map<std::string, SegmentPair> segments_, segments_fixed_; | ||
| <<<<<<< HEAD |
There was a problem hiding this comment.
Yup. Sure enough, the merge conflict is here. Good catch, @remod. Did this branch need to be rebased before merging?
This results in a failure when someone adds find_package(robot_state_publisher) to their CMakeLists.txt. You get the error 'robot_state_publisher' exports 'robot_state_publisher' which can't be found. Signed-off-by: Shane Loretz <[email protected]>
Depends on ros/geometry2#114
Added a "use_tf_static" parameter, which enables broadcasting static transforms over /tf_static using a latched publisher.