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

Example component node usage #60

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

tylerjw
Copy link
Contributor

@tylerjw tylerjw commented Sep 6, 2022

Fixes #56

This also fixes some deadlock conditions that were possible and updates the example readme.

@tylerjw tylerjw force-pushed the component_node branch 2 times, most recently from 1ac1259 to 442471c Compare September 6, 2022 01:50
Copy link
Contributor

@destogl destogl left a comment

Choose a reason for hiding this comment

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

A few things go clarify before merging. Next time would be great to get such PR split into multiple ones.

We now have:

  1. Fixing potential deadlocks in generated code
  2. Renaming executable name in the CMakeLists.txt
  3. Creating Component Node example.

example/src/minimal_publisher.cpp Outdated Show resolved Hide resolved
admittance_controller::Params params_;
};

int main(int numArgs, const char** args) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that for the Compoennts one doesn't need main. Nevertheless, would it make sense to have both examples, Component Node and "normal" Node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you create a component node it also creates an executable. You can see from the cmake and the readme of the example that there is still an executable and this file is now redundant.

@tylerjw tylerjw force-pushed the component_node branch 3 times, most recently from d41dd06 to 4c0519b Compare September 6, 2022 14:55
@tylerjw
Copy link
Contributor Author

tylerjw commented Sep 6, 2022

@destogl These are now three separate PRs. Hopefully, that makes this easier to review.

)
rclcpp_components_register_node(minimal_publisher
PLUGIN "admittance_controller::MinimalPublisher"
EXECUTABLE test_node
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@destogl when you create a component node, it now creates a stand-alone executable to use that node with this macro. This is what makes the main.cpp redundant as it is now generated for us.

Copy link
Contributor

@destogl destogl left a comment

Choose a reason for hiding this comment

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

This looks fine to me. I am just not sure how to test it. I have tried using ros2 run but I am missing the parameters file to add as argument.

@tylerjw
Copy link
Contributor Author

tylerjw commented Sep 7, 2022

The readme in the example directory has instructions for running it.

@tylerjw tylerjw merged commit 7b8c0a9 into PickNikRobotics:main Sep 7, 2022
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.

Examples showing proper use within a Component library
2 participants