tesseract-robotics / tesseract

Motion Planning Environment
http://tesseract-docs.rtfd.io
Other
278 stars 88 forks source link

Make checkKin optional in debug #1043

Closed marrts closed 1 month ago

marrts commented 2 months ago

If a manipulator group has a lot of joints the kinematic check can take a very long time. This optionally removes that check which was previously always run in debug mode.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.60%. Comparing base (a2750a9) to head (369ddb8). Report is 38 commits behind head on master.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/tesseract-robotics/tesseract/pull/1043/graphs/tree.svg?width=650&height=150&src=pr&token=nh4aHZzgpR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tesseract-robotics)](https://app.codecov.io/gh/tesseract-robotics/tesseract/pull/1043?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tesseract-robotics) ```diff @@ Coverage Diff @@ ## master #1043 +/- ## ========================================== - Coverage 89.86% 89.60% -0.27% ========================================== Files 280 284 +4 Lines 15908 15990 +82 ========================================== + Hits 14296 14328 +32 - Misses 1612 1662 +50 ``` [see 25 files with indirect coverage changes](https://app.codecov.io/gh/tesseract-robotics/tesseract/pull/1043/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tesseract-robotics)
Levi-Armstrong commented 2 months ago

I do not think this is necessary because it should only run if built in debug mode. This should run if you build in release.

Levi-Armstrong commented 2 months ago

Is this check running when built in release?

marrts commented 2 months ago

Is this check running when built in release?

No, but I was developing in debug with a high DOF system and it was taking 30+ seconds to run the check which was annoying. At first I thought something broke.

I figured this way it wouldn't introduce any breaking changes but would just allow users to get around it if they need to test something else

Levi-Armstrong commented 2 months ago

I would prefer not to make this type of change, but instead update the if statement to also check if TESSERACT_ENABLE_TESTING is enabled which should solve the issue you are having. These types of checks are intended to be ran during unit tests.

Levi-Armstrong commented 1 month ago

Replaced by PR #1048