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

Add sdf param to skip GetGeoRef call when loading DEMs #3215

Closed
wants to merge 2 commits into from

Conversation

adityapande-1995
Copy link
Contributor

@adityapande-1995 adityapande-1995 commented Apr 29, 2022

Partially resolves #2881

This adds a sdf tag <skipGeoRef> to skip GetGeoRef call and stops it from spamming the console with error msgs. Added extra functions to maintain ABI. Currently this works for gzserver, and gzclient will still throw the same errors.

Testing

This same test file (#2881 (comment)) should not throw any more errors, with a slight modification :

<heightmap>
    <uri>file://dem_neg.tif</uri>
    <size>80.078 80.078 5</size>
    <skipGeoRef>true</skipGeoRef>
</heightmap>

The problem

The chain of function calls is as follows :

Dem.cc::GetGeoRef() --> This function is spamming the error, need to skip it based on an sdf flag.
  |
Dem.cc::Load()
  |
HeightMapDataLoader::LoadDEMAsTerrain()
 |
HeightMapDataLoader::LoadTerrainFile()
 |
HeightMapShape::LoadTerrainFile() --> This has access to the sdf element

Solution 1

This means the sdf flag has to be trickled down without affecting ABI. I could create copies of these functions with different names and have them called by the lowest level LoadTerrainFile() based on the sdf flag.

Issues :

  • This makes it difficult to implement in gzclient though, as it does not have access to the sdf element.
  • The code just looks unnecessarily complicated with the extra functions.

Solution 2

We could have a service that is advertising a "global" flag that would speficy whether we should run the GetGeoRef function. This would mean the code would be cleaner and changes would be applied to gzclient as well, but it would apply to ALL heightmaps.

Signed-off-by: Aditya <aditya050995@gmail.com>
Signed-off-by: Aditya <aditya050995@gmail.com>
@adityapande-1995
Copy link
Contributor Author

The error seems to be gone in the latest Gazebo Garden, closing this PR for now.

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.

Dem size is not computed correctly for extraterrestrial celestial bodies
1 participant