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

Plane: correct reporting of relative altitude as a global altitude #19992

Merged
merged 1 commit into from
Feb 9, 2022

Conversation

peterbarker
Copy link
Contributor

There's an expectation that next_WP_loc is in the absolute frame at the moment.

LoiterAltQLand wasn't obeying that, setting it above-home or above-terrain; this has the mode set the altitude in absolute frame, setting the terrain flag if appropriate.

The test included here fails on master:

AT-0022.0: AP: Battery 1 is low 12.59V used 429 mAh
Mode(25)> Mode Mode(25)
AP: Battery 1 is low 12.59V used 429 mAh
AT-0022.0: 2022-02-04 18:25:27.03: POSITION_TARGET_GLOBAL_INT (id=87) (link=None) (signed=False) (seq=158) (src=1/1)
    time_boot_ms: 109636ms
    coordinate_frame: 0 (MAV_FRAME_GLOBAL)
    type_mask: 65016
      ! POSITION_TARGET_TYPEMASK_X_IGNORE
      ! POSITION_TARGET_TYPEMASK_Y_IGNORE
      ! POSITION_TARGET_TYPEMASK_Z_IGNORE
        POSITION_TARGET_TYPEMASK_VX_IGNORE
        POSITION_TARGET_TYPEMASK_VY_IGNORE
        POSITION_TARGET_TYPEMASK_VZ_IGNORE
        POSITION_TARGET_TYPEMASK_AX_IGNORE
        POSITION_TARGET_TYPEMASK_AY_IGNORE
        POSITION_TARGET_TYPEMASK_AZ_IGNORE
      ! POSITION_TARGET_TYPEMASK_FORCE_SET
        POSITION_TARGET_TYPEMASK_YAW_IGNORE
        POSITION_TARGET_TYPEMASK_YAW_RATE_IGNORE
        [UNKNOWN]
        [UNKNOWN]
        [UNKNOWN]
        [UNKNOWN]
    lat_int: -27.27057deg
    lon_int: 151.2948922deg
    alt: 15.0m
    vx: 0.0m/s
    vy: 0.0m/s
    vz: 0.0m/s
    afx: 0.0m/s/s
    afy: 0.0m/s/s
    afz: 0.0m/s/s
    yaw: 0.0rad (0.0deg)
    yaw_rate: 0.0rad/s (0.0deg/s)

AT-0022.0: Sending param_request_read for (Q_RTL_ALT)
AT-0022.0: get_parameter(Q_RTL_ALT): PARAM_VALUE {param_id : Q_RTL_ALT, param_value : 15.0, param_type : 4, param_count : 1511, param_index : 65535}
AT-0022.0: Exception caught: Unexpected altitude; expected=359.000000 got=15.000000
Traceback (most recent call last):
  File "/home/pbarker/rc/ardupilot/Tools/autotest/common.py", line 6522, in run_one_test_attempt
    test_function()
  File "/home/pbarker/rc/ardupilot/Tools/autotest/quadplane.py", line 814, in LoiterAltQLand
    (expected_alt, m.alt))
common.NotAchievedException: Unexpected altitude; expected=359.000000 got=15.000000

but passes on this PR.

I haven't spotted any code paths which would make this instantly fatal - but something updating plane.guided_WP_loc but not updating its frame would potentially cause the plane to navigate to (e.g.) 15m absolute rather than 15m relative, which might be bad.

@peterbarker
Copy link
Contributor Author

(it isn't fatal in LotierAltQLand because we're using methods on the location object to get a difference in height)

@IamPete1
Copy link
Member

IamPete1 commented Feb 4, 2022

Doesn't this break terrain following?

Planes altitude handling is such a mess...

@peterbarker
Copy link
Contributor Author

Doesn't this break terrain following?

I did try to preserve that behaviour my setting the terrain-follow bit in the location.

... I guess I'll have to add a test for that. It's a bit of a PITA to test, of course - I might just run the entire test at the grand canyon.

Planes altitude handling is such a mess...

Yes - I stumbled across this when trying to get rid of the persistent guided_WP which I think will be a good start on fixing things. Eliminating direct access to the alt fields in the location object is something I've had as a goal for a while....

@peterbarker
Copy link
Contributor Author

I'm really not sure about leaving the next_WP_loc as home-relative here, but the problem I specifically saw was definitely a bug (reporting relative altitude as a global altitude), so I've narrowed this PR to just fix that problem.

@peterbarker peterbarker changed the title Plane: Have LoiterAltQLand set next_wp height in absolute rather than relative frame GCS_MAVLink: correct reporting of relative altitude as a global altitude Feb 7, 2022
@peterbarker peterbarker changed the title GCS_MAVLink: correct reporting of relative altitude as a global altitude Plane: correct reporting of relative altitude as a global altitude Feb 8, 2022
@peterbarker peterbarker merged commit d914e4e into ArduPilot:master Feb 9, 2022
@peterbarker peterbarker deleted the pr/lotieraltqland-alt branch February 9, 2022 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants