Update KDL SegmentMap interface to use shared pointers #43
Conversation
|
Just like discussed in ros/robot_state_publisher#5 we need to figure out when and where this change will need to be deployed to line up with the new release of |
|
Updated this PR to reflect the newest changes in the upstream pull request. It seems I accidentally rebased on indigo-devel, is this a big problem? |
|
Not at all. All these changes should actually be rebased for indigo, and sent for the indigo-devel branches. |
The KDL Tree API optionally uses shared pointers on platforms where the STL containers don't support incomplete types.
|
Now that the change has been merged upstream I properly rebased on hydro-devel. |
|
I completely forgot to check up on the PR, is there anything holding this back currently from being merged into hydo-devel and indigo-devel? |
|
bump EDIT: The bump is about getting these changes released into hydro/indigo in a coordinatd fashion: #43, ros/robot_state_publisher#5, #4, #19 |
|
Now that orocos_kdl has been re-released (https://github.com/smits/orocos-kdl-release) including the patch that this PR depends on, can we merge this and #60 and then possibly some time get a hydro and an indigo release? |
|
@NikolausDemmel do these need to be |
|
No @jensenb: Am I right? |
|
Ok, I see now, I think you are right @NikolausDemmel. |
|
@isucan with your permission I would merge and release this at least into Indigo, but probably also Hydro. Otherwise you can do it, but I know you are busy. |
|
@NikolausDemmel That is how it was intended. That is the best way i could come up with to mask the underlying API. |
Update KDL SegmentMap interface to use shared pointers
|
Releasing would be great! Thanks! |
|
Sorry I just realized you wanted me to release, doing it now. |
|
@isucan is hydro ok for release? (also your accidental version bump to 1.11.0 in hydro-devel some how resulted in no 1.10.19 tag, so I fixed that) I'm a little weary about releasing hydro given the set of changes. |
|
These are the changes: 1.10.19...hydro-devel |
|
These are the changes that were already on |
|
@NikolausDemmel I'm not sure, this pull request was only one commit, but it is possible something got messed up and the indigo changes ended up on the hydro branch by accident. That's why I am going to wait on @isucan to give the green light on the pending changes for hydro first. |
|
@NikolausDemmel I am not sure which changes you are referring to. Are they from a different commit? If something got accidentally merged from indigo, I agree we should remove it. If it's something that does not break ABI though, it should be fine. |
|
Sorry for the confusion. @isucan: I'm talking about the changes in 1.10.19...hydro-devel, i.e. all changes on hydro-devel since the last hydro commit. All these would be released into hydro if a release were done now. Only the last two commits and the one from March 7 are relevant to this patch. Ignore my comment about them possibly being merged inadvertedly from indigo-devel; I just looked at the commit graph again; that doesn't seem to be the case. So the remaining question from @wjwwood to you, @isucan, is to confirm that robot_model should indeed be released into hydro, given that this would not only release the patches relevant to this KDL issue, but also all the other patches shown in 1.10.19...hydro-devel. If you give your ok, William will do the release. |
|
@isucan Yeah, @NikolausDemmel has it right, I'm looking for a 👍/👎 from you. |
|
You guys are awesome! Thanks for making it so easy for me. +1 @wjwwood |
|
Ok, pulling the trigger. |
This is a dependent change for the orocos KDL API modification discussed in: orocos/orocos_kinematics_dynamics#4
To summarize the KDL API used incomplete type definitions in STL containers, which is undefined behaviour according the c++ standard. As it was, GCC / libstc++ handles this behaviour with problems; however, on systems with clang/libc++ using incomplete type definitions in STL containers results in compilation errors. Instead a solution involving shared pointers has been proposed. More rational the change is discussed in the upstream pull request.
The upstream pull request is necessary for this change to be merged.