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

Adds reference link to complete test of rrtest. #391

Closed
wants to merge 28 commits into from

Conversation

Voldivh
Copy link

@Voldivh Voldivh commented Jun 2, 2023

This PR addresses an item from #366 according to this discussion.

Yadunund and others added 28 commits March 15, 2023 12:28
Signed-off-by: Yadunund <[email protected]>
This matches the current best practice, which we updated
in Iron.

Signed-off-by: Chris Lalancette <[email protected]>
…row#348)

There is no '_sync' version, so this must have been a typo for
the '_async' version, which does exist.

Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Yadunund <[email protected]>
Signed-off-by: Yadunund <[email protected]>
* Replace executables with link to demos readme

Signed-off-by: Yadunund <[email protected]>

* address feedback

Signed-off-by: Yadunund <[email protected]>

---------

Signed-off-by: Yadunund <[email protected]>
checks:
- name: Check `rttest_plot`
try:
- stdin: ros2 run rttest rttest_plot
- stdin: pendulum_demo -f pendulum_demo_results
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, I'm a bit conflicted here. On the one hand, this command is absolutely correct for Iron. On the other hand, for Rolling we ended up changing this (via ros2/demos#624) to ros2 run pendulum_control pendulum_demo -f pendulum_demo_results. So if we end up copying this for Jazzy (which we almost certainly will), this will no longer be correct.

@Yadunund is going to do some work here to split these configurations out of this repository. I would suggest we hold off on this one until we do that, at which point we can add in a Jazzy set of configuration. And then we can merge this one into the Iron configuration, and the updated one into the Jazzy configuration.

Copy link
Author

Choose a reason for hiding this comment

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

Ok sounds good!

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.

3 participants