ros2 / ros2cli

ROS 2 command line interface tools
Apache License 2.0
173 stars 159 forks source link

feat(echo --clear): add --clear option to echo #819

Open Guillaumebeuzeboc opened 1 year ago

Guillaumebeuzeboc commented 1 year ago

Adds the --clear/-c option to echo.

Solves this issue: https://github.com/ros2/ros2cli/issues/725

This was previously available in ROS.

This clears the screen before a new message is displayed.

ESC[2J corresponds to erase entire screen ESC[H corresponds to moves cursor to home position (0, 0)

I had to build rolling from source to successfully run the tests

Guillaumebeuzeboc commented 1 year ago

implementation looks good to me but many tests are failing.

could you address the following failures?

https://build.ros2.org/job/Rpr__ros2cli__ubuntu_jammy_amd64/238/testReport/ros2topic.ros2topic.test/test_cli/test_cli/

https://build.ros2.org/job/Rpr__ros2cli__ubuntu_jammy_amd64/238/testReport/ros2topic.ros2topic.test/test_echo_pub/test_echo_pub/

I had the same failure with the HEAD of rolling for this repo. I had to rebuild all rolling from source for them to pass. I believe the CI is using the debians of rolling right? I pulled all the repo from HEAD.

clalancette commented 1 year ago

I believe the CI is using the debians of rolling right?

Yes, that's correct, though as of yesterday, they are the same thing. That said, these tests have indeed been flaky here for a while, and we haven't had time to track it down. So I wouldn't worry about those two in particular right now.

ahemwe commented 1 year ago

Does that PR require further changes?

clalancette commented 1 year ago

Does that PR require further changes?

Yes, we need to come to a conclusion on this discussion.