squirrel-project / squirrel_driver

0 stars 14 forks source link

Safety now works. #165

Closed ghost closed 6 years ago

tpatten commented 6 years ago

Hi Philip,

Out robot is alive again! I tested this pull request and saw that it safely kills rqt when any emergency is triggered.

ghost commented 6 years ago

Cool. Yeah, this is standard behavior now, as we just unload the controller upon triggering the safety and upon reset we reload it. So rqt has to crash - but this assures that it does not continue publishing anything. Unfortunately this unloading/loading seems to be the suggested way to do it.

tpatten commented 6 years ago

What is the behaviour with, e.g., the motion planner? It can't cause it to crash like rqt.

ghost commented 6 years ago

We plan to test this tomorrow. In theory the planner is completely unaware of that as it just published to a topic. For sure, while the controller is not running, we also have no joint information for planning which however is no issues, as the robot anyways won't execute anything. But again, for now there's no other way to achieve the desired behavior using ROS control (in one of the mails I added a link to a github discussion about that topic - unloading/loading is the moral of the story).

tpatten commented 6 years ago

Would there be an issue, say when the planner is sending commands, the safety is triggered, the safety is reset before the planner finishes and the controller resumes moving the arm half way through the trajectory?

ghost commented 6 years ago

Of course. That's something we cannot handle at that level but instead has to be done by the planner. So ideally @philkark also listens to that topic. However, it also depends on how he sends the trajectories. One-by-one or the full trajectory. In the latter case there is no problem, in the former one can construct a problem.

tpatten commented 6 years ago

Yes makes sense. In the case when commands are given one-by-one not much can be done on the controller side. The motion planner can easily check the safety state as well. But I don't think this is the case anyway, @philkark always sends full trajectories.

ghost commented 6 years ago

I see. Well, then we're fine!

ipa-nhg commented 6 years ago

@lokalmatador tested and working?

ghost commented 6 years ago

Tested and working!

tpatten commented 6 years ago

The safety aspect of this PR is working, however, the commit regarding the odom-map offset is not yet working properly. Could we revert and just merge the first two commits while we still work on the odom-map problem.