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

fix(actionlib): Remove getState error when no goal is running #97

Merged
merged 1 commit into from
Feb 5, 2019
Merged

fix(actionlib): Remove getState error when no goal is running #97

merged 1 commit into from
Feb 5, 2019

Conversation

alireza-hosseini
Copy link
Contributor

As explained in #40 in many situations the developer needs to know if the action client is tracking a goal or not which is possible by the getState method of the SimpleActionClient. But the problem is, using this function while the action client didn't send any goal before, results to the following error:

Trying to getState() when no goal is running. You are incorrectly using SimpleActionClient

This PR simply removes this ROS error since it's not really an incorrect usage of the SimpleActionClient (closes #40).

 - calling the `getState` function while no goal is running shouldn't
   report any error.
@alireza-hosseini
Copy link
Contributor Author

I checked the Jenkins report for the failed jobs, apparently, they are not related to my changes.

@alireza-hosseini
Copy link
Contributor Author

Is there any plan to review this simple PR?

@jschleicher
Copy link
Contributor

Looks good to me. @mikaelarguedas Could you please review and merge this change?

@mikaelarguedas
Copy link
Member

@alireza-hosseini @jschleicher This looks good to me, I agree it's not really an invalid usage of the simple action client.
I originally held this as I was planning on releasing it only for ROS Melodic to not change behavior for people that may be expecting these logging messages.

I'll leave it up to the current maintainer to review / merge / release but you have my 👍

@jschleicher
Copy link
Contributor

@mikaelarguedas merging this for kinetic onwards would be fine for me. On the other hand this logging message shouldn't break any existing code (and is ABI compatible of course).

Who is the current maintainer?

@mikaelarguedas
Copy link
Member

On the other hand this logging message shouldn't break any existing code (and is ABI compatible of course).

agreed 👍

Who is the current maintainer?

@mjcarroll is the current maintainer

Copy link
Member

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution.

@mjcarroll mjcarroll merged commit a178e16 into ros:indigo-devel Feb 5, 2019
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.

SimpleActionClient is tracking goal
4 participants