Skip to content

Add test for geopose heading#28091

Merged
tridge merged 1 commit intoArduPilot:masterfrom
Ryanf55:test-ros-geopose-heading
Sep 17, 2024
Merged

Add test for geopose heading#28091
tridge merged 1 commit intoArduPilot:masterfrom
Ryanf55:test-ros-geopose-heading

Conversation

@Ryanf55
Copy link
Contributor

@Ryanf55 Ryanf55 commented Sep 13, 2024

Purpose

Add a test to prove the reported heading on the geopose interface is REP-103 compliant.

This matches the proposed clarifications here, and can be used as an example in the REP.
ros-infrastructure/rep#407

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
@Ryanf55 Ryanf55 force-pushed the test-ros-geopose-heading branch from b07a3c4 to a24440b Compare September 13, 2024 01:08
@Ryanf55 Ryanf55 added the ROS label Sep 13, 2024

def ros_quat_to_heading(quat):
def ros_quat_to_heading_deg(quat):
# By default, scipy follows scalar-last order – (x, y, z, w)
Copy link
Contributor

Choose a reason for hiding this comment

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

scipy is a large library to pull in just for quaternions and rotations. Have you thought about transforms3d (https://matthew-brett.github.io/transforms3d/)?

Copy link
Contributor Author

@Ryanf55 Ryanf55 Sep 13, 2024

Choose a reason for hiding this comment

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

Yea, I just saw scipy was already used a bunch of places, including in the setup script, and thought the team would prefer a library they are familiar with.

PYTHON_PKGS+=" matplotlib scipy opencv-python pyyaml"

Your link looks like a much more focused library, but the apt version is out of date:
matthew-brett/transforms3d#65

Copy link
Contributor

@srmainwaring srmainwaring left a comment

Choose a reason for hiding this comment

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

LGTM. Added notes with suggestions, but no blockers.

and math.isclose(position.altitude, CMAC_ABS_ALT, abs_tol=1.0)
)

def wrap_360(angle):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use the same calc as AP_Math here?

float wrap_360(const float angle)
{
    float res = fmodf(angle, 360.0f);
    if (res < 0) {
        res += 360.0f;
    }
    return res;
}

The calc does not wrap 360 deg to 0, not sure if that's an issue (means the function is not invertible).

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 pulled this from pymavlink but added recursion so you can handle multi-wrap. If the initial heading was directly north, the comparison would get tricky, but this util is only used in this hard-coded cmac test.


Per ROS REP-103, the quaternion should report 0 when the vehicle faces east,
and 90 degrees when facing north.
Because CMAC is NNW, we expect ~97 degees.
Copy link
Contributor

Choose a reason for hiding this comment

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

./Tools/autotest/locations.txt has this for CMAC. Heading is 353 => rotation = 97 as described.

CMAC=-35.363261,149.165230,584,353

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I just had to add a tolerance because the data is through the EAHRS with noise added. Peter mentioned to set the EKF type to match the SIM so it's exact, but we don't depend on mavproxy or support params yet, so I left that out of scope and used a tolerance instead.

@tridge tridge merged commit 0cc07ac into ArduPilot:master Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants