osrf / ovc

the Open Vision Computer
Apache License 2.0
198 stars 42 forks source link

ROS Drivers for OVC5 #46

Closed gbalke closed 3 years ago

gbalke commented 3 years ago

This PR will introduce ROS1/2 drivers for OVC. My intent is to use https://github.com/romainreignier/share_ros1_ros2_lib_demo as an example for building the libs in a convenient way.

TODO:

luca-della-vedova commented 3 years ago

I was halfway through reviewing when I saw that quite a bit of the diff is just changing style for parenthesis from

void fun()
{
   // stuff
}

to

void fun() {
    // stuff
}

Regardless of which way is better (I personally prefer the first but I know it's a pretty trivial matter) I think the codebase is pretty consistent in the newline style and this style change makes the diff a lot larger, as well as introduce inconsistencies between all the files that have been touched by this PR and all of those that were not.

gbalke commented 3 years ago

I was halfway through reviewing when I saw that quite a bit of the diff is just changing style for parenthesis from

void fun()
{
   // stuff
}

to

void fun() {
    // stuff
}

Regardless of which way is better (I personally prefer the first but I know it's a pretty trivial matter) I think the codebase is pretty consistent in the newline style and this style change makes the diff a lot larger, as well as introduce inconsistencies between all the files that have been touched by this PR and all of those that were not.

I use :FormatCode from https://github.com/google/vim-codefmt. I'll try to adjust the formatter to match this style requirement and give you a copy of what I get (or maybe just include it in the root of the repo). I'd rather just always run code through it rather than having to have formatting "holy wars" :smile:

gbalke commented 3 years ago

Waiting on #48 to merge.

gbalke commented 3 years ago

I'll give this a test tomorrow before merging.