Fix/py collision obj

[chhinze]

Adds Python bindings to get more complete Contact/Collision information, especially to get BodyNodes involved in a particular contact via someContact.collisionObject1.getShapeFrame().getBodyNodePtr(). The following changes are made

• CollisionObject's methods from C++ were completed
• CollisionObject declaration added in python/dartpy/collision/module.cpp -> Collision objects can be used now from Python code
• add getBodyNodePtr() for dartpy::dynamics::ShapeNode. Here, a discussion may be needed. As getBodyNodePtr() returns a dart::dynamics::BodyNodePtr (not implemented in dartpy yet) the easiest implementation was to return the raw BodyNode* using getBodyNodePtr().get(). So reference counting of dart will be wrong here. Can this become a problem, or is this okay? Should it be implemented via adding BodyNodePtr objects as well?

---

Before creating a pull request

• Document new methods and classes
• Format new code files using clang-format

Before merging a pull request

• Set version target by selecting a milestone on the right side
• Summarize this change in CHANGELOG.md
• Add unit test(s) for this change

[codecov]

Codecov Report

Merging #1457 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1457      +/-   ##
==========================================
+ Coverage   58.22%   58.23%   +0.01%     
==========================================
  Files         412      412              
  Lines       29923    29923              
==========================================
+ Hits        17422    17425       +3     
+ Misses      12501    12498       -3     

Impacted Files	Coverage Δ
dart/dynamics/Skeleton.cpp	66.16% <0.00%> (-0.17%)	⬇️
dart/dynamics/EulerJoint.cpp	73.00% <0.00%> (+3.68%)	⬆️

[jslee02]

So reference counting of dart will be wrong here. Can this become a problem, or is this okay?

This would be okay for most of the cases. The only downside is that we cannot take advantage of using BodyNodePtr, which ensures that the BodyNode (and its Skeleton) doesn't get deleted until we hold the BodyNodePtr. So we have to make sure the BodyNode (or Skeleton) is not deleted to keep the raw pointer valid.

Should it be implemented via adding BodyNodePtr objects as well?

It'd be ideal as we already exposed the object in:

dart/python/dartpy/dynamics/Node.cpp

Lines 87 to 97 in 35bd08e

	.def(
	"getBodyNodePtr",
	+[](dart::dynamics::Node* self) -> dart::dynamics::BodyNodePtr {
	return self->getBodyNodePtr();
	})
	.def(
	"getBodyNodePtr",
	+[](const dart::dynamics::Node* self)
	-> dart::dynamics::ConstBodyNodePtr {
	return self->getBodyNodePtr();
	})

which currently causes runtime error:

>>> b.getBodyNodePtr()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Unable to convert function return value to a Python type! The signature was
	(self: dartpy.dynamics.Node) -> dart::dynamics::TemplateBodyNodePtr<dart::dynamics::BodyNode>

[chhinze]

I just found out that there is a mapping of TemplateBodyNodePtr<T> as custom smart pointer already in

dart/python/dartpy/dynamics/BodyNode.cpp

Line 39 in 35bd08e

PYBIND11_DECLARE_HOLDER_TYPE(T, dart::dynamics::TemplateBodyNodePtr<T>, true);

As far as I understand, the PYBIND11_DECLARE_HOLDER_TYPE macro will implicitly convert the representation, so by changing the return values from dart::dynamics::BodyNodePtr to dart::dynamics::BodyNode* implicit conversion of this holder type would take place. This seems to be the pybind11 way to embed custom smart pointers (https://pybind11.readthedocs.io/en/stable/advanced/smart_ptrs.html#custom-smart-pointers). But the implicit conversion would cause the reference counting object to run out of scope after the function call. This would defeat the purpose of having a reference counting object in the first place, right?

If we won't go this way, the other solution would be to implement BodyNodePtr as a simple class. In this case, I have the following questions:

• Do you have a preferred place for the implementation?
• The only methods to implement would be get(), set() and constructors, or am I missing something?

[jslee02]

I just found out that there is a mapping of TemplateBodyNodePtr<T> as custom smart pointer already in

Good catch! As the holding type of BodyNode is BodyNodePtr, we can simply return BodyNodePtr instead returning the raw pointer (using .get()). The runtime error I mentioned above was just because PYBIND11_DECLARE_HOLDER_TYPE(T, dart::dynamics::TemplateBodyNodePtr<T>, true); is declared later than the Node bindings. The runtime error would disappear by adding the declaration in python/dartpy/dynamics/Node.cpp as well or moving it to

dart/python/dartpy/dynamics/module.cpp

Line 82 in 35bd08e

Let me create a PR to resolve the runtime error. Then you can simply return BodyNodePtr in this PR.

[jslee02]

#1465 is created based on this PR. Could you check the PR if it resolves the issues this PR attempts to fix? By #1465, you should be able to access the body node from a shape node as shape_node.getBodyNodePtr() that returns dart::dynamics::BodyNodePtr.

[jslee02]

Closing this PR in favor of #1465

[chhinze]

Thank you for your efforts.