Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate ros2 devel #11

Merged
merged 16 commits into from
Jan 17, 2020
Merged

Conversation

Karsten1987
Copy link

@Karsten1987 Karsten1987 commented Jun 16, 2019

This is a first PR to import the work done by @bponsler and @MarcTestier.

I am trying to be non-intrusive in terms of style, but I am happy to run the batch of default ros2 linters. I leave that decision though to @tonybaltovski.

bponsler and others added 8 commits June 9, 2017 17:59
Initial port of the urg_c package to ros 2
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
@tonybaltovski
Copy link
Member

@Karsten1987 the default ros2 linters sound good to me.

@Karsten1987
Copy link
Author

I tried to enable the default linters (cpplint, uncrustify, etc.) but given the amount of utf-8 character and such applying the linters is basically impossible.
That being said, I'd leave the PR as-is and enable the linters on the follow up packages.

Copy link

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a couple of comments for improvement.

CMakeLists.txt Outdated Show resolved Hide resolved
current/src/urg_time.c Show resolved Hide resolved
The CMake target was named "liburg_c", which means that the
final library was "libliburg_c", which is just weird.  Rename
the target to urg_c, and also mark it as SHARED so it works
when building as a component.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@Karsten1987 Karsten1987 changed the title mirgate ros2 devel migrate ros2 devel Jan 13, 2020
Karsten1987 and others added 3 commits January 13, 2020 09:06
Rename the urg_c library and make it shared.
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
This reverts commit 83e68fb.
@Karsten1987
Copy link
Author

had to remove the static cast as it's a C-only library.

Signed-off-by: Karsten Knese <karsten@openrobotics.org>
@clalancette
Copy link

had to remove the static cast as it's a C-only library.

Oh, haha, my bad. I didn't realize that. Looks good 👍

Signed-off-by: Karsten Knese <karsten@openrobotics.org>
@clalancette
Copy link

@chadrockey @tonybaltovski This looks good to me now, and is passing CI. I think this is ready to merge. Please give it a look and let us know of any comments you have. If I don't hear anything by the end of the week, I'll go ahead and merge and we can follow-up later.

@clalancette
Copy link

I'm going to go ahead and merge this, but as always, feel free to leave comments and we can follow-up later.

@clalancette clalancette merged commit e9582f4 into ros-drivers:ros2-devel Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants