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

rostopic: repeatedly republish message from file #1635

Merged
merged 2 commits into from
Mar 9, 2019

Conversation

cwecht
Copy link
Contributor

@cwecht cwecht commented Feb 28, 2019

This replaces #1348

@cwecht
Copy link
Contributor Author

cwecht commented Feb 28, 2019

hm, looks like the build-infastructure had an hiccup ...

@dirk-thomas
Copy link
Member

@ros-pull-request-builder retest this please

if exactly_one_message:
exactly_one_message = False
break
exactly_one_message = True
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this alter the content returned by the iterator in the later loop - in the case of the iterator being stdin_yaml_arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's right. The fix makes not much sense for stdin stream anyway, so I limited it to actual files now.

@cwecht
Copy link
Contributor Author

cwecht commented Mar 1, 2019

flaky tests again...

@cwecht
Copy link
Contributor Author

cwecht commented Mar 8, 2019

@ros-pull-request-builder retest this please

@dirk-thomas
Copy link
Member

Thanks for the patch and for iterating on it.

Ignoring the flaky test on CI...

@dirk-thomas dirk-thomas merged commit 7e6bd99 into ros:melodic-devel Mar 9, 2019
tahsinkose pushed a commit to tahsinkose/ros_comm that referenced this pull request Apr 15, 2019
* rostopic: repeatedly republish message from file

* rostopic pub: limit 'exactly one message'-fix to real files, not streams
@pavloblindnology
Copy link

substitute_keywords argument isn't used in this scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants