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 flickering visualization #4561

Conversation

VladyslavHrynchak200204
Copy link
Contributor

@VladyslavHrynchak200204 VladyslavHrynchak200204 commented Jul 25, 2024


Basic Info

Info Please fill out this column
Primary OS tested on (Ubuntu)
Robotic platform tested on gazebo simulation

Purpose:

The map_vis_z parameter has been introduced to help modify the map slightly below the default plane, aiming to eliminate flickering issues.

Default Value:

map_vis_z: 0.0

Minimum Value Without Flickering:

map_vis_z: -0.008

For Maintainers:

  • Check that any new parameters added are updated in docs.nav2.org
  • Check that any significant change is added to the migration guide
  • Check that any new features OR changes to existing behaviors are reflected in the tuning guide
  • Check that any new functions have Doxygen added
  • Check that any new features have test coverage
  • Check that any new plugins is added to the plugins page
  • If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

Copy link
Contributor

mergify bot commented Jul 25, 2024

This pull request is in conflict. Could you fix it @VladyslavHrynchak200204?

Copy link
Contributor

mergify bot commented Jul 25, 2024

@VladyslavHrynchak200204, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

@VladyslavHrynchak200204 VladyslavHrynchak200204 force-pushed the logivations/fix_flickering_visualization branch from 89f43fc to f1b1bfe Compare July 25, 2024 10:31
Copy link
Contributor

mergify bot commented Jul 25, 2024

@VladyslavHrynchak200204, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

Copy link
Contributor

mergify bot commented Jul 25, 2024

@VladyslavHrynchak200204, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

@VladyslavHrynchak200204 VladyslavHrynchak200204 changed the title Logivations/fix flickering visualization fix flickering visualization Jul 25, 2024
@VladyslavHrynchak200204
Copy link
Contributor Author

still in progress

@SteveMacenski
Copy link
Member

SteveMacenski commented Jul 25, 2024

OK, I'm going to refrain from a detailed review until you're ready! I am curious however what value you set this to

Signed-off-by: Vladyslav Hrynchak <[email protected]>
@VladyslavHrynchak200204 VladyslavHrynchak200204 force-pushed the logivations/fix_flickering_visualization branch from 6ace864 to 3bf69bf Compare July 29, 2024 07:57
Copy link
Contributor

mergify bot commented Jul 29, 2024

This pull request is in conflict. Could you fix it @VladyslavHrynchak200204?

@VladyslavHrynchak200204 VladyslavHrynchak200204 force-pushed the logivations/fix_flickering_visualization branch from 2058054 to fd0f756 Compare July 29, 2024 10:04
Copy link

codecov bot commented Jul 29, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Files Patch % Lines
nav2_costmap_2d/src/costmap_2d_publisher.cpp 75.00% 1 Missing ⚠️
Files Coverage Δ
...d/include/nav2_costmap_2d/costmap_2d_publisher.hpp 100.00% <ø> (ø)
...tmap_2d/include/nav2_costmap_2d/costmap_2d_ros.hpp 100.00% <ø> (ø)
nav2_costmap_2d/src/costmap_2d_ros.cpp 88.42% <100.00%> (-0.22%) ⬇️
nav2_costmap_2d/src/costmap_2d_publisher.cpp 62.06% <75.00%> (+0.26%) ⬆️

... and 1 file with indirect coverage changes

@VladyslavHrynchak200204 VladyslavHrynchak200204 force-pushed the logivations/fix_flickering_visualization branch 12 times, most recently from 3d137f9 to 8b2ba75 Compare July 30, 2024 07:54
@VladyslavHrynchak200204 VladyslavHrynchak200204 force-pushed the logivations/fix_flickering_visualization branch 4 times, most recently from 30dac64 to fd0f756 Compare July 30, 2024 10:16
Copy link
Contributor

mergify bot commented Jul 30, 2024

This pull request is in conflict. Could you fix it @VladyslavHrynchak200204?

Copy link
Contributor

mergify bot commented Jul 30, 2024

@VladyslavHrynchak200204, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

@VladyslavHrynchak200204 VladyslavHrynchak200204 force-pushed the logivations/fix_flickering_visualization branch 3 times, most recently from f601192 to 3bf69bf Compare July 30, 2024 11:04
@VladyslavHrynchak200204
Copy link
Contributor Author

OK, I'm going to refrain from a detailed review until you're ready! I am curious however what value you set this to

We tested different value which is as close to 0 as possible without flickering. From out testing map_vis_z: -0.008 is minimum value for this purpose.

@SteveMacenski Could you check these changes please?

Copy link
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.

The parameter / value in the occ grid are doubles, so keep them doubles please. Otherwise LGTM.

You do need to add to:

  • the parameter guide the new param
  • the migration guide the new param and reasons - an image would be nice

@@ -54,12 +54,14 @@ Costmap2DPublisher::Costmap2DPublisher(
Costmap2D * costmap,
std::string global_frame,
std::string topic_name,
bool always_send_full_costmap)
bool always_send_full_costmap,
float map_vis_z)
Copy link
Member

Choose a reason for hiding this comment

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

double

@@ -404,6 +404,7 @@ class Costmap2DROS : public nav2_util::LifecycleNode
bool track_unknown_space_{false};
double transform_tolerance_{0}; ///< The timeout before transform errors
double initial_transform_timeout_{0}; ///< The timeout before activation of the node errors
float map_vis_z_{0};
Copy link
Member

Choose a reason for hiding this comment

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

double

@@ -166,6 +167,7 @@ class Costmap2DPublisher
double saved_origin_y_;
bool active_;
bool always_send_full_costmap_;
float map_vis_z_;
Copy link
Member

Choose a reason for hiding this comment

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

double

@@ -72,7 +72,8 @@ class Costmap2DPublisher
Costmap2D * costmap,
std::string global_frame,
std::string topic_name,
bool always_send_full_costmap = false);
bool always_send_full_costmap = false,
float map_vis_z = 0.0);
Copy link
Member

Choose a reason for hiding this comment

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

double

Signed-off-by: Vladyslav Hrynchak <[email protected]>
@VladyslavHrynchak200204 VladyslavHrynchak200204 force-pushed the logivations/fix_flickering_visualization branch 2 times, most recently from 01620ce to bea10b3 Compare July 31, 2024 08:02
@VladyslavHrynchak200204
Copy link
Contributor Author

The parameter / value in the occ grid are doubles, so keep them doubles please. Otherwise LGTM.

You do need to add to:

  • the parameter guide the new param
  • the migration guide the new param and reasons - an image would be nice

Done. ros-navigation/docs.nav2.org#578

@SteveMacenski
Copy link
Member

CI failed in a weird way, so I retriggered that - so waiting to merge the software pending that. Also, your docs are missing the configuration guide update - please update that and I can merge the pair! Thanks!

@VladyslavHrynchak200204
Copy link
Contributor Author

CI failed in a weird way, so I retriggered that - so waiting to merge the software pending that. Also, your docs are missing the configuration guide update - please update that and I can merge the pair! Thanks!

Done

@SteveMacenski
Copy link
Member

Its still failing weirdly and no other pipeline is failing. Is this branch perhaps super old or something? Maybe rebasing or pulling in main could resolve.

@VladyslavHrynchak200204 VladyslavHrynchak200204 force-pushed the logivations/fix_flickering_visualization branch from bea10b3 to 9cd3c9c Compare August 2, 2024 06:46
Signed-off-by: Vladyslav Hrynchak <[email protected]>
@VladyslavHrynchak200204 VladyslavHrynchak200204 force-pushed the logivations/fix_flickering_visualization branch from 9cd3c9c to 29fafb5 Compare August 2, 2024 07:15
@VladyslavHrynchak200204
Copy link
Contributor Author

VladyslavHrynchak200204 commented Aug 2, 2024

I pulled the last changes and test have passed. Does codecov need to be fixed as well?

@SteveMacenski
Copy link
Member

No, that is fine, thanks for the follow up!

@SteveMacenski SteveMacenski merged commit f956626 into ros-navigation:main Aug 2, 2024
10 of 11 checks passed
SteveMacenski pushed a commit that referenced this pull request Aug 24, 2024
* Fix Flickering visualization

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Refactoring Costmap2DPublisher and Costmap2DROS

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Refactoring costmap_2d_ros.cpp

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Refactoring Costmap2DPublisher and Costmap2DROS

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Update costmap_2d_publisher.cpp

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Change map_vis_z from float to double

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Add comment to  map_vis_z_ parameter

Signed-off-by: Vladyslav Hrynchak <[email protected]>

---------

Signed-off-by: Vladyslav Hrynchak <[email protected]>
SteveMacenski added a commit that referenced this pull request Aug 24, 2024
* Updated README Table once Jazzy jobs turn over (#4482)

* add new Jazzy matrix

* missing header

* test toolg

* retry

* done!

* trim

* trim

* fix OS[0]

* shutdown services in destructor of `ClearCostmapService` (#4495)

Signed-off-by: GoesM_server <[email protected]>
Co-authored-by: GoesM_server <[email protected]>

* adjusting backup speed to be more reasonable (#4501)

* Adding Costmap filters to system tests and cleaning up old gazebo classic files (#4502)

* removing old files

* removing old refs to gazebo classic

* porting test body

* including in root

* Dock panel (#4458)

* Initial docking panel

Signed-off-by: Alberto Tudela <[email protected]>

* Only one goal status

Signed-off-by: Alberto Tudela <[email protected]>

* Added dock  pose

Signed-off-by: Alberto Tudela <[email protected]>

* Fix size of text

Signed-off-by: Alberto Tudela <[email protected]>

* Update rviz

Signed-off-by: Alberto Tudela <[email protected]>

* Update rviz config

Signed-off-by: Alberto Tudela <[email protected]>

---------

Signed-off-by: Alberto Tudela <[email protected]>

* Fix default view (#4504)

* Fix logo in nav2 panel (#4505)

* Fix logo in nav2 panel

Signed-off-by: Alberto Tudela <[email protected]>

* Move icon

Signed-off-by: Alberto Tudela <[email protected]>

---------

Signed-off-by: Alberto Tudela <[email protected]>

* Fix world to map coordinate conversion (#4506)

Signed-off-by: HovorunBh <[email protected]>

* Update README.md

Signed-off-by: Steve Macenski <[email protected]>

* Add dock id (#4511)

* Implement dock id

Signed-off-by: redvinaa <[email protected]>

* Update tests

Signed-off-by: redvinaa <[email protected]>

* Update docs

Signed-off-by: redvinaa <[email protected]>

* Fix virtual override error

Signed-off-by: redvinaa <[email protected]>

---------

Signed-off-by: redvinaa <[email protected]>

* fix(nav2_costmap_2d): make obstacle layer not current on enabled toggle (#4507)

Signed-off-by: Kemal Bektas <[email protected]>
Co-authored-by: Kemal Bektas <[email protected]>

* min_turning_r_ getting param fix (#4510)

* min_turning_r_ getting param fix

Signed-off-by: Ivan Radionov <[email protected]>

* Update nav2_mppi_controller/include/nav2_mppi_controller/motion_models.hpp

Signed-off-by: Steve Macenski <[email protected]>
Signed-off-by: Ivan Radionov <[email protected]>

---------

Signed-off-by: Ivan Radionov <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
Co-authored-by: Ivan Radionov <[email protected]>
Co-authored-by: Steve Macenski <[email protected]>

* fixing gz sim launch file by using gz directly (#4514)

* port wait behavior to new gazebo (#4471)

Signed-off-by: stevedan <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
Co-authored-by: Steve Macenski <[email protected]>

* Completely rewritten spin, backup, and drive on heading tests to remove flakiness (#4515)

* port backup behavior to new gazebo

Signed-off-by: stevedan <[email protected]>

* port drive on heading behavior to new gazebo

Signed-off-by: stevedan <[email protected]>

* completely rewritten spin test

* lint

* complete flaky test rewrite

---------

Signed-off-by: stevedan <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
Co-authored-by: stevedan <[email protected]>

* Return out of map update if frames mismatch. Signed-off-by Joey Yang (#4517)

Signed-off-by: Joey Yang <[email protected]>

* Fix error_code_id (#4522)

Signed-off-by: redvinaa <[email protected]>

* completely shutdown dyn_params_handler_ (s)  (#4521)

* completely shutdown dny_params_handler_ in nav2_amcl

Signed-off-by: GoesM_server <[email protected]>

* completely shutdown dyn_param_handler_ in controller_server

Signed-off-by: goes <[email protected]>

* compeletly shutdown dyn_params_handler in nav2_costmap_2d

Signed-off-by: goes <[email protected]>

* compeletly shutdown dyn_param_handler in nav2_docking

Signed-off-by: goes <[email protected]>

* completely shutdown dyn_params_handler in nav2_velocity_smoother

Signed-off-by: goes <[email protected]>

* compeletly shutdown dyn_param_handler in waypoint_follower

Signed-off-by: goes <[email protected]>

* typo fixed

Signed-off-by: goes <[email protected]>

* graceful-controller & dwb_controller

Signed-off-by: goes <[email protected]>

* mppi-controller

Signed-off-by: goes <[email protected]>

* navfn_planner & regulated_..controller

Signed-off-by: goes <[email protected]>

* rotation_..controller & samc_planners

Signed-off-by: goes <[email protected]>

* A*planner

Signed-off-by: goes <[email protected]>

* code style

Signed-off-by: goes <[email protected]>

* 1

Signed-off-by: goes <[email protected]>

* fixed

Signed-off-by: goes <[email protected]>

* fix the usage of weak_ptr

Signed-off-by: goes <[email protected]>

* code-style

Signed-off-by: goes <[email protected]>

* weak_ptr released

Signed-off-by: goes <[email protected]>

* code style

Signed-off-by: goes <[email protected]>

* code style

Signed-off-by: goes <[email protected]>

* code style

Signed-off-by: goes <[email protected]>

* code style update

Signed-off-by: goes <[email protected]>

* back

Signed-off-by: goes <[email protected]>

* rebase conflict resovled

Signed-off-by: goes <[email protected]>

* rebase error fixed

Signed-off-by: goes <[email protected]>

* fixed2

Signed-off-by: goes <[email protected]>

* rebase fixed 3

Signed-off-by: goes <[email protected]>

* 33

Signed-off-by: goes <[email protected]>

* shared_ptr into weak_ptr

Signed-off-by: GoesM <[email protected]>

* remove adundant node.resest()

Signed-off-by: GoesM <[email protected]>

---------

Signed-off-by: GoesM_server <[email protected]>
Signed-off-by: goes <[email protected]>
Signed-off-by: GoesM <[email protected]>
Co-authored-by: GoesM_server <[email protected]>

* check nullptr in smoothPlan() (#4544)

* check nullptr in smoothPlan()

Signed-off-by: GoesM <[email protected]>

* code-style

Signed-off-by: GoesM <[email protected]>

* code-style

Signed-off-by: GoesM <[email protected]>

* simple change

Signed-off-by: GoesM <[email protected]>

---------

Signed-off-by: GoesM <[email protected]>
Co-authored-by: GoesM <[email protected]>

* check nullPtr in `computeControl()` (#4548)

Signed-off-by: goes <[email protected]>
Co-authored-by: goes <[email protected]>

* Straight analytic expansions (#4549)

* Add test to verify analytic expansions are straight

Signed-off-by: James Ward <[email protected]>

* Use continuous scaling of angle when performing analytic expansion

Only applies to Hybrid A* - behaviour in lattice planner is unchanged

Signed-off-by: James Ward <[email protected]>

---------

Signed-off-by: James Ward <[email protected]>

* Rviz tool to get cost of costmap cell (#4546)

* Added GetCost srv

Signed-off-by: Jatin Patil <[email protected]>

* Added Service in costmap_2d

Signed-off-by: Jatin Patil <[email protected]>

* Added Rviz tool

Signed-off-by: Jatin Patil <[email protected]>

* Fixed Styling

Signed-off-by: Jatin Patil <[email protected]>

* Fixed Styles and Linting

Signed-off-by: Jatin Patil <[email protected]>

* Fixed Linting

Signed-off-by: Jatin Patil <[email protected]>

* Added Bool use_footprint to srv

Signed-off-by: Jatin Patil <[email protected]>

* Added unit test for costmap costcell cost service

Signed-off-by: Jatin Patil <[email protected]>

* Fixed unit test script

Signed-off-by: Jatin Patil <[email protected]>

* Added theta, Updated unit test, Updated rviz tool service call logic

Signed-off-by: Jatin Patil <[email protected]>

* Updated requested changes

Signed-off-by: Jatin Patil <[email protected]>

---------

Signed-off-by: Jatin Patil <[email protected]>

* Switch to new-style static_transform_publisher arguments. (#4563)

These arguments have been the preferred way to use things
since at least Humble.  This avoids warnings when running it for the tests.

Signed-off-by: Chris Lalancette <[email protected]>

* Updated slack link (#4565)

Signed-off-by: Steve Macenski <[email protected]>

* Update README.md (#4589)

Signed-off-by: Steve Macenski <[email protected]>

* fix flickering visualization (#4561)

* Fix Flickering visualization

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Refactoring Costmap2DPublisher and Costmap2DROS

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Refactoring costmap_2d_ros.cpp

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Refactoring Costmap2DPublisher and Costmap2DROS

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Update costmap_2d_publisher.cpp

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Change map_vis_z from float to double

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Add comment to  map_vis_z_ parameter

Signed-off-by: Vladyslav Hrynchak <[email protected]>

---------

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Copy fix-terminate diff from opennav_docking repo (#4598)

* Copy fix-terminate diff from opennav_docking repo

Signed-off-by: redvinaa <[email protected]>

* Lint

Signed-off-by: redvinaa <[email protected]>

---------

Signed-off-by: redvinaa <[email protected]>

* Fix race condition in AMCL for #4537 (#4605)

* Fixed timed_behavior.hpp (#4602)

Signed-off-by: Vladyslav Hrynchak <[email protected]>

* Adding new Nav2 loopback simulator (#4614)

* adding Nav2 loopback sim

Signed-off-by: Steve Macenski <[email protected]>

* drop performance by half

* lintin

* Add multirobot usecase comment

* fixing copy paste error

* fixing review comments

---------

Signed-off-by: Steve Macenski <[email protected]>

* Added laser data from map in nav2_loopback_sim (#4617)

* Added laser data from map

Signed-off-by: Jatin Patil <[email protected]>

* Fixed Linting

Signed-off-by: Jatin Patil <[email protected]>

* Fixed Linting

Signed-off-by: Jatin Patil <[email protected]>

* Fixed few requested changes

Signed-off-by: Jatin Patil <[email protected]>

* Fixed linting

Signed-off-by: Jatin Patil <[email protected]>

* Requested changes

Signed-off-by: Jatin Patil <[email protected]>

* Linting

Signed-off-by: Jatin Patil <[email protected]>

* Added parameters and fixed requested changes

Signed-off-by: Jatin Patil <[email protected]>

* linting

Signed-off-by: Jatin Patil <[email protected]>

* Added scan  using LineIterator

Signed-off-by: Jatin Patil <[email protected]>

* LineIterator max_distance or range_max

Signed-off-by: Jatin Patil <[email protected]>

* min of max_distance or range_max

Signed-off-by: Jatin Patil <[email protected]>

* final updates working correctly

Signed-off-by: Jatin Patil <[email protected]>

* Fix lint

Signed-off-by: Jatin Patil <[email protected]>

* requested changes

Signed-off-by: Jatin Patil <[email protected]>

* Update nav2_loopback_sim/nav2_loopback_sim/loopback_simulator.py

Signed-off-by: Steve Macenski <[email protected]>

* Update nav2_loopback_sim/nav2_loopback_sim/loopback_simulator.py

Signed-off-by: Steve Macenski <[email protected]>

---------

Signed-off-by: Jatin Patil <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
Co-authored-by: Steve Macenski <[email protected]>

* Making base frame ID for map to base link transform based on base frame ID parameter (#4632)

Signed-off-by: Steve Macenski <[email protected]>

* Update Smac Planner for rounding to closest bin rather than flooring (#4636)

* adding stamped option for loopbacks im (#4637)

Signed-off-by: Steve Macenski <[email protected]>

---------

Signed-off-by: GoesM_server <[email protected]>
Signed-off-by: Alberto Tudela <[email protected]>
Signed-off-by: HovorunBh <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
Signed-off-by: redvinaa <[email protected]>
Signed-off-by: Kemal Bektas <[email protected]>
Signed-off-by: Ivan Radionov <[email protected]>
Signed-off-by: stevedan <[email protected]>
Signed-off-by: Joey Yang <[email protected]>
Signed-off-by: goes <[email protected]>
Signed-off-by: GoesM <[email protected]>
Signed-off-by: James Ward <[email protected]>
Signed-off-by: Jatin Patil <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Vladyslav Hrynchak <[email protected]>
Co-authored-by: GoesM <[email protected]>
Co-authored-by: GoesM_server <[email protected]>
Co-authored-by: Alberto Tudela <[email protected]>
Co-authored-by: Bohdan <[email protected]>
Co-authored-by: Vince Reda <[email protected]>
Co-authored-by: Kemal Bektas <[email protected]>
Co-authored-by: Kemal Bektas <[email protected]>
Co-authored-by: Ivan Radionov <[email protected]>
Co-authored-by: Ivan Radionov <[email protected]>
Co-authored-by: Stevedan Ogochukwu Omodolor <[email protected]>
Co-authored-by: stevedan <[email protected]>
Co-authored-by: Joey Yang <[email protected]>
Co-authored-by: James Ward <[email protected]>
Co-authored-by: Jatin Patil <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
Co-authored-by: Vladyslav Hrynchak <[email protected]>
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.

2 participants