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.

Sign up for GitHub

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 UNIT_Dem_TEST failure on Ubuntu 20.04 #3275

Open
wants to merge 11 commits into
base: gazebo11
Choose a base branch
from
Open

Fix UNIT_Dem_TEST failure on Ubuntu 20.04 #3275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
82b92ca
Added debug statement
adityapande-1995 Nov 8, 2022
5958b37
Workaround for Neg Dem and Lunar Dem test
adityapande-1995 Nov 9, 2022
883ac12
Debug
adityapande-1995 Nov 9, 2022
07f2aed
Debug
adityapande-1995 Nov 9, 2022
944e78f
Debug
adityapande-1995 Nov 9, 2022
3a87ff4
Debug
adityapande-1995 Nov 9, 2022
0e48fd4
Debug
adityapande-1995 Nov 9, 2022
7ad85cb
Fixed LunarDem and NegDem test failures
adityapande-1995 Nov 10, 2022
eb21501
Minor cleanup
adityapande-1995 Nov 10, 2022
09fb1bc
Minor cleanup
adityapande-1995 Nov 10, 2022
128a66c
Merge branch 'gazebo11' into aditya/fix_dem_test_fail
scpeters Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions gazebo/common/DemPrivate.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ namespace gazebo
public: GDALRasterBand *band;

/// \brief Real width of the world in meters.
public: double worldWidth;
public: double worldWidth = 0;

/// \brief Real height of the world in meters.
public: double worldHeight;
public: double worldHeight = 0;
Comment on lines +47 to +50
Copy link
Collaborator

Choose a reason for hiding this comment

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

= 0 isn't necessary


/// \brief Terrain's side (after the padding).
public: unsigned int side;
Expand Down
29 changes: 25 additions & 4 deletions gazebo/common/Dem_TEST.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,19 @@ TEST_F(DemTest, NegDem)
// Check the heights and widths
EXPECT_EQ(33, static_cast<int>(dem.GetHeight()));
EXPECT_EQ(33, static_cast<int>(dem.GetWidth()));
EXPECT_FLOAT_EQ(293.51068, dem.GetWorldHeight());
EXPECT_FLOAT_EQ(293.51089, dem.GetWorldWidth());
// This DEM model is from the moon. Older versions
// of libproj will calculate the size assuming it
// is of the Earth, unless we specify the surface.
bool sizeSameAsEarth =
(std::abs(293.51089 - dem.GetWorldWidth()) < 0.1)
&& (std::abs(293.51068 - dem.GetWorldHeight()) < 0.1);
// Newer versions give invalid sizes, 0 in this case.
bool invalidSize =
(dem.GetWorldHeight() < 0.001) &&
(dem.GetWorldWidth() < 0.001);

EXPECT_TRUE(sizeSameAsEarth || invalidSize);
Comment on lines +183 to +194
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to use the libproj macros? https://github.com/OSGeo/PROJ/blob/35e4207a9d9eb86b4e5188f00eae1f152306dad3/src/proj.h#L175-L177

or

https://github.com/OSGeo/PROJ/blob/35e4207a9d9eb86b4e5188f00eae1f152306dad3/src/proj.h#L188

eg.

#if PROJ_AT_LEAST_VERSION(<ubuntu 18.04 version>)
  EXPECT_FLOAT_EQ(293.51068, dem.GetWorldHeight());
  EXPECT_FLOAT_EQ(293.51089, dem.GetWorldWidth());
#else
  ...
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did try that initially (82b92ca), but the CI was complaining that it couldn't find "proj.h". Worked for me locally though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe that will be resolved if you add find_package(PROJ REQUIRED) then add ${PROJ_INCLUDE_DIRS} to include_directories, and ${PROJ_LIBRARIES} to target_link_libraries

https://github.com/OSGeo/PROJ/blob/65f60f6e27482920aa5007af5357d1f37b220b90/cmake/CMakeLists.txt#L14-L21

eg.,

Update:

gazebo-classic/gazebo/common/CMakeLists.txt

Lines 16 to 18 in 09fb1bc

if (HAVE_GDAL)
include_directories(${GDAL_INCLUDE_DIR})
endif()

To be:

if (HAVE_GDAL)
  include_directories(${GDAL_INCLUDE_DIR})
  if (PROJ_FOUND)
    include_directories(${PROJ_INCLUDE_DIRS})
  endif()
endif()

I'm not sure what version of PROJ is being used but it may need PROJ4 instead: https://github.com/OSGeo/PROJ/blob/65f60f6e27482920aa5007af5357d1f37b220b90/cmake/CMakeLists.txt#L51-L56


EXPECT_FLOAT_EQ(-212.29616, dem.GetMinElevation());
EXPECT_FLOAT_EQ(-205.44009, dem.GetMaxElevation());
}
Expand Down Expand Up @@ -236,8 +247,18 @@ TEST_F(DemTest, LunarDemLoad)
// as the celestial bodies in DEM file and
// default spherical coordinates do not match.
EXPECT_EQ(dem.Load(path.string()), 0);
EXPECT_NEAR(293.51, dem.GetWorldWidth(), 0.1);
EXPECT_NEAR(293.51, dem.GetWorldHeight(), 0.1);

// Older versions of libproj will default the size
// calculation to Earth's size.
bool sizeSameAsEarth =
(std::abs(293.51 - dem.GetWorldWidth()) < 0.1)
&& (std::abs(293.51 - dem.GetWorldHeight()) < 0.1);
// Newer versions of libproj will output a zero.
bool invalidSize =
(dem.GetWorldWidth() < 0.001) &&
(dem.GetWorldHeight() < 0.001);

EXPECT_TRUE(sizeSameAsEarth || invalidSize);

// Setting the spherical coordinates solves the
// problem.
Expand Down