Added bodies::Body::computeBoundingBox (aligned box version). by peci1 · Pull Request #104 · moveit/geometric_shapes

peci1 · 2019-05-22T15:27:45Z

Solves #89 for axis-aligned bounding boxes.

peci1 · 2019-05-22T15:32:16Z

One thing that needs attention: I needed to make use of class AABB which I originally added about a year ago to moveit::core . But since moveit_core depends on this package and not the other way round, I needed to copy the class' definition.

I think it anyways makes more sense for the AABB class to defined in geometric_shapes. My suggestion on how to tackle code duplication is the following: wait until this PR is merged and new version of geometric_shapes released to public, and then I can send a PR to moveit that will remove the class from there and leave there only a typedef to bodies::AABB for backwards compatibility. So there will be just a short time window where the same class will be defined at two places (in different namespaces, though).

BryceStevenWilley

Minor nit with the test, but otherwise great!

BryceStevenWilley · 2019-05-25T20:02:40Z

include/geometric_shapes/aabb.h

+*     copyright notice, this list of conditions and the following
+*     disclaimer in the documentation and/or other materials provided
+*     with the distribution.
+*   * Neither the name of Willow Garage, Inc. nor the names of its


Suggested change

* * Neither the name of Willow Garage, Inc. nor the names of its

* * Neither the name of Open Robotics, nor the names of its

And elsewhere.

BryceStevenWilley · 2019-05-25T20:05:39Z

test/test_bounding_box.cpp

+
+  // box mesh
+
+  m.vertices[3 * 0 + 0] = -1;


Factor out the mesh making to a separate function

peci1 · 2019-05-25T22:02:15Z

I did the requested fixes.

BryceStevenWilley · 2019-05-25T22:25:07Z

Forgot to clang-format, and for some reason, the tests are failing? Can't tell why however.

peci1 · 2019-05-25T22:38:44Z

Okay, I formatted the code. I also have no idea why the test didn't, run, so let's see if it wasn't just a random glitch.

peci1 · 2019-05-25T23:27:27Z

Hmm, I still don't see why exactly is the test being not launched. However - do you think it's even correct to test this PR against kinetic-devel if it is only targeted for melodic?

BryceStevenWilley · 2019-05-28T03:10:07Z

It is strange, but since the melodic branch still builds on kinetic, I don't we want this PR to break that. I've got a kinetic machine that I have access to, I'll try to run that test there and see if I can debug it this week.

peci1 · 2019-05-29T03:40:51Z

It segfaults in test MeshBoundingBox.Mesh1. This is the stack trace:

#0  _int_malloc (av=av@entry=0x7ffffe1f4b20 <main_arena>, bytes=bytes@entry=352) at malloc.c:3516
#1  0x00007ffffdeb4184 in __GI___libc_malloc (bytes=352) at malloc.c:2913
#2  0x00007ffffee89df3 in Eigen::internal::aligned_malloc (size=352) at /usr/include/eigen3/Eigen/src/Core/util/Memory.h:224
#3  0x00007ffffee8afa8 in Eigen::internal::conditional_aligned_malloc<true> (size=352) at /usr/include/eigen3/Eigen/src/Core/util/Memory.h:305
#4  0x00007ffffee8a8da in bodies::ConvexMesh::MeshData::operator new (size=352) at /mnt/d/programovani/ROS/ubuntu16/ws/src/geometric_shapes/include/geometric_shapes/bodies.h:476
#5  0x00007ffffee8600f in bodies::ConvexMesh::useDimensions (this=0x7ffffffec220, shape=0x7ffffffec1b0) at /mnt/d/programovani/ROS/ubuntu16/ws/src/geometric_shapes/src/bodies.cpp:739
#6  0x00007ffffee81fea in bodies::Body::setDimensions (this=0x7ffffffec220, shape=0x7ffffffec1b0) at /mnt/d/programovani/ROS/ubuntu16/ws/src/geometric_shapes/src/bodies.cpp:111
#7  0x0000000000449b24 in bodies::ConvexMesh::ConvexMesh (this=0x7ffffffec220, shape=0x7ffffffec1b0) at /mnt/d/programovani/ROS/ubuntu16/ws/src/geometric_shapes/include/geometric_shapes/bodies.h:418
#8  0x0000000000446b78 in MeshBoundingBox_Mesh1_Test::TestBody (this=0x6bcce0) at /mnt/d/programovani/ROS/ubuntu16/ws/src/geometric_shapes/test/test_bounding_box.cpp:322
#9  0x00007fffff1d39e8 in testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void> (object=0x6bcce0, method=&virtual testing::Test::TestBody(), location=0x7fffff1e00c3 "the test body")
    at /usr/src/gtest/src/gtest.cc:2078
#10 0x00007fffff1cea3d in testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void> (object=0x6bcce0, method=&virtual testing::Test::TestBody(), location=0x7fffff1e00c3 "the test body")
    at /usr/src/gtest/src/gtest.cc:2114
#11 0x00007fffff1b37be in testing::Test::Run (this=0x6bcce0) at /usr/src/gtest/src/gtest.cc:2151
#12 0x00007fffff1b400e in testing::TestInfo::Run (this=0x6c1760) at /usr/src/gtest/src/gtest.cc:2326
#13 0x00007fffff1b46a7 in testing::TestCase::Run (this=0x6c1850) at /usr/src/gtest/src/gtest.cc:2444
#14 0x00007fffff1bb82e in testing::internal::UnitTestImpl::RunAllTests (this=0x6c08e0) at /usr/src/gtest/src/gtest.cc:4315
#15 0x00007fffff1d4941 in testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> (object=0x6c08e0,
    method=(bool (testing::internal::UnitTestImpl::*)(testing::internal::UnitTestImpl * const)) 0x7fffff1bb55c <testing::internal::UnitTestImpl::RunAllTests()>,
    location=0x7fffff1e08d0 "auxiliary test code (environments or event listeners)") at /usr/src/gtest/src/gtest.cc:2078
#16 0x00007fffff1cf8b3 in testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> (object=0x6c08e0,
    method=(bool (testing::internal::UnitTestImpl::*)(testing::internal::UnitTestImpl * const)) 0x7fffff1bb55c <testing::internal::UnitTestImpl::RunAllTests()>,
    location=0x7fffff1e08d0 "auxiliary test code (environments or event listeners)") at /usr/src/gtest/src/gtest.cc:2114
#17 0x00007fffff1ba3cc in testing::UnitTest::Run (this=0x7fffff3fc8c0 <testing::UnitTest::GetInstance()::instance>) at /usr/src/gtest/src/gtest.cc:3926
#18 0x0000000000449d5c in RUN_ALL_TESTS () at /usr/include/gtest/gtest.h:2288
#19 0x0000000000448eb6 in main (argc=1, argv=0x7ffffffec9c8) at /mnt/d/programovani/ROS/ubuntu16/ws/src/geometric_shapes/test/test_bounding_box.cpp:386

I'll keep investigating

peci1 · 2019-05-29T04:12:45Z

The tests are fixed now.

peci1 · 2019-06-18T13:47:29Z

Friendly ping. Who could be the 2nd reviewer for this?

v4hn

looks good to merge to me.

Could you please cleanup the commit history before?

peci1 · 2019-06-18T16:58:34Z

Done.

v4hn · 2019-07-03T11:33:41Z

Thanks! sorry, I lost track of this for a while.

peci1 · 2019-08-07T14:14:09Z

Hi @v4hn , could you please make a new release of geometric_shapes so that this feature is available in the released packages, too?

rhaschke · 2019-08-08T06:42:32Z

Before preparing a new release of geometric_shapes, we should take care of some other pending PRs.
@ros-planning/moveit-maintainers: Looks like nobody takes care of this repo actively. Please have a look at @peci1's PRs that are open since May...

peci1 force-pushed the bounding_box branch 2 times, most recently from ab30d16 to 555cfc7 Compare May 22, 2019 15:38

peci1 mentioned this pull request May 24, 2019

Added bodies::Body::computeBoundingBox (oriented box version). #105

Merged

BryceStevenWilley suggested changes May 25, 2019

View reviewed changes

BryceStevenWilley approved these changes May 29, 2019

View reviewed changes

BryceStevenWilley added the awaits 2nd approval label May 29, 2019

v4hn reviewed Jun 18, 2019

View reviewed changes

Added bodies::Body::computeBoundingBox (aligned box version).

4c8931d

peci1 force-pushed the bounding_box branch from 165316d to 4c8931d Compare June 18, 2019 16:46

v4hn merged commit f8ca209 into moveit:melodic-devel Jul 3, 2019

	* * Neither the name of Willow Garage, Inc. nor the names of its
	* * Neither the name of Open Robotics, nor the names of its

Conversation

peci1 commented May 22, 2019

Uh oh!

peci1 commented May 22, 2019

Uh oh!

BryceStevenWilley left a comment

Choose a reason for hiding this comment

Uh oh!

BryceStevenWilley May 25, 2019

Choose a reason for hiding this comment

Uh oh!

BryceStevenWilley May 25, 2019

Choose a reason for hiding this comment

Uh oh!

BryceStevenWilley May 25, 2019

Choose a reason for hiding this comment

Uh oh!

peci1 commented May 25, 2019

Uh oh!

BryceStevenWilley commented May 25, 2019

Uh oh!

peci1 commented May 25, 2019

Uh oh!

peci1 commented May 25, 2019

Uh oh!

BryceStevenWilley commented May 28, 2019

Uh oh!

peci1 commented May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peci1 commented May 29, 2019

Uh oh!

peci1 commented Jun 18, 2019

Uh oh!

v4hn left a comment

Choose a reason for hiding this comment

Uh oh!

peci1 commented Jun 18, 2019

Uh oh!

v4hn commented Jul 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peci1 commented Aug 7, 2019

Uh oh!

rhaschke commented Aug 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

peci1 commented May 29, 2019 •

edited

Loading

v4hn commented Jul 3, 2019 •

edited

Loading