-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Cleaning up build exports and imports. #27
Conversation
This will automatically run uncrustify, cppcheck, and cpplint whenever you run the test suite (eg. colcon test) Also, somewhat unrelated, I added whitespace to package.xml to make it a bit easier to read.
src/control/CMakeLists.txt
Outdated
@@ -42,6 +42,20 @@ install(DIRECTORY include/ | |||
DESTINATION include/ | |||
) | |||
|
|||
if(BUILD_TESTING) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any way to include this section without duplicating in each of the CMakeLists.txt files? Perhaps include the boilerplate stuff that's not specific to a module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I agree. I've already got a few things I want to improve with the test invocation. I'll try to think of a way to do this. It might require having a seperate package with CMake utilities that every other package imports.
src/control/package.xml
Outdated
@@ -6,14 +6,18 @@ | |||
<description>TODO</description> | |||
<maintainer email="oregon.robotics.team@intel.com">Oregon Robotics Team</maintainer> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think having a generic e-mail account like this is worth pursuing? It could provide some stability as team members change. Eventually, if Intel no longer supported the project, it could be an account with an automatic response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have mixed feelings on that. It seems like a good idea, but none of the other projects do that.
Looks good to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
* WMO-56756 params for goal checker * remove unneeded changes * call teb goal checker * probable fix * reverting logic * add controller isGoalReached method * fix pose arg * remove dependencies on unused packages * add missing method * decrease goal tolerance. Give progress checker more time. * via point weight for pallet pickup * do not clear costmap. Wait longer before retry. * node name Co-authored-by: andriimaistruk <andriy.maistruk@logivations.com> Co-authored-by: liyuanlogi <li-yuan.hsu@logivations.com>
Co-authored-by: redvinaa <redvinaa@gmail.com>
I turned task and robot into actual packages that export their header files so that the other packages could pick them up without needing explicit paths. This seems to be more like the ros2 standard.
The rest is just cleanup to remove unnecessary statements given the new packages.