borglab / GTDynamics

Full kinodynamics constraints for arbitrary robot configurations with factor graphs.
BSD 2-Clause "Simplified" License
39 stars 10 forks source link

removing world context #238

Closed danbarla closed 3 years ago

danbarla commented 3 years ago

changes included in first commit:

  1. Link class refactored to hold one member : bMcom - link com in base frame at rest. wTl and lTcom removed. Constructor adjusted.
  2. sdf reading functions adjusted. joints are defined in link frames (not COM) so we need to read the bTl in some places.
  3. tests adjusted.

changes in second commit:

  1. only change of naming in few files from wTj to bTj.

still missing:

  1. need to adjust interface file (.i)
  2. check python wrapper and tests
  3. wTcom function still exists in Link class.
dellaert commented 3 years ago

@gchenfc and @yetongumich please fix CI to include jumping robot and cable robot ASAP? we will merge and break your code if CI says that’s OK.

dellaert commented 3 years ago

I think you are still working on this so just re-request a review when you need it.

dellaert commented 3 years ago

@danbarla Please (a) merge in master (b) let me know in reply what things are still missing from this PR (if any).

danbarla commented 3 years ago

@dellaert left to do:

  1. examples/example_quadruped_mp/main.cpp : wTb is still there in many places.
  2. tests/testInitializeSolutionUtils.cpp : wTb and wTl still widely used there
  3. add unit tests for the bMcom of the "body" link to be identity
  4. interface file, python wrapper and python tests.

do I need to open a new PR that merges my branch to master?

dellaert commented 3 years ago

do I need to open a new PR that merges my branch to master?

@danbarla you need to merge master into this branch, not the other way around. That way you get the latest updates from @varunagrawal and @gchenfc that will run the python unit tests in CI. To do this, open a terminal, make sure you are at the latest version of the branch, and do

git status # to make sure you’re in the branch
git pull
git merge master
dellaert commented 3 years ago

PS You might get conflicts, but probably not. I actually rarely use the command line, instead I use smartgit (free for non-commercial use) which makes it super easy to resolve conflicts.

danbarla commented 3 years ago

so I merged master to my branch. testContactPointFactor.cpp doesn't pass now (it uses wtl(), I think its a new test) I am also having some trouble with the python tests.

dellaert commented 3 years ago

Maybe we should try to meet? Send me an email

varunagrawal commented 3 years ago

@dellaert did we ever resolve the question about computing contact points if we don't have the wTl/bTl methods?

Another use case I've encountered is when using other people's code. E.g. when I use Pybullet, everything is in wTl and not wTcom so this requires me to manually add the transforms for lTcom. I imagine this will be the case for multiple scenarios and would affect interoperability with other libraries.

dellaert commented 3 years ago

@varunagrawal We will create an issue to bring back bTl as bMlink in a separate PR.