ipab-slmc / exotica

Extensible Optimization Framework
https://ipab-slmc.github.io/exotica
BSD 3-Clause "New" or "Revised" License
149 stars 70 forks source link

[exotica_core] Implement velocity & acceleration joint limit exposure #715

Closed the-raspberry-pi-guy closed 4 years ago

the-raspberry-pi-guy commented 4 years ago

This PR addresses the fact that there is no way to get velocity/acceleration limits from a URDF using EXOTica. The edits here add this functionality. In detail, the changes I have made are:

wxmerkt commented 4 years ago

I am not sure that we should extend the joint position limit matrix. I would instead add extra vectors for velocity limits and acceleration limits. What do you think @VladimirIvan?

the-raspberry-pi-guy commented 4 years ago

I am not sure that we should extend the joint position limit matrix. I would instead add extra vectors for velocity limits and acceleration limits. What do you think @VladimirIvan?

That was Vlad's decision, but can be amended.

wxmerkt commented 4 years ago

Once we update our KinematicTree to quaternions, we will have different sizes of configuration and tangent vectors. It's also not standard in the community and other software to mix position, velocity, and acceleration limits. In a model we want to keep track of (a) configuration (position) limits, (b) velocity limits, (c) acceleration limits [if defined, not always!], and (d) effort limits.

the-raspberry-pi-guy commented 4 years ago

Once we update our KinematicTree to quaternions, we will have different sizes of configuration and tangent vectors. It's also not standard in the community and other software to mix position, velocity, and acceleration limits. In a model we want to keep track of (a) configuration (position) limits, (b) velocity limits, (c) acceleration limits [if defined, not always!], and (d) effort limits.

This makes sense to me though will see @VladimirIvan thoughts to why I implemented it this way.

the-raspberry-pi-guy commented 4 years ago

So what exactly is the consensus on this? Have a new vector with velocity and acceleration limits? Or 2 new vectors? Or keep in the same vector? I won't instantiate acceleration limits by default - so does that mean having them as inf still?

Consensus saves my computer struggling through extra builds ;)

Also, I notice that my PR failed some integration checks - what does this mean exactly? Is this a Kinetic issue? I am developing on Melodic and so unsure how to fix.

wxmerkt commented 4 years ago
  1. Add a new vector for each effort and acceleration limit
  2. You can populate the effort limit vector with information from the URDF. Acceleration limits should be inf by default?
  3. Add a boolean flag "has_acceleration_limits" and get_has_acceleration_limits, get_acceleration_limits(), set_acceleration_limits(a_max)
  4. Expose effort limits with get_effort_limits
  5. You can check why the build server failed by clicking on "Details" next to Travis. It is most likely either build or formatting. In your case it's formatting: https://travis-ci.org/github/ipab-slmc/exotica/builds/686184166 - check the documentation for how to apply formatting. Short answer: Install clang-format-3.9 and run the "apply-format" script in the root of the repository.
  6. Try ccache to save yourself some compilation time. Even though changing KinematicTree will force a recompile of everything...
the-raspberry-pi-guy commented 4 years ago

In the latest commits, I have made the requested changes. Velocity and acceleration limits are now in separate vectors, acceleration defaults to inf, and has_acceleration_limits defaults to false (and changes to true when they are set using the setter).

ccache was helpful in speeding up my compile times (whole build of EXOTica is 20-30 mins on my PC).

For the formatting @wxmerkt, do you mean run the command given in the PR window: find -name '*.cpp' -o -name '*.h' -o -name '*.hpp' | xargs clang-format-3.9 -style=file -i. When I run this it runs without error and returns nothing? Or are you referring to something else?

the-raspberry-pi-guy commented 4 years ago

Implemented all of the changes - that should be everything: name changes, checking velocity/acceleration sizes, setting to inf/throwing errors if incorrect, checking for bounds before joint limit reset etc.

Also ran the apply_format script again so this should pass all formatting integration tests.

the-raspberry-pi-guy commented 4 years ago

That should be everything

the-raspberry-pi-guy commented 4 years ago

Removed default values and == true

wxmerkt commented 4 years ago

Codacy Here is an overview of what got changed by this pull request:


Clones added
============
- exotica_core/src/kinematic_tree.cpp  2

See the complete overview on Codacy