search

merose / diff_drive

ROS nodes for controlling and monitoring a differential drive robot.
BSD 3-Clause "New" or "Revised" License
129 stars 61 forks source link

Only publish succes on succes, not on shutdown or preempt #9

Closed Timple closed 1 year ago

merose commented 6 years ago

I'm not sure this change is correct. If the service request is preempted or canceled, then the server must still send the result, albeit with a "False" success indication. What problem are you trying to solve?

merose commented 6 years ago

On further thought, omitting the message on shutdown is correct (though I'm not sure it would cause a problem), but I'm still not sure about a preempted goal. I'm worried about a client hanging because the service request never finishes. Please convince me.

Timple commented 6 years ago

I have a moving goal which I am trying to follow. Therefore I am streaming goals to your node. But I get the following log output:

[INFO] [1533532707.023159]: Goal: (1.000000,0.000000,0.000000)
[INFO] [1533532707.125228]: Goal preempted
[ERROR] [1533532707.128091]: To transition to a succeeded state, the goal must be in a preempting or active state, it is currently in state: 2
[INFO] [1533532707.230104]: Goal: (1.000000,0.000000,0.000000)
[INFO] [1533532707.325252]: Goal preempted
[ERROR] [1533532707.327762]: To transition to a succeeded state, the goal must be in a preempting or active state, it is currently in state: 2
[INFO] [1533532707.429853]: Goal: (1.000000,0.000000,0.000000)
[INFO] [1533532707.525231]: Goal preempted

According to the actionlib description a goal is preempted when a new goal arrives. And according to this image no setSucceeded or setAborted can or needs to be called.

And to ease your worries, a result is actually already published by the action server and can be reproduces with these steps:

roslaunch diff_drive demo.launch
rostopic echo /diff_drive_go_to_goal/result
rostopic pub -r0.3 /diff_drive_go_to_goal/goal diff_drive/GoToPoseActionGoal "{goal: {pose: {pose: {position: {x: 100}}}}}"

But I am not saying there might not be a better way of checking the state of the goal (or actionserver) than self.goal is None