ros / roslisp_common

Clone of the old roslisp_common SVN repo from code.ros.org.
8 stars 14 forks source link

Another TF implementation suggestion #31

Closed gaya- closed 8 years ago

gaya- commented 8 years ago

Major differences:

gaya- commented 8 years ago

Oh, GitHub even remembered the old discussions, nice.

airballking commented 8 years ago

@gaya- With the SAPHARI review over, I will look into this this week. Is this PR still current? I'll see about this merge conflict that github is reporting.

gaya- commented 8 years ago

Oh, wow, I didn't notice the merge conflict, it's something new I guess. The PR is current, I've been testing this for some time now.

gaya- commented 8 years ago

The conflict is because of the last commit on the master branch from 8 days ago. The file corresponding to cl_tf2/src/message-conversions.lisp is cl_transforms_stamped/src/ros-message.lisp in this PR. I'll leave the merging to you then as you're the author of the commit.

gaya- commented 8 years ago

No conflicts.

gaya- commented 8 years ago

Will need to merge https://github.com/ros/roslisp_common/pull/32 first though (for the sake of cleanness).

airballking commented 8 years ago

@gaya- Thanks for unifying the two interface. Obviously, I have done this differently. But I think we can also continue with your suggestion. Thanks for putting in the work!

gaya- commented 8 years ago

Thanks for merging this PR.

Here is a final summary of what changed for those interested.

The point of the Pull Request was to create a unified interface for the two TF implementations - cl_tf and cl_tf2 for TF and TF2 correspondingly - such that the user code could be independent of the actual implementation. That is, the user writes their code using the interface (implemented mostly through generic functions) and later runs the code using a specific instance of TF.