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

Added additional pose offset for the performer detector #236

Merged
merged 6 commits into from
Jul 13, 2020

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Jul 9, 2020

It's currently not possible to set the pose of an <include>ed performer detector relative to the parent model.

For example, consider this parent model:

<sdf version="1.6">
    <model name="cave_starting_area">
        <static>true</static>
        <include>
          <uri>https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Cave Staging Exit Detector</uri>
        </include>
....
  </model>
</sdf>

And the included Cave Staging Exit Detector.

<sdf version="1.6">
    <model name="cave_staging_exit_detector">
        <static>true</static>
        <pose>13.5 0 0 0 0 0</pose>
       <plugin filename="libignition-gazebo-performer-detector-system.so" name="ignition::gazebo::systems::PerformerDetector">
            <topic>/subt_performer_detector</topic>
            <geometry>
                <box>
                    <size>4 30 20</size>
                </box>
            </geometry>
        </plugin>
    </model>

The <pose> in Cave Staging Exit Detector is not used by the plugin, because of the <include> process. You also cannot set the <pose> in the <include> statement for the same reason.

This means the plugin is always located at 0 0 0 0 0 0 in the parent model.

This PR adds an optional <pose> to the plugin to address this situation.

Signed-off-by: Nate Koenig [email protected]

@nkoenig nkoenig requested a review from azeey July 9, 2020 04:04
@nkoenig nkoenig requested a review from chapulina as a code owner July 9, 2020 04:04
@github-actions github-actions bot added the 📜 blueprint Ignition Blueprint label Jul 9, 2020
@codecov
Copy link

codecov bot commented Jul 9, 2020

Codecov Report

Merging #236 into ign-gazebo2 will increase coverage by 0.19%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo2     #236      +/-   ##
===============================================
+ Coverage        62.14%   62.34%   +0.19%     
===============================================
  Files              123      123              
  Lines             6102     6102              
===============================================
+ Hits              3792     3804      +12     
+ Misses            2310     2298      -12     
Impacted Files Coverage Δ
src/SimulationRunner.cc 88.77% <0.00%> (+2.40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3539321...e470f13. Read the comment docs.

Changelog.md Outdated Show resolved Hide resolved
src/systems/performer_detector/PerformerDetector.hh Outdated Show resolved Hide resolved
nkoenig and others added 3 commits July 9, 2020 04:36
Co-authored-by: iche033 <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Signed-off-by: Nate Koenig <[email protected]>
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! My only concern with this PR is that the nesting example you gave, even with this PR, leads to unexpected results. i.e, with parent model:

<sdf version="1.6">
    <model name="cave_starting_area">
        <static>true</static>
        <include>
          <uri>https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Cave Staging Exit Detector</uri>
           <pose>0 0 10 0 0 0</pose>
        </include>
....
  </model>
</sdf>

And the included Cave Staging Exit Detector.

<sdf version="1.6">
    <model name="cave_staging_exit_detector">
        <static>true</static>
        <pose>13.5 0 0 0 0 0</pose>
       <plugin filename="libignition-gazebo-performer-detector-system.so" name="ignition::gazebo::systems::PerformerDetector">
            <topic>/subt_performer_detector</topic>
            <geometry>
                <box>
                    <size>4 30 20</size>
                </box>
            </geometry>
        </plugin>
    </model>

The expected pose of the detector's box in world frame is 13.5 0 10 0 0 0, but the result will still be 0 0 0 0 0 0.

@nkoenig
Copy link
Contributor Author

nkoenig commented Jul 13, 2020

LGTM! My only concern with this PR is that the nesting example you gave, even with this PR, leads to unexpected results. i.e, with parent model:

<sdf version="1.6">
    <model name="cave_starting_area">
        <static>true</static>
        <include>
          <uri>https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Cave Staging Exit Detector</uri>
           <pose>0 0 10 0 0 0</pose>
        </include>
....
  </model>
</sdf>

And the included Cave Staging Exit Detector.

<sdf version="1.6">
    <model name="cave_staging_exit_detector">
        <static>true</static>
        <pose>13.5 0 0 0 0 0</pose>
       <plugin filename="libignition-gazebo-performer-detector-system.so" name="ignition::gazebo::systems::PerformerDetector">
            <topic>/subt_performer_detector</topic>
            <geometry>
                <box>
                    <size>4 30 20</size>
                </box>
            </geometry>
        </plugin>
    </model>

The expected pose of the detector's box in world frame is 13.5 0 10 0 0 0, but the result will still be 0 0 0 0 0 0.

Yup, thi

LGTM! My only concern with this PR is that the nesting example you gave, even with this PR, leads to unexpected results. i.e, with parent model:

<sdf version="1.6">
    <model name="cave_starting_area">
        <static>true</static>
        <include>
          <uri>https://fuel.ignitionrobotics.org/1.0/OpenRobotics/models/Cave Staging Exit Detector</uri>
           <pose>0 0 10 0 0 0</pose>
        </include>
....
  </model>
</sdf>

And the included Cave Staging Exit Detector.

<sdf version="1.6">
    <model name="cave_staging_exit_detector">
        <static>true</static>
        <pose>13.5 0 0 0 0 0</pose>
       <plugin filename="libignition-gazebo-performer-detector-system.so" name="ignition::gazebo::systems::PerformerDetector">
            <topic>/subt_performer_detector</topic>
            <geometry>
                <box>
                    <size>4 30 20</size>
                </box>
            </geometry>
        </plugin>
    </model>

The expected pose of the detector's box in world frame is 13.5 0 10 0 0 0, but the result will still be 0 0 0 0 0 0.

Yup, that is not intuitive, but I don't know how to fix that right now.

Signed-off-by: Nate Koenig <[email protected]>
@nkoenig nkoenig merged commit 39c9f7d into ign-gazebo2 Jul 13, 2020
@nkoenig nkoenig deleted the performer_detector_pose branch July 13, 2020 19:14
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.

3 participants