search

ami-iit / bipedal-locomotion-framework

Suite of libraries for achieving bipedal locomotion on humanoid robots
https://ami-iit.github.io/bipedal-locomotion-framework/
BSD 3-Clause "New" or "Revised" License
146 stars 37 forks source link

Track the feature upgrades related to FloatingBaseEstimators library and related features #135

Open prashanthr05 opened 3 years ago

prashanthr05 commented 3 years ago

This epic will track the pending PRs/issues that will introduce feature updates to BipedalLocomotion::FloatingBaseEstimators library relevant to the online base estimation using state of the art filters and the DILIGENT filter.

Currently, the library contains the implementation of InvariantEKF.

The pending PRs include,

Additionally, while porting the code from https://github.com/dic-iit/element_floating-base-estimation/tree/devel/code/cpp/libraries/humanoid-estimators/src/estimation/include/HumEst/iDynTree/Estimation to BLF, I introduced a regression in the way contacts were handled. Earlier I was handling an arbitrary number of contacts, while the current implementations in BLF handle only the two support feet contacts. This must be reverted to the original version of handling an arbitrary number of contacts, which scales up the use of the filters to multiple contacts and also EKF SLAM related problems.

Other changes should include,

cc @S-Dafarra @traversaro @GiulioRomualdi

traversaro commented 3 years ago

fyi @diegoferigo

prashanthr05 commented 3 years ago

A related ongoing epic https://github.com/dic-iit/component_wholebody-teleoperation/issues/297 towards testing the estimators in closed loop.

prashanthr05 commented 3 years ago

IMUBipedMatrixLieGroup This class was dependent on manif::SE_2_3 which was merged recently. The current implementation is available at https://github.com/prashanthr05/bipedal-locomotion-framework/tree/feature/IMUBipedMatrixLieGroup. A proper unit test is missing for this class. Once written, we can open a PR for code review.

Relating this, and also considering,

Earlier I was handling an arbitrary number of contacts, while the current implementations in BLF handle only the two support feet contacts. This must be reverted to the original version of handling an arbitrary number of contacts, which scales up the use of the filters to multiple contacts and also EKF SLAM related problems. Switch to using arbitrary contact features in the existing framework in a clean way.

I refactored the code in IMUBipedMatrixLieGroup into a more consolidated class considering arbitrary number of contacts in FloatingBaseExtendedKinematicsLieGroup also with a test in FloatingBaseExtendedKinematicsLieGroupTest.

I know I have been overloading you guys with many recent PR review requests. I don't want to bother you more with too many new PRs given that we are already busy with our other recent tasks. For the moment, I will keep introducing changes towards DILIGENT considering arbitrary contacts in the same branch prashanthr05/feature/DILIGENT. Once everything is working, I will spit the branch into multiple small PRs (atleast class wise). Let me know if this is ok? Or would you have an alternative suggestion?

S-Dafarra commented 3 years ago

IMUBipedMatrixLieGroup This class was dependent on manif::SE_2_3 which was merged recently. The current implementation is available at https://github.com/prashanthr05/bipedal-locomotion-framework/tree/feature/IMUBipedMatrixLieGroup. A proper unit test is missing for this class. Once written, we can open a PR for code review.

Relating this, and also considering,

Earlier I was handling an arbitrary number of contacts, while the current implementations in BLF handle only the two support feet contacts. This must be reverted to the original version of handling an arbitrary number of contacts, which scales up the use of the filters to multiple contacts and also EKF SLAM related problems. Switch to using arbitrary contact features in the existing framework in a clean way.

I refactored the code in IMUBipedMatrixLieGroup into a more consolidated class considering arbitrary number of contacts in FloatingBaseExtendedKinematicsLieGroup also with a test in FloatingBaseExtendedKinematicsLieGroupTest.

I know I have been overloading you guys with many recent PR review requests. I don't want to bother you more with too many new PRs given that we are already busy with our other recent tasks. For the moment, I will keep introducing changes towards DILIGENT considering arbitrary contacts in the same branch prashanthr05/feature/DILIGENT. Once everything is working, I will spit the branch into multiple small PRs (atleast class wise). Let me know if this is ok? Or would you have an alternative suggestion?

It seems reasonable. Do not worry too much about PRs. Do not hesitate to ping from time to time if a PR got stale. It is pretty easy to forget. In any case, for sure having a PR for a class may help. Nonetheless, be careful about interdependencies. Maybe some comments or change requests may break your already existing code and may become difficult to merge the new version.

prashanthr05 commented 3 years ago

Nonetheless, be careful about interdependencies. Maybe some comments or change requests may break your already existing code and may become difficult to merge the new version.

I have been trying to be careful with that (trying to rebase everytime over the latest updates). Let's see how it unfolds.

traversaro commented 3 years ago

It seems reasonable. Do not worry too much about PRs. Do not hesitate to ping from time to time if a PR got stale. It is pretty easy to forget.

Same for me.

prashanthr05 commented 3 years ago

cc @HosameldinMohamed