-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
General improvement of DynamicalSystem and implementation of FixedBaseDynamics #242
Conversation
...DynamicalSystem/include/BipedalLocomotion/ContinuousDynamicalSystem/CompliantContactWrench.h
Show resolved
Hide resolved
...tinuousDynamicalSystem/include/BipedalLocomotion/ContinuousDynamicalSystem/DynamicalSystem.h
Outdated
Show resolved
Hide resolved
...tinuousDynamicalSystem/include/BipedalLocomotion/ContinuousDynamicalSystem/DynamicalSystem.h
Show resolved
Hide resolved
...nuousDynamicalSystem/include/BipedalLocomotion/ContinuousDynamicalSystem/FixedBaseDynamics.h
Outdated
Show resolved
Hide resolved
...ousDynamicalSystem/include/BipedalLocomotion/ContinuousDynamicalSystem/FixedStepIntegrator.h
Outdated
Show resolved
Hide resolved
...calSystem/include/BipedalLocomotion/ContinuousDynamicalSystem/FloatingBaseSystemKinematics.h
Outdated
Show resolved
Hide resolved
434d0fd
to
86953d6
Compare
- Integrators and DynamicalSystems are now in the ContinuousDynamicalSystem component - Enhance the CRPT in DynamicalSystem and Integrator
…cro to simplify the definition of a DynamicalSystem
86953d6
to
cea6f53
Compare
...tinuousDynamicalSystem/include/BipedalLocomotion/ContinuousDynamicalSystem/DynamicalSystem.h
Show resolved
Hide resolved
@@ -42,20 +65,10 @@ namespace System | |||
* - Eigen::VectorXd: the joint velocities [in rad/s]. | |||
* - DynamicalSystem::InputType is described by an std::tuple containing: | |||
* - Eigen::VectorXd: the joint torques [in Nm]; | |||
* - std::vector<ContactWrench>: List of contact wrenches. | |||
* - std::vector<CompliantContactWrench>: List of contact wrenches. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the class name in the documentation needs to be updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments.
...calSystem/include/BipedalLocomotion/ContinuousDynamicalSystem/FloatingBaseSystemKinematics.h
Outdated
Show resolved
Hide resolved
...calSystem/include/BipedalLocomotion/ContinuousDynamicalSystem/FloatingBaseSystemKinematics.h
Outdated
Show resolved
Hide resolved
...calSystem/include/BipedalLocomotion/ContinuousDynamicalSystem/FloatingBaseSystemKinematics.h
Show resolved
Hide resolved
} | ||
|
||
m_knownCoefficent *= -1; | ||
m_knownCoefficent.tail(m_actuatedDoFs) += jointTorques; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a comment here maybe, saying that jointTorques account for both internal and external stress?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I didn't understand your comment. Here actually we are interested only in the joint dynamics in case of zero external wrench applied on the system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what I meant to say was to add a comment saying that J'f = 0 in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what I meant to say was to add a comment saying that J'f = 0 in this case, but clearly I wrote something else. ;D
Hi @prashanthr05, in 95d596b I tried to improve the documentation of the classes |
Great, thanks @GiulioRomualdi !! It's a green flag from my side for this PR. |
@S-Dafarra let me know if you are fine with the PR 😃 |
* @param handler pointer to the parameter handler. | ||
* @return true in case of success/false otherwise. | ||
*/ | ||
bool initalize(std::weak_ptr<ParametersHandler::IParametersHandler> handler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: initalize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh noooooooooooooo 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 84e69ad (The same for the others)
* | `gravity` | `double` | Value of the Gravity. If not defined Math::StandardAccelerationOfGravitation is used | No | | ||
* @return true in case of success/false otherwise. | ||
*/ | ||
bool initalize(std::weak_ptr<ParametersHandler::IParametersHandler> handler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo initalize
* | `rho` | `double` | Baumgarte stabilization parameter over the SO(3) group. The default value is 0.01 | No | | ||
* @return true in case of success/false otherwise. | ||
*/ | ||
bool initalize(std::weak_ptr<ParametersHandler::IParametersHandler> handler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here
* | `rho` | `double` | Baumgarte stabilization parameter over the SO(3) group. The default value is 0.01 | No | | ||
* @return true in case of success/false otherwise. | ||
*/ | ||
bool initalize(std::weak_ptr<ParametersHandler::IParametersHandler> handler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
With this PR:
ContinuousDynamicalSystem
component