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.

Sign up for GitHub

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

Jump to bottom

Move trigger logic to base Sensor class and enable trigger for all camera sensors #431

Merged
merged 5 commits into from
May 15, 2024
Merged

Move trigger logic to base Sensor class and enable trigger for all camera sensors #431

merged 5 commits into from
May 15, 2024

Conversation

shameekganguly
Copy link
Contributor

@shameekganguly shameekganguly commented May 10, 2024 •
edited
Loading

…mera sensors

🦟 Bug fix

Partially addresses #426

Summary

This PR moves the trigger logic from CameraSensor to the Sensor base class. Additionally, if Update() is called with _force = true, data will be always generated irrespective of whether a trigger is pending or not.
To do this, the following changes are made:

  • Moved trigger logic to base Sensor class
  • Added new Sensor class methods SetTriggered and HasPendingTrigger
  • Set all camera-type sensors to correctly enable triggered mode if it is specified in the SDF
  • Update integration tests

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@shameekganguly
Move trigger logic to base Sensor class and enable trigger for all ca…
d954dff 
…mera sensors

Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label May 10, 2024
@shameekganguly
Update tests to not force image rendering
0e1856e 
Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
@shameekganguly shameekganguly marked this pull request as ready for review May 10, 2024 21:10
@shameekganguly shameekganguly requested a review from iche033 as a code owner May 10, 2024 21:10
iche033
iche033 reviewed May 13, 2024
View reviewed changes
include/gz/sensors/Sensor.hh Outdated Show resolved Hide resolved
include/gz/sensors/Sensor.hh Outdated Show resolved Hide resolved
src/Sensor.cc Outdated Show resolved Hide resolved
@shameekganguly
Updated as per review comments
b8a6410 
Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
@shameekganguly
Remove unnecessary stdlib headers
ffe2466 
Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
@shameekganguly
Remove one more unnecessary stdlib header
ad59aca 
Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
@shameekganguly shameekganguly requested a review from iche033 May 14, 2024 22:44
@shameekganguly shameekganguly mentioned this pull request May 14, 2024
Merged
iche033
iche033 approved these changes May 15, 2024
View reviewed changes
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

looks good to me

@iche033 iche033 merged commit 930a4a0 into gazebosim:gz-sensors8 May 15, 2024
8 of 9 checks passed
@azeey azeey changed the title Move trigger logic to base Sensor class and enable trigger for all ca… Move trigger logic to base Sensor class and enable trigger for all camera sensors May 20, 2024
@iche033 iche033 mentioned this pull request Nov 20, 2024
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iche033 iche033 iche033 approved these changes

Assignees
No one assigned
Labels
🎵 harmonic Gazebo Harmonic
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@shameekganguly @iche033