Skip to content

Use current time to check if observation is out of date#1872

Merged
SteveMacenski merged 1 commit intoros-navigation:masterfrom
marting87:purge_fix
Jul 16, 2020
Merged

Use current time to check if observation is out of date#1872
SteveMacenski merged 1 commit intoros-navigation:masterfrom
marting87:purge_fix

Conversation

@marting87
Copy link
Copy Markdown
Contributor


Basic Info

Info Please fill out this column
Ticket(s) this addresses #545
Primary OS tested on Ubuntu
Robotic platform tested on prototype

Issue:

Points remain in obstacle layer after we stopped publishing a point cloud on regular basis. Settings to reproduce:

      obstacle_layer:
        enabled: true
        observation_sources: pointcloud
        pointcloud:
          topic: "blabla"
          max_obstacle_height: 2.0
          data_type: "PointCloud2"
          expected_update_rate: 0.0
          observation_persistence: 1.0
          clearing: true
          marking: true

According to observation_persistence: 1.0 the point clouds used by the obstacle layers should time-out after 1 second.

Analysis:

  • last_updated_ is updated on every new cloud that needs to be added to the buffer.
  • purgeStaleObservations checks the cloud's time stamp with last_updated_.
  • When we stop publishing a point cloud last_updated_ will never be updated.
  • Hence the observations will not time-out and remain in the observation_list_.
  • This may result in the following warning, when the robot drives away from the clouds in observation_list_:

Sensor origin at (x, y) is out of map bounds. The costmap cannot raytrace for it.


Description of contribution in a few bullet points

  • Fixed it by replacing last_updated_ with nh_->now() in purgeStaleObservations

Description of documentation updates required from your changes

  • None

Future work that may be required in bullet points

  • None

@marting87
Copy link
Copy Markdown
Contributor Author

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 14, 2020

Codecov Report

Merging #1872 into master will decrease coverage by 0.85%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1872      +/-   ##
==========================================
- Coverage   70.14%   69.29%   -0.86%     
==========================================
  Files         218      218              
  Lines       10586    10586              
==========================================
- Hits         7426     7336      -90     
- Misses       3160     3250      +90     
Impacted Files Coverage Δ
nav2_costmap_2d/src/observation_buffer.cpp 63.10% <0.00%> (ø)
...re/include/dwb_core/illegal_trajectory_tracker.hpp 40.00% <0.00%> (-60.00%) ⬇️
...roller/dwb_core/src/illegal_trajectory_tracker.cpp 25.00% <0.00%> (-55.00%) ⬇️
nav2_map_server/src/map_server/main.cpp 63.63% <0.00%> (-36.37%) ⬇️
...map_2d/include/nav2_costmap_2d/inflation_layer.hpp 91.66% <0.00%> (-8.34%) ⬇️
nav2_navfn_planner/src/navfn_planner.cpp 82.09% <0.00%> (-6.80%) ⬇️
nav2_costmap_2d/src/clear_costmap_service.cpp 17.89% <0.00%> (-6.32%) ⬇️
..._dwb_controller/dwb_core/src/dwb_local_planner.cpp 77.32% <0.00%> (-4.97%) ⬇️
...tree/include/nav2_behavior_tree/bt_action_node.hpp 79.26% <0.00%> (-4.88%) ⬇️
nav2_controller/src/nav2_controller.cpp 79.38% <0.00%> (-3.10%) ⬇️
... and 7 more

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 69977cd...9819cf3. Read the comment docs.

@SteveMacenski
Copy link
Copy Markdown
Member

@DLu can you take a look at this? I could go either way on it.

Copy link
Copy Markdown
Contributor

@DLu DLu left a comment

Choose a reason for hiding this comment

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

That makes sense to me.

Copy link
Copy Markdown
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

OK, I wasn't sure.

@SteveMacenski SteveMacenski merged commit 4e1c18d into ros-navigation:master Jul 16, 2020
@SteveMacenski
Copy link
Copy Markdown
Member

Thanks for the contribution @marting87

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants