search

gazebosim / gazebo-classic

Gazebo classic. For the latest version, see https://github.com/gazebosim/gz-sim
http://classic.gazebosim.org/
Other
1.18k stars 479 forks source link

Default SphericalCoordinates frame should be East-North-Up (ENU) #2022

Open osrf-migration opened 8 years ago

osrf-migration commented 8 years ago

Original report (archived issue) by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).

The <spherical_coordinates> tag in sdformat declares itself to be using an East-North-Up (ENU) coordinate frame in spherical_coordinates.sdf (though rather indirectly, the language there should be clarified).

However, the gazebo SphericalCoordinates class appears to be using a West-South-Up (WSU) coordinate frame. I've committed a test in 67dde6716abb495224422f5e6287f13ce33aeb04 (branch spherical_coordinates_enu) showing the failure.

osrf-migration commented 8 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).

chapulina commented 3 years ago

Fixing the current behaviour in a minor / patch release would break existing code that is compensating for it, which we shouldn't do.

But luckily, the compensation just involves a 180 degree heading rotation like this:

    <spherical_coordinates>
        <!-- currently gazebo has a bug: instead of outputing lat, long, altitude in ENU
        (x = East, y = North and z = Up) as the default configurations, it's outputting (-E)(-N)U,
        therefore we rotate the default frame 180 so that it would go back to ENU -->
        <heading_deg>180</heading_deg>
    </spherical_coordinates>
chapulina commented 3 years ago

I'm addressing this issue for ignition::math::SphericalCoordinates in https://github.com/ignitionrobotics/ign-math/pull/235. If that goes in, it could potentially be ported to gazebo::common::SphericalCoordinates.

beaucolley commented 2 years ago

I find this hard to believe but this still seems to be an issue in both ignition::math::SphericalCoordinates and gazebo::common::SphericalCoordinates can anyone confirm?

peci1 commented 9 months ago

OMG lost a few hours debugging this on G11!!! Couldn't there be at least some warning in G11 when user code calls SphericalFromLocal etc.? And also when there is a subscriber to the default GPS sensor's output?

traversaro commented 9 months ago

Couldn't there be at least some warning in G11 when user code calls SphericalFromLocal etc.?

This is a bit trick, but doable. Gazebo Classic does not depend on any C++14 feature, but it compiles using the default version of C++ provided by the compiler, that is C++14 at least on GCC since GCC 6 (see https://gcc.gnu.org/gcc-6/changes.html). So we could just use some preprocessor logic to define a macro (for example in https://github.com/gazebosim/gazebo-classic/blob/7ccef40e24831eb5cd974b489ee279fc064eacc4/gazebo/util/system.hh#L317, or even locally to the affected file) that is defined as [[deprecated(message)]] if the code is compiled with C++14, that will end to be defined on any reasonable modern system.

And also when there is a subscriber to the default GPS sensor's output?

That may be more tricky, I am not sure how we can achieve this.

traversaro commented 9 months ago

I find this hard to believe but this still seems to be an issue in both ignition::math::SphericalCoordinates and gazebo::common::SphericalCoordinates can anyone confirm?

Indeed the issue is still there in gazebo::common::SphericalCoordinates, and it is still there as default behaviour in ignition::math::SphericalCoordinates, even if in https://github.com/gazebosim/gz-math/pull/235 a way to fix it by slightly changing the calling code was implemented.

peci1 commented 4 months ago

I find this hard to believe but this still seems to be an issue in both ignition::math::SphericalCoordinates and gazebo::common::SphericalCoordinates can anyone confirm?

Indeed the issue is still there in gazebo::common::SphericalCoordinates, and it is still there as default behaviour in ignition::math::SphericalCoordinates, even if in gazebosim/gz-math#235 a way to fix it by slightly changing the calling code was implemented.

I've just sent a fix for this to gz-math main branch: https://github.com/gazebosim/gz-math/pull/596.

peci1 commented 1 month ago

A proper fix for this issue has landed into Gazebo Ionic: https://github.com/gazebosim/gz-math/pull/616, https://github.com/gazebosim/gz-msgs/pull/450 and https://github.com/gazebosim/gz-sim/pull/2535 . I'll have a look at ways how to backport it into Classic.