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.

Sign up for GitHub

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

Jump to bottom

Add error message if ros2controlcli commands fail #309

Merged
merged 1 commit into from
Feb 1, 2021
Merged

Add error message if ros2controlcli commands fail #309

merged 1 commit into from
Feb 1, 2021

Conversation

v-lopez
Copy link
Contributor

@v-lopez v-lopez commented Jan 26, 2021

No description provided.

@v-lopez v-lopez requested a review from destogl January 26, 2021 17:17
@v-lopez v-lopez mentioned this pull request Jan 26, 2021
Merged
@destogl
Copy link
Member

destogl commented Jan 26, 2021

@v-lopez what do you think about #308? do you have any idea how to combine it with this PR?

@v-lopez
Copy link
Contributor Author

v-lopez commented Jan 26, 2021

#308 is right, I assumed the return value was ignored, but it had to be negated.

I still like the error messages added here, we could add to #308, but I don't know what's the standard practice. ros2param doesn't use exceptions, but ros2service does.

@destogl
Copy link
Member

destogl commented Jan 27, 2021

@bmagyar what do you think? which way to go?

@bmagyar
Copy link
Member

bmagyar commented Jan 28, 2021

Yeah I think the error messages are valuable but we should pass the return values as done in #308 . That one is ready to merge so let's rework this on top of that

Print error messages if ros2controlcli commands fail
578ee68 
Signed-off-by: Victor Lopez <victor.lopez@pal-robotics.com>
@v-lopez v-lopez changed the title Throw error if cli call fail, because return value is ignored Add error message if ros2controlcli commands fail Jan 29, 2021
@v-lopez
Copy link
Contributor Author

v-lopez commented Jan 29, 2021

Done, feel free to take a look.

@bmagyar bmagyar merged commit f506c2a into ros-controls:master Feb 1, 2021
mahaarbo pushed a commit to mahaarbo/ros2_control that referenced this pull request Feb 4, 2021
@v-lopez @mahaarbo
Print error messages if ros2controlcli commands fail (ros-controls#309)
d1246a0 
Signed-off-by: Victor Lopez <victor.lopez@pal-robotics.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmagyar bmagyar bmagyar approved these changes

@destogl destogl Awaiting requested review from destogl

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@v-lopez @destogl @bmagyar