search

borglab / GTDynamics

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

removed robot_ in GTDynamics class hierarchy #252

Closed dellaert closed 3 years ago

dellaert commented 3 years ago

This was already mostly done in Disha's PR #224 but I decided to cleanly separate it.

varunagrawal commented 3 years ago

Why are we passing in links when we can pass in robot and call robot.links()?

dellaert commented 3 years ago

Why are we passing in links when we can pass in robot and call robot.links()?

Where?

dellaert commented 3 years ago

Why are we passing in links when we can pass in robot and call robot.links()?

I see now. That method is broken and will be fixed in next PR.

@varunagrawal can you look over and approve? I asked @jerredchen but he's really a co-author.

varunagrawal commented 3 years ago

I'm a bit lost as to why robot is no longer a class variable since the way the robot is right now, it doesn't store any joint/link data other than the names/IDs and the initial config. This is just making us pass in an extra parameter in multiple places when we could have just passed it in the constructor.

Can you please explain the rationale here?

dellaert commented 3 years ago

Sounds good. Will address all comments in Disha’s PR.