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: ATT.DesPitch now reports actual target, not nav output (Float version) #29067

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Georacer
Copy link
Contributor

Alternative to #28991

It converts Plane::demanded_pitch_cd into a float, and can thus save a lot of headache: It is no longer necessary to have a second variable to track the value validity.

Still tested to work in SITL, by changing MANUAL->FBWA->MANUAL.

int32_t nav_pitch_cd;

// The instantaneous desired pitch angle. Hundredths of a degree.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// The instantaneous desired pitch angle. Hundredths of a degree.
// The instantaneous desired pitch angle in degrees

Also initialized to NaN which will not show up in the ATT.DesPitch
plots, unless a mode (usually) overwrites it.
@Georacer Georacer force-pushed the pr/log_des_pitch_float branch from c434b8a to 58218d6 Compare January 21, 2025 13:55
Copy link
Member

@IamPete1 IamPete1 left a comment

Choose a reason for hiding this comment

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

There are some modes where we swap between angle controller and not. It does look like we currently just leave the angle targets hanging in those cases. Training is one case:

if (plane.training_manual_pitch) {
SRV_Channels::set_output_scaled(SRV_Channel::k_elevator, pexpo);
} else {
plane.stabilize_pitch();

Nav scripting is the other:

} else if (nav_scripting_active()) {
// scripting is in control of roll and pitch rates and throttle
const float speed_scaler = get_speed_scaler();
const float aileron = rollController.get_rate_out(nav_scripting.roll_rate_dps, speed_scaler);
const float elevator = pitchController.get_rate_out(nav_scripting.pitch_rate_dps, speed_scaler);
SRV_Channels::set_output_scaled(SRV_Channel::k_aileron, aileron);
SRV_Channels::set_output_scaled(SRV_Channel::k_elevator, elevator);
float rudder = 0;
if (yawController.rate_control_enabled()) {
rudder = nav_scripting.rudder_offset_pct * 45;
if (nav_scripting.run_yaw_rate_controller) {
rudder += yawController.get_rate_out(nav_scripting.yaw_rate_dps, speed_scaler, false);
} else {
yawController.reset_I();
}
}
SRV_Channels::set_output_scaled(SRV_Channel::k_rudder, rudder);
SRV_Channels::set_output_scaled(SRV_Channel::k_steering, rudder);
SRV_Channels::set_output_scaled(SRV_Channel::k_throttle, plane.nav_scripting.throttle_pct);

It would be nice to reflect that in the log, but it is a existing issue, so it doesn't need to be resolved here.

@@ -511,6 +511,10 @@ void Plane::update_control_mode(void)
takeoff_state.throttle_lim_max = 100.0f;
takeoff_state.throttle_lim_min = -100.0f;

// Preemptively set demanded pitch to NaN, so that it properly doesn't
// get logged. Modes ovewriting it (most modes do) will change it.
demanded_pitch = NAN;
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be demanded_pitch_log, we need to make sure no one tries to use it for anything else. Should this be a logger.quiet_nanf()?

@@ -209,7 +209,7 @@ float Plane::stabilize_pitch_get_pitch_out()
const bool quadplane_in_frwd_transition = false;
#endif

int32_t demanded_pitch = nav_pitch_cd + int32_t(g.pitch_trim * 100.0) + SRV_Channels::get_output_scaled(SRV_Channel::k_throttle) * g.kff_throttle_to_pitch;
demanded_pitch = nav_pitch_cd*0.01f + g.pitch_trim + SRV_Channels::get_output_scaled(SRV_Channel::k_throttle)*0.01f*g.kff_throttle_to_pitch;
Copy link
Member

Choose a reason for hiding this comment

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

I think this also needs setting on 196 for the quadplane case. use_fw_attitude_controllers is not quite the same as show_vtol_view that we use for switching over logging, there maybe some overlap where this would be a change in behavior.

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