robotology / unicycle-footstep-planner

Repository for the Unicycle-based FootStep Planner.
BSD 3-Clause "New" or "Revised" License
20 stars 11 forks source link

New path interpolation method to sample footsteps from a path of poses/waypoints #60

Closed SimoneMic closed 1 year ago

SimoneMic commented 1 year ago

PR for discussion of:

S-Dafarra commented 1 year ago

Hi @SimoneMic. please let me know when the PR is ready to be reviewed. I quickly checked the code. Here some very preliminary comments:

Maybe can you explain better what you are trying to achieve. Why do you need the additional methods?

SimoneMic commented 1 year ago

Hi @SimoneMic. please let me know when the PR is ready to be reviewed. I quickly checked the code. Here some very preliminary comments:

* There is a typo: `setLateralVelocityConservaiveFactor` -> `setLateralVelocityConservativeFactor`

* Please avid to duplicate the logic of `computeNewSteps`. It is the most complex bit of the library, and having two copies of them is not desirable.

Maybe can you explain better what you are trying to achieve. Why do you need the additional methods?

For what regards the purpose, I need to actually follow a path of X, Y, and Theta poses, which is not supported by the actual method. Also, I need to be able to rotate the robot in place if I have a too big yaw difference between two poses, which is not possible to do with the actual method. Furthermore, I don't have any error due to a virtual controller, this means that the footsteps will be sample from a path which is exactly following the given waypoints. In this way, we don't have a unique very complex method, but two (with some redundancies) less complicated.

Under an architecture point of view, the evaluation of the path could be put in another function and shared. But it is more about how do you want to structure the whole module overall. Moreover we could imagine a unique computeNewSteps method divided into four sections: init, integration/interpolation, evaluation, footsteps finalization. But I think that this is more a topic for a separate discussion

SimoneMic commented 1 year ago

Closing in favor of #62