Improve how links with no collision elements are handled in contact detection

[diegoferigo]

Before this PR, if a scenario:gazebo::Link had no collisions, the method Link::contactsEnabled was returning true.

Refer to discussion below #334 (comment).

[traversaro]

So if I call link.enableContacts(true) if link has no contacts, link::contactsEnabled() will return false if it has not collision? This is not really intuitive, can you add a line in the documentation of contactsEnabled clarifying this aspect?

[diegoferigo]

I checked again the logic in the case you described.

The desiderata is the following:

Contacts disabled

1. If the link has collision elements, Link::enableContactDetection(true) returns true and enables contact support.
2. If the link has no collision elements, Link::enableContactDetection(true) returns false and prints an error message.
3. If the link has collision elements, Link::enableContactDetection(false) returns true and and does nothing.
4. If the link has no collision elements, Link::enableContactDetection(false) returns true and and does nothing.

Contacts enabled

5. If the link has collision elements, Link::enableContactDetection(true) returns true does nothing.
6. If the link has no collision elements: cannot happen.
7. If the link has collision elements, Link::enableContactDetection(false) returns true and and disables contact support.
8. If the link has no collision elements: cannot happen.

---

I spotted a problem in 2, the method was returning true without error message. Fixed in 7aaa8d1.

So if I call link.enableContacts(true) if link has no contacts, link::contactsEnabled() will return false if it has not collision? This is not really intuitive, can you add a line in the documentation of contactsEnabled clarifying this aspect?

Now, this scenario remains as you described, but the exit status of link.enableContacts(true) will be false with an error message, therefore it makes sense that a lated call to link::contactsEnabled() returns false if the link has no collisions.

[traversaro]

I think the thing that I do not understand why enabling contact on a link without collision should fail in the first place. Let's say I want to enable collision on all the links of a model, should I manually check if a link has a collision before calling enableContacts(true) ?

[traversaro]

Some comments inline, not a big problem anyhow.

[diegoferigo]

I think the thing that I do not understand why enabling contact on a link without collision should fail in the first place.

Mmh. Let's assume a user enables contact on a link with link.enableContactDetection(true) (returning true). Everything seems ok but if they call later link.inContact(), it will return false even if they can see from the GUI the link touching (in this case, penetrating) other shapes.

Let's say I want to enable collision on all the links of a model, should I manually check if a link has a collision before calling enableContacts(true) ?

The model-wise methods are actually more convoluted. After the previous modifications, Model::enableContacts enables contacts for all links, but returns false if there is at least one link with no collision elements (and this is quite common). Furthermore, Model::contactsEnabled returns true only if all links have collision elements and contact detection is enabled for all of them.

---

You're right that the logic is not optimal, especially for the model-wise methods. Let's see if we can find a workaround that prevents any kind of surprise:

• Let's assume to have a model that has some link with collisions elements, and some other without.
• We can assume that the user knows enough about the model they are using, however I'd like to add at least a high-verbosity message (e.g. sDebug or sMessage) in case of ambiguity.
• If a link has no collision elements, it will penetrate all other shapes (regardless if they belong to other models or self).
• If a link has no collision elements, calling link.enableContactDetection() returns true (so that model-wise methods work fine) and produce a sDebug print saying that the link has no collision elements.
• If a link has no collision elements, calling link.contactsEnabled always return true. This is the only weird point that is difficult to work around. Also here, a sDebug print could print that the link has no collision elements.
• All links with collision elements will work as expected (functionality and exit statuses).
• Calling Model::enableContacts(true) returns true if and only if contacts are successfully enabled on all links with collision elements. If the verbosity level is high, a lot of sDebug will be printed for links without collision elements.
• Calling Model::contactsEnabled returns true if and only if contacts are enabled on all links with collision elements. If the verbosity level is high, a lot of sDebug will be printed for links without collision elements.

How does it sound?

[traversaro]

I think the thing that I do not understand why enabling contact on a link without collision should fail in the first place.

Mmh. Let's assume a user enables contact on a link with link.enableContactDetection(true) (returning true). Everything seems ok but if they call later link.inContact(), it will return false even if they can see from the GUI the link touching (in this case, penetrating) other shapes.

In general, collision and visual shapes are different, so that can happen even if a link has a collision element, and that is the reason why I would personally prefer to keep the thing as simple as possible and just permit to enable collision on links without collisions. If a user want to check collision, it should visualize collision meshes, not the visual ones. However, if you think the complexity <--> advantaged tradeoff make sense, I think keeping things as they are is probably the safest option, not sure that even more complexity is helping.

[diegoferigo]

I think the thing that I do not understand why enabling contact on a link without collision should fail in the first place.

Mmh. Let's assume a user enables contact on a link with link.enableContactDetection(true) (returning true). Everything seems ok but if they call later link.inContact(), it will return false even if they can see from the GUI the link touching (in this case, penetrating) other shapes.

In general, collision and visual shapes are different, so that can happen even if a link has a collision element, and that is the reason why I would personally prefer to keep the thing as simple as possible and just permit to enable collision on links without collisions. If a user want to check collision, it should visualize collision meshes, not the visual ones. However, if you think the complexity <--> advantaged tradeoff make sense, I think keeping things as they are is probably the safest option, not sure that even more complexity is helping.

When writing my previous comment, I convinced myself that what wrote there, that I think complies with your comment, is the best approach. I try to implement it rolling back to a behavior that is more similar to the original implementation. At first sight, the real changes are a more verbose output for links with no collision elements.

[diegoferigo]

PR updated with the logic described in #334 (comment). Title updated accordingly. @traversaro thanks for the feedback!

[traversaro]

This seems much simpler so it is good for me, but why link::contactsEnabled() should always return true for link without collisions? It is not easier to return the value set by enableContacts?

[diegoferigo]

This seems much simpler so it is good for me, but why link::contactsEnabled() should always return true for link without collisions? It is not easier to return the value set by enableContacts?

If I understood what you meant, I have to point out a very very subtle thing to consider. Implementing what you proposed would mean adding a boolean value in the Link object. This information cannot be stored in the ECM nor inferred from it.

All the ScenarIO Gazebo classes (World, Model, Link, ...) are in theory kind-of stateless. And this means that objects can be destroyed and re-created being able to restore their state just by reading the ECM.

If I managed to clarify the situation, by inspecting the logic used in Link::contactsEnabled you can see that there is no way to make its output match what the user passed to Link::enableContactDetection(true|false) by using just the ECM.

[traversaro]

This seems much simpler so it is good for me, but why link::contactsEnabled() should always return true for link without collisions? It is not easier to return the value set by enableContacts?

If I understood what you meant, I have to point out a very very subtle thing to consider. Implementing what you proposed would mean adding a boolean value in the Link object. This information cannot be stored in the ECM nor inferred from it.

All the ScenarIO Gazebo classes (World, Model, Link, ...) are in theory kind-of stateless. And this means that objects can be destroyed and re-created being able to restore their state just by reading the ECM.

If I managed to clarify the situation, by inspecting the logic used in Link::contactsEnabled you can see that there is no way to make its output match what the user passed to Link::enableContactDetection(true|false) by using just the ECM.

I see, that make sense!