Convert package to a pure CMake package

[moriarty]

This PR started with a simple cherry-pick and has grown to include several other cherry-picks and review feedback. It is to address #283

cherry-picking commit 4d5be00 from @cottsay See: https://github.com/wjwwood/serial/pull/209#issuecomment-520018303

Author: Scott K Logan [email protected] Date: Wed Jul 3 13:24:15 2019 -0700

• Conflicts:
  • CMakeLists.txt
  • package.xml
  • tests/CMakeLists.txt

[moriarty]

In testing this locally it looks like I'll also need to cherry-pick

https://github.com/wjwwood/serial/commit/d8d160678aa0b31cdf467c052b954fa287cc6cdf

But this might just be due to newer system warnings

[moriarty]

I tried to cherry-pick https://github.com/wjwwood/serial/commit/d8d160678aa0b31cdf467c052b954fa287cc6cdf from @tylerjw but it didn't apply cleanly so I just took the few lines directly in 380c4e4

However, I'll take a look through: https://github.com/wjwwood/serial/pull/231 from @leamas and try to follow up on the this comment https://github.com/wjwwood/serial/pull/231#issuecomment-859110462 to get a working shared version instead of just the static library

@wjwwood would you want the shared object version done in a follow up PR?

[moriarty]

FYI @tonybaltovski do you have time to give this a look over or a test? I know you left a comment here https://github.com/wjwwood/serial/pull/209#issuecomment-1538507835 that you're also depending on it and potentially going to fork it.

[moriarty]

Generally this lgtm, though it seems like we either need to address the lingering conversations with changes or new issues before merging.

I stopped editing this because I was not getting any feedback from you, and wanted to avoid what happened to all the previous attempts and pull requests to this repository.

If you’re okay with the lingering issues I’ll go ahead and make those changes.

Also, if you guys decide to fork, then maybe this can be done on the fork?

We were trying to avoid forming, and one reason to move to ros-drivers and not fork is because of how GitHub handles tracking forks and forks of forks. Issues opened on this repo would be lost on a fork, the link between forks of this repo and the forked repo would be lost or difficult to find.

Are you oppose to both having an additional maintainer on this project, and moving it to a common ros org… or just oppose #284 to moving it ?

[wjwwood]

I'm fine with the changes being proposed, but I think we should decide about moving/forking/add maintainers first.

I'll reply about moving vs forking on the #284 issue.

[moriarty]

@wjwwood or @leamas if we do fork to ros-drivers any input or suggestions on the name?

@leamas you've released this to debian as libcxx-serial-dev I would then try to get the rest of your changes in to keep from #231 previously I just skimmed it for what looked like the MVP pure CMake for the ROS 2 usecase

@wjwwood the serial name is two generic for the https://www.ros.org/reps/rep-0144.html ?

[wjwwood]

I answered the same question on the other issue, but yes it's too generic after the updates to rep-144.

I suggested ros_drivers_serial, but you guys can decide.

[leamas]

I don't really care either, but it would be nice to keep the ability to configure with an alternative name as of 5f1ab387b7dd7b. This is partly about Debian users, so they don't need to cope with a new library name.

It is also about me, changing the Debian package name is a convoluted process...

[moriarty]

I don't really care either, but it would be nice to keep the ability to configure with an alternative name as of 5f1ab38. This is partly about Debian users, so they don't need to cope with a new library name.

It is also about me, changing the Debian package name is a convoluted process...

Yes that makes sense don't want to make the debian package process any harder.

Would it make sense to take the cxx-serial name as the default name? ROS packages are still released to debian as ros-${ROS_DISTRO}-package-name eg: ros-iron-cxx-serial == libcxx-serial-dev ?

[leamas]

ROS is basically a mystery for me, haven't worked with it. That said, for me it would of course be convenient to use what I already use i. e., cxx-serial it it's ok with other stakeholders.

[moriarty]

Sorry have been out due to work/travel... Still out actually but I pushed added the src build of the ros2_robotiq_gripper to ros distro https://github.com/ros/rosdistro/pull/37857

@wjwwood do you have permission to create a fork of this repo on the github.com/ros-drivers org?

[wjwwood]

Yes, I can do that, what name did we land on?

[moriarty]

I answered the same question on the other issue, but yes it's too generic after the updates to rep-144.

I suggested ros_drivers_serial, but you guys can decide.

There are already these two ros specific serial drivers:

https://github.com/ros-drivers/rosserial https://github.com/ros-drivers/transport_drivers/tree/main/serial_driver

Since this is already released in debian by @leamas I would use the debian name, cxx-serial

[wjwwood]

For me, ros_drivers_serial reads as "The org ros_drivers's serial library", not "a ROS specific serial driver".

However cxx_serial is fine with me, but not that it should be cxx_serial and not cxx-serial in my opinion. The package name must use _ and I think to be consistent with other ROS packages the repository name should match the package name.

[wjwwood]

Let me know if cxx_serial is fine and I'll set that up. Also let me know who should be contributors to it initially.

[tylerjw]

Sounds good to me. @moriarty, @leamas, and yourself?

[leamas]

I can live with this, for sure. Naming the package cxx-serial even if the upstream is named cxx_serial should be fine. After all, I have named it cxx-serial for an upstream named serial...

EDIT: However, this only flies of we can keep 5f1ab387b7.

[moriarty]

I can live with this, for sure. Naming the package cxx-serial even if the upstream is named cxx_serial should be fine. After all, I have named it cxx-serial for an upstream named serial...

EDIT: However, this only flies of we can keep 5f1ab38.

Yes, I will go over your previous PR #231 and keep as much of your work, I really don't want to create a hard fork I'd rather the debian and he ros pkg be essentially the same.

EDIT: @leamas I'll aslo take your cmake related variable naming conventions which were heavily discussed above.

[wjwwood]

EDIT: However, this only flies of we can keep https://github.com/wjwwood/serial/commit/5f1ab387b7dd7bc9f08bd5d8961a7e101a5dcca2.

We can't keep that because of the package.xml.in stuff. Also the CMake project name would need to be cxx_serial and not cxx-serial.

[wjwwood]

Ok, what I'm going to do is make a fork on my personal account, move any issues that should be moved, then move it to the ros-drivers once we've done that.

[wjwwood]

New repository is at https://github.com/wjwwood/cxx_serial, I invited you @moriarty and I'll try to triage the issues tonight and move the repository too. But I have to go for now.

[moriarty]

New repository is at https://github.com/wjwwood/cxx_serial, I invited you @moriarty and I'll try to triage the issues tonight and move the repository too. But I have to go for now.

Thanks! I'll take a look tomorrow morning.

[nagayev]

@moriarty Hey, whats the PR status now?

[moriarty]

@moriarty Hey, whats the PR status now?

I plan to cherry-pick parts of it with the PR feedback from above to the new repository. Unfortunately priorities shifted but I still plan to get back to it

[nagayev]

@moriarty Can I help you?