Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional physics features #62

Merged
merged 3 commits into from
Apr 22, 2020
Merged

Optional physics features #62

merged 3 commits into from
Apr 22, 2020

Conversation

chapulina
Copy link
Contributor

Reopening this BitBucket PR, resolves #58.


Overview

Reduce MinimumFeatureList so that physics engines are not required to implement all features, and some of them are treated as optional.

Bullet

This enables running Bullet with Ignition. You must use ign-physics' bullet_chapulina branch. ⚠️ As of that branch, Bullet is not fully functional yet though. But it loads 🙂

Implementation

This PR makes the following features optional:

  • Features for detachable joints:

    • ignition::physics::AttachFixedJointFeature
    • ignition::physics::DetachJointFeature
    • ignition::physics::SetJointTransformFromParentFeature
  • Features for bounding box:

    • ignition::physics::GetModelBoundingBox
  • Features for joint velocity commands:

    • ignition::physics::SetJointVelocityCommandFeature
  • Features for 3D meshes:

    • ignition::physics::mesh::AttachMeshShapeFeature
  • For each group of optional features:

    • Create a feature list, i.e. DetachableJointFeatureList
    • Create entity types which use that list, i.e. JointDetachableJointPtrType
    • Create an entity map that holds the specialized entity type, i.e. entityJointDetachableJointMap
    • Use entityCast to cast from the original entity, created with MinimumFeatureList, to the specialized entity. entityCast will also populate the specialized map the first time it's called and get the entity from that map on subsequent calls. This means that RequestFeature is only called once for each combination of entity + feature list.

Any engine implementing all the minimum features will be loaded. If, at runtime, there's an attempt to use an unsupported feature, a warning is printed. That is:

  • If a component::DetachableJoint is created, a warning is printed and the joint isn't attached or detached.
  • If a components::AxisAlignedBox is created, a warning is printed and the box is not populated.
  • If a components::JointVelocityCmd is created, a warning is printed and the command has no effect.
  • If a collision has a mesh geometry, a warning is printed and the geometry isn't created.

I'll remove features not needed by TPE on a subsequent PR, that's currently on the chapulina/tpe branch.

@chapulina chapulina added this to Inbox in Core development via automation Apr 16, 2020
@chapulina
Copy link
Contributor Author

@osrf-jenkins run tests

@chapulina chapulina moved this from Inbox to In review in Core development Apr 18, 2020
@azeey
Copy link
Contributor

azeey commented Apr 20, 2020

@chapulina, should we conclude the test failures from thils thread were flaky tests?

@chapulina
Copy link
Contributor Author

should we conclude the test failures from thils thread were flaky tests?

No, I think they're introduced by this PR. I'll take a look. Also, I'd like to refactor this a bit to prevent some Each calls for unsupported features. Sorry I only remembered all this after reopening the PR.

@chapulina chapulina self-assigned this Apr 20, 2020
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Contributor Author

should we conclude the test failures from thils thread were flaky tests?

No, I think they're introduced by this PR.

@azeey , I just realized that the test passed on the latest OSX build. 🤔

I'd like to refactor this a bit to prevent some Each calls for unsupported features.

Done on db37fb6. I had thought of caching an engine pointer for each set of features and then just checking that pointer before some Each calls, but I thought that may be overkill? Went instead with breaking the Each loop. Let me know what you think.

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina merged commit 52b3c05 into ign-gazebo2 Apr 22, 2020
Core development automation moved this from In review to Done Apr 22, 2020
@chapulina chapulina deleted the chapulina/bullet branch April 22, 2020 22:24
nkoenig pushed a commit that referenced this pull request May 6, 2020
* Add and use entityCast helper function

* return early for missing features

* move minimum map out of entityCast

Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Nate Koenig <nate@openrobotics.org>
nkoenig pushed a commit that referenced this pull request May 6, 2020
* Add and use entityCast helper function

* return early for missing features

* move minimum map out of entityCast

Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Nate Koenig <nate@openrobotics.org>
@chapulina chapulina added the 📜 blueprint Ignition Blueprint label Jun 3, 2020
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants