Fix broken industrial collision detection #77
Conversation
Does not git history achieve this "just in case" scenario? It seems to me keeping it will only confuse future contributors / developers and cause more maintenance hassle |
| @@ -176,9 +125,9 @@ namespace collision_detection | |||
| double dist_threshold = cdata->req->distance_threshold; | |||
| std::map<std::string, DistanceResultsData>::iterator it1, it2; | |||
|
|
|||
| if (!cdata->req->global) | |||
| if (false /*!cdata->req->global*/) | |||
There was a problem hiding this comment.
add comment for why you are disabling this
|
|
||
| if (!cdata->req->global) | ||
| if (false /*!cdata->req->global*/) |
@davetcoleman thanks for reviewing, indeed git does keep the history. My argument for keeping industrial_collision_detection around is that there may be exclusive functionality there that the constrained_ik planner still needs. I did my best to "upgrade" constrained_ik to the new moveit Distance Query Structures but some code didn't directly translate and so I could've broken something and it would be easier to fix/restore if industrial collision detection was kept. |
|
@davetcoleman I made the requested changes. |
Agreed - can you just copy your explanation in your previous comment into the README? |
|
@davetcoleman a REAME with an explanation of industrial_collision_detection has been added. |
jrgnicho
force-pushed
the
fix-broken-industrial-collision-detection
branch
from
cf72139
to
1527220
Compare
| } | ||
|
|
||
| fcl_result.min_distance = dist_threshold; | ||
| double d = fcl::distance(o1, o2, fcl::DistanceRequest(cdata->req->detailed), fcl_result); | ||
| double d = fcl::distance(o1, o2, fcl::DistanceRequest(), fcl_result); |
There was a problem hiding this comment.
needs to be fcl::DistanceRequest(cdata->req->enable_nearest_points).
| * MoveIt! library. Unfortunately the legacy code here isn't fully compatible with the new structure types and so for | ||
| * now the broken code below has been disabled until the discrepancies get amended. | ||
| */ | ||
| if (false /*!cdata->req->global*/) |
There was a problem hiding this comment.
cdata->req->type == DistanceRequestType::GLOBAL
| * MoveIt! library. Unfortunately the legacy code here isn't fully compatible with the new structure types and so for | ||
| * now the broken code below has been disabled until the discrepancies get amended. | ||
| */ | ||
| if (false /*!cdata->req->global*/) |
| dreq.enable_nearest_points = false; | ||
| dreq.enable_signed_distance = true; | ||
| dreq.group_name = req.group_name; | ||
| dreq.acm = acm; | ||
| DistanceResult dres; |
There was a problem hiding this comment.
add dreq.type == DistanceRequestType::GLOBAL
| dreq.enable_nearest_points = false; | ||
| dreq.enable_signed_distance = true; | ||
| dreq.group_name = req.group_name; | ||
| dreq.acm = acm; | ||
| DistanceResult dres; |
There was a problem hiding this comment.
add dreq.type == DistanceRequestType::GLOBAL
|
@Levi-Armstrong I've made the requested changes. |
This PR addresses the issue described in #70 where the industrial_collision_detection package broke after changes made to moveit collision library that introduced the ability to perform distance queries along with some other improvements. Thus this fix involves the following changes:
As of now the industrial_collision_detection package is no longer a dependency to any other package, nevertheless I chose to keep it around just in case.
This PR replaces #71 and #76
@Levi-Armstrong I had to modify constrained_ik in order for it to build so hopefully I didn't break anything but could you suggest a way to check?