Fix bug with action server (issue #95)

MisterOwlPT commented 1 year ago

Hi :)

I saw the open issue #95 and decided to fix the reported bug. As reported, an error occurs when you try to run the ROS action example from the documentation. The proposed changes solve the said bug.

What type of change is this?

[x] Bug fix in a backwards-compatible manner.
[ ] New feature in a backwards-compatible manner.
[ ] Breaking change: bug fix or new feature that involve incompatible API changes.
[ ] Other (e.g. doc update, configuration, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

[x] I added a line to the CHANGELOG.rst file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
[x] I ran all tests on my computer and it's all green (i.e. invoke test). NOTE: One test is failing. However, I checked the main branch and that same test also failed there so I don't think it's related to my changes.
[x] I ran lint on my computer and there are no errors (i.e. invoke check).
[ ] I have added tests that prove my fix is effective or that my feature works.
[ ] I have added necessary documentation (if appropriate)

Some remarks

After some debugging, I think I've found the origin of the problem.

It seems that the action client was receiving the result all along! However, the client's wait function always raises the shown exception because no status message is being published with adequate SUCCEEDED and PREEMPTED status codes by the action server. Since these messages are not published, the client's is_finished property never returns True and therefore the client keeps on waiting for the result (even after having received it).

After ensuring that proper status messages are published inside the server's set_succeeded and set_preempted functions, the following now happens when running the documentation example:

$ python3 ros-action-client.py 
[0, 1, 1]
[0, 1, 1, 2]
[0, 1, 1, 2, 3]
[0, 1, 1, 2, 3, 5]
[0, 1, 1, 2, 3, 5, 8]
[0, 1, 1, 2, 3, 5, 8, 13]
[0, 1, 1, 2, 3, 5, 8, 13, 21]
Result: [0, 1, 1, 2, 3, 5, 8, 13, 21]

Checking published messages with rostopic echo the following message are now detected:

---
header: 
  seq: 6
  stamp: 
    secs: 1670093700
    nsecs: 961247206
  frame_id: ''
status_list: 
  - 
    goal_id: 
      stamp: 
        secs: 0
        nsecs:         0
      id: "goal_0.6644001782365995_1670093700955"
    status: 3
    text: ''
---

No such message was shown before.

Please let me know what you think of these changes. If they seem okay to you, I can implement some unit tests later :)

gonzalocasas commented 1 year ago

Thanks! On a first glance, it looks ok, I'll check a bit more in detail and we merge. In the meantime, could you please add your name to the AUTHORS.rst and also try merging main to your branch, because that should fix the build issue? thanks!

MisterOwlPT commented 1 year ago

Hi :) Suggested alterations were done.

gramaziokohler / roslibpy