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

Fix SensorsBatteryState test #1529

Merged
merged 1 commit into from
Jun 13, 2022
Merged

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Jun 8, 2022

Signed-off-by: Ian Chen ichen@osrfoundation.org

🦟 Bug fix

Summary

This should fix the test failure when running INTEGRATION_sensors_system_battery. The expected image count is updated to allow for one extra image received.

More info:

#1480 broke this test. The side effect of that PR is that there is a change in timing of the rendering thread in sensors system - the rendering loop is now running faster before any subscribers and created. When the subscribers are created, the first camera / depth camera images are now rendered at an earlier timestep (because they do not need to wait for previous updates to finish). Because of this slight change in timing, they publish one extra image with the given time period.

Here are sample debug outputs showing the timestamp of the camera image, in sec nsec:

before:

0 124000000
0 154000000
0 184000000
0 214000000
0 244000000
0 274000000
0 304000000

after:

0 132000000
0 165000000
0 198000000
0 231000000
0 264000000
0 297000000

The above output shows that the update rates are still correct and the rest of the integration test ensures no images are published after the battery is drained.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

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.

Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033 iche033 requested a review from chapulina as a code owner June 8, 2022 22:33
@iche033 iche033 mentioned this pull request Jun 8, 2022
8 tasks
@chapulina chapulina added 🏯 fortress Ignition Fortress sensors Sensors and sensor data labels Jun 13, 2022
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Thanks!

@chapulina chapulina merged commit b8c09b6 into ign-gazebo6 Jun 13, 2022
@chapulina chapulina deleted the fix_sensor_battery_test branch June 13, 2022 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress sensors Sensors and sensor data
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants