search

Jmeyer1292 / opw_kinematics

Closed form IK for parallel base, spherical wrist industrial manipulators
Apache License 2.0
71 stars 26 forks source link

Make opw_kinematics a pure cmake package #20

Closed Levi-Armstrong closed 5 years ago

Levi-Armstrong commented 5 years ago

I believe it would be beneficial to make this a pure cmake package since it does not leverage ros directly to allow for it to be leverage by the broader opensource community.

Levi-Armstrong commented 5 years ago

@Jmeyer1292 Do you have any objection to making this a pure cmake package?

Levi-Armstrong commented 5 years ago

@gavanderhoorn What are your thoughts?

gavanderhoorn commented 5 years ago

Makes sense to me. With catkin_tools and colcon this will still build in a ROS workspace, and with a 3rd party release it would still be releasable through the ROS buildfarm.

(I know we can release non-ROS pkgs using other means (PPAs fi), but I doubt that is going to be very convenient)

Levi-Armstrong commented 5 years ago

(I know we can release non-ROS pkgs using other means (PPAs fi), but I doubt that is going to be very convenient)

I agree, I think this should be avoided for main release, but we could leverage this to provide nightly builds and versioned builds. I have done this before and it works nicely where you can link it to a github repository and when new commits are pushed to master it would create and debian automatically.

gavanderhoorn commented 5 years ago

Slightly off-topic, but apparently there is a "official Kitware APT repository": apt.kitware.com.

This supports installing CMake 3.14 on Ubuntu Xenial fi.

Levi-Armstrong commented 5 years ago

@gavanderhoorn I think I came up with a solution that does not require a different branch, let me know what you think?

Levi-Armstrong commented 5 years ago

@gavanderhoorn You OK with the new changes?

gavanderhoorn commented 5 years ago

Ha, nice work-arounds.

I'd be ok with doing it like this for now.

If/when we become really unhappy about this we can always go for the branching we discussed above.

Levi-Armstrong commented 5 years ago

@gavanderhoorn Sounds good to me. Thank you for the review.

gavanderhoorn commented 5 years ago

Please note that this package will not build any more in workspaces that contain Catkin packages if users are using plain catkin_make.

You might want to make note of that somewhere, as it's a breaking change.

haudren commented 5 years ago

Hey guys, I just noticed that this change is breaking our Xenial builds. Time to upgrade I guess, but was this really intended? Seems like the plan was to not have branches.

gavanderhoorn commented 5 years ago

@haudren: can you please open a new issue, describe your problems there (include copy-pastes of errors) and reference this PR?

gavanderhoorn commented 5 years ago

@haudren: tracking the breakage in #23.