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

[Backport] Align multiple DEMs with TF (#34) #36

Merged
merged 5 commits into from
Jan 18, 2024
Merged

Conversation

Jaeyoung-Lim
Copy link
Member

@Jaeyoung-Lim Jaeyoung-Lim commented Jan 11, 2024

Problem Description
This is a backport of #34 to ROS2

rviz_screenshot_2024_01_18-10_09_17

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Jan 15, 2024

Want help fixing merge conflicts?

@Jaeyoung-Lim
Copy link
Member Author

Jaeyoung-Lim commented Jan 15, 2024

@Ryanf55 I actually need to fix the part where tf seeped into the grid_map_geo library first.

I would like to keep ros dependencies outside grid_map_geo

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Jan 15, 2024

@Ryanf55 I actually need to fix the part where tf seeped into the grid_map_geo library first.

I would like to keep ros dependencies outside grid_map_geo

Sounds good. If you want to split the package into grid_map_geo_core and grid_map_geo_ros packages, that could work too. It's a common pattern (and used in grid_map).

* Remove terrain alignment

* Make frame_id a member of GridMapGeo

* Broadcast Tf

* Load multiple tif files
Backport


WIP transform stamped


Fix path to rviz config
F
@Jaeyoung-Lim Jaeyoung-Lim marked this pull request as ready for review January 18, 2024 02:59
@Jaeyoung-Lim
Copy link
Member Author

@Ryanf55 This should now allow you to download multiple DEMs and Rviz will show them already aligned.

include/grid_map_geo/grid_map_geo.hpp Outdated Show resolved Hide resolved
launch/load_multiple_tif_launch.xml Outdated Show resolved Hide resolved
launch/load_multiple_tif_launch.xml Show resolved Hide resolved
src/grid_map_geo.cpp Outdated Show resolved Hide resolved
src/grid_map_geo.cpp Outdated Show resolved Hide resolved
src/test_tif_loader.cpp Outdated Show resolved Hide resolved
src/test_tif_loader.cpp Outdated Show resolved Hide resolved
@Ryanf55
Copy link
Collaborator

Ryanf55 commented Jan 18, 2024

image
Nice work! Just had to zoom out a bit from the default location in the rviz config to see both. A few small things to touch up if you want, otherwise LGTM.

@Jaeyoung-Lim
Copy link
Member Author

Jaeyoung-Lim commented Jan 18, 2024

@Ryanf55 You can actually call davosdorf and dischma_valley that looks like the following

rviz_screenshot_2024_01_18-10_09_17

@Jaeyoung-Lim
Copy link
Member Author

@Ryanf55 I have tried to address the review comments! Thanks for having a look!

I would probably ignore the style checks and leave it to #45

@Jaeyoung-Lim
Copy link
Member Author

@Ryanf55 Good to get in?

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Jan 18, 2024

@Ryanf55 Good to get in?

Looks good!

@Jaeyoung-Lim Jaeyoung-Lim merged commit 58f6052 into ros2 Jan 18, 2024
3 of 4 checks passed
@Jaeyoung-Lim Jaeyoung-Lim deleted the pr-backport-tf branch January 18, 2024 15:46
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