Make sure acceleration limits match velocity limits #46
Conversation
include/libada/detail/Ada-impl.hpp
Outdated
| ? getAccelerationLimits() | ||
| : accelerationLimits; | ||
| auto sentVelocityLimits | ||
| = (velocityLimits.squaredNorm() == 0.0 |
There was a problem hiding this comment.
This condition is a bit of a doozy -- maybe pull it out into a separate variable?
include/libada/detail/Ada-impl.hpp
Outdated
| = (velocityLimits.squaredNorm() == 0.0 | ||
| || (std::size_t)velocityLimits.size() | ||
| != mArm->getMetaSkeleton()->getNumDofs()) | ||
| ? mArm->getMetaSkeleton()->getVelocityUpperLimits() |
There was a problem hiding this comment.
It seems that the current implementations of Ada::getVelocityLimits() and Ada::getAccelerationLimits() return the limits of the whole robot, including the fingers. I'm not especially familiar with this codebase, but are you using those finger limits at all? If not, why not change those methods (and maybe some other Ada methods, to be consistent) to just operate on the arm rather than the whole robot?
There was a problem hiding this comment.
We are not using the finger limits, so I agree that just sticking with the arm limits should work just fine. Changed.
| : accelerationLimits; | ||
| bool velLimitsInvalid | ||
| = (velocityLimits.squaredNorm() == 0.0) | ||
| || velocityLimits.size() != getVelocityLimits().size(); |
There was a problem hiding this comment.
Nit: I wonder if getVelocityLimits().size() should instead be path->getStateSpace()->getDimension()?
Fixing a quick bug introduced from the last PR.
We only need the first 6 joints of the acceleration limits (to match the size of the velocity limits).
Before creating a pull request
make formatBefore merging a pull request