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: Remove takeoff recalculation dead code #29079

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rubenp02
Copy link
Contributor

The takeoff calculation changes introduced in c15fa7b and 6ce6ef8 mean the heading no longer gets recalculated at TKOFF_LVL_ALT. Removed the related code as it never runs.

The takeoff calculation changes introduced in c15fa7b and 6ce6ef8 mean
the heading no longer gets recalculated at TKOFF_LVL_ALT. Removed the
related code as it never runs.
@rubenp02 rubenp02 force-pushed the cleanup/remove-takeoff-recalc-dead-code branch from b39795c to 8d64acc Compare January 15, 2025 10:05
@Georacer
Copy link
Contributor

Hello @rubenp02 !

Thanks for this code inspection.

So basically after 6ce6ef8, the segment

    if (plane.flight_stage == AP_FixedWing::FlightStage::TAKEOFF
        && plane.steer_state.hold_course_cd == -1 // This is the enter-once flag.
        && (altitude_cm >= (level_alt * 100.0f) || start_loc.get_distance(plane.current_loc) >= dist)
    ) {
        const float direction = start_loc.get_bearing_to(plane.current_loc) * 0.01;
        plane.next_WP_loc = start_loc;
        plane.next_WP_loc.offset_bearing(direction, dist);
        plane.next_WP_loc.alt += alt*100.0;
        plane.steer_state.hold_course_cd = wrap_360_cd(direction*100); // Necessary to allow Plane::takeoff_calc_roll() to function.
    }

never gets executed.

This means that the loiter target direction for the TAKEOFF mode is set once during the first motion of the plane on the track and is never re-set once the plane reaches TKOFF_LVL_ALT.
@Hwurzburg does it sound like this is a behaviour we want?

@rubenp02
Copy link
Contributor Author

I think it's desirable because the main point of the heading recalc was to correct yaw errors which isn't needed with the new changes. Also, silently recalculating the heading like this caused issues. For example, planes with no rudder and some propeller induced yaw veer slightly before TKOFF_LVL_ALT as they can't roll to compensate, which meant that in this case the actual takeoff heading never matched the one reported to the GCS.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Jan 15, 2025

Have you tried adding an AP_InternalError in the code block and run the entire plane autotest suite to better ensure it's never called currently?

@Hwurzburg
Copy link
Collaborator

@Hwurzburg does it sound like this is a behaviour we want?

sounds like it

@rubenp02
Copy link
Contributor Author

Have you tried adding an AP_InternalError in the code block and run the entire plane autotest suite to better ensure it's never called currently?

Will do

@rubenp02
Copy link
Contributor Author

Can confirm that it's never called in the tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants