Plane: added systemID as an aux function #29237

tridge · 2025-02-06T05:07:06Z

This adds VTOL systemID as an AUX function to quadplanes. It works in QSTABILIZE, QHOVER and QLOITER modes.
In QLOITER it multiplies the XY position and velocity gains by SID_XY_CTRL_MUL to reduce the impact of the XY controller on the systemID result
The AP_SystemID class is structured in a way to make it fairly easy to move this to a common library in the future if copter wants to adopt the same approach

timtuxworth · 2025-02-06T05:15:34Z

I'm struggling to understand why this is called "System ID". It doesn't seem to have anything to do with identifying the system - which is what we have SYSID_THISMAV for no?

tridge · 2025-02-06T06:43:19Z

I'm struggling to understand why this is called "System ID"

"system identification" is a control systems term that refers to characterising the dynamics of a system (a vehicle in this case). In copter it is a flight mode called SYSTEMID flight mode.
and yes, this means we have overlap in naming with the MAVLink SYSID. Welcome to the world of UAVs and re-used names.

IamPete1 · 2025-02-06T12:26:43Z

ArduPlane/systemid.cpp

+    // @DisplayName: System identification axis
+    // @Description: Controls which axis are being excited.  Set to non-zero to see more parameters
+    // @User: Standard
+    // @Values: 0:None, 1:Input Roll Angle, 2:Input Pitch Angle, 3:Input Yaw Angle, 4:Recovery Roll Angle, 5:Recovery Pitch Angle, 6:Recovery Yaw Angle, 7:Rate Roll, 8:Rate Pitch, 9:Rate Yaw, 10:Mixer Roll, 11:Mixer Pitch, 12:Mixer Yaw, 13:Mixer Thrust, 14:Measured Lateral Position, 15:Measured Longitudinal Position, 16:Measured Lateral Velocity, 17:Measured Longitudinal Velocity, 18:Input Lateral Velocity, 19:Input Longitudinal Velocity


Should add "VTOL" to these, would be good to support in FW flight in the future.

I was planning to re-use the same axes (where relevant, can't do lateral vel and pos).

remove values 14-19. I did not implement them in my version of sysid. it doesn't look like you did either. Lets keep it to the attitude controller. We can expand to the position controller later.

IamPete1 · 2025-02-06T12:28:00Z

ArduPlane/systemid.cpp

+        gcs().send_text(MAV_SEVERITY_WARNING, "SystemID: must be armed");
+        return;
+    }
+    if (!plane.quadplane.enabled()) {


This case should be handled by plane.control_mode->supports_systemid()

timtuxworth · 2025-02-06T14:52:50Z

I'm struggling to understand why this is called "System ID"

"system identification" is a control systems term that refers to characterising the dynamics of a system (a vehicle in this case). In copter it is a flight mode called SYSTEMID flight mode. and yes, this means we have overlap in naming with the MAVLink SYSID. Welcome to the world of UAVs and re-used names.

I'm aware of this problem hence bringing it up. I've found it helps to provide some way to differentiate overloaded terms.

Could the new (user facing) name be something like "Dynamic System Identification" or "System Model Identification" or DSMI (Dynamic System Model Identification)?

bnsgeyer · 2025-02-07T03:23:16Z

ArduPlane/systemid.cpp

+
+        auto *attitude_control = plane.quadplane.attitude_control;
+        attitude_control->bf_feedforward(restore.att_bf_feedforward);
+        attitude_control->rate_bf_roll_sysid(0);


Setting these to zero is not necessary. These are set to zero in the attitude controller at the end of every loop.
https://github.com/ArduPilot/ardupilot/blob/master/ArduPlane/quadplane.cpp#L1948

this was paranoia to ensure that even a single loop is not incorrect

bnsgeyer · 2025-02-07T03:41:12Z

ArduPlane/systemid.cpp

+// @Field: Az: Delta velocity, Z-Axis
+
+// log system id
+void AP_SystemID::log_data() const


I see you removed the logging of RATE and ANG from this function. It is imperative that these all be logged at the same rate. Why don't you use the same technique as copter to allow logging at multiples of the fast loop rate. this is important to allow users to choose an appropriate rate for their use case.

It is imperative that these all be logged at the same rate.

unlike copter, the default setup in plane is to log all these at the full loop rate, so they are all logged at the same rate. We could force the LOG_BITMASK in case a user has changed that, or we could just document that the defaults should be used.

Why don't you use the same technique as copter to allow logging at multiples of the fast loop rate. this is important to allow users to choose an appropriate rate for their use case.

it would be good to discuss why this is needed. Is it to make logs smaller? The sysid runs for a short time (usually) and the extra complexity to support down sampling didn't seem worthwhile, but let's discuss when you have a chance

The data logging for system ID is extreemly important if we intend to make this a reliable system. Leaving logging to other parts of the code invites crashed aircraft after small logging problems result in unstable tunes that look great in the too. This is why logging perfection is so important in system ID.

@tridge
I agree with @lthall that the logging is extremely important. Any time difference between signals can cause issues when either using it to develop a model or tune the aircraft. In most cases, I use the gyro rates from SIDD because they haven't been filtered by the INS filters and I use the motors inputs from RATE. Having these signals logged at the same time is very important. Keeping them all in the SystemID log function also provides higher confidence that future developers won't change how things are logged for System ID.

As for the various rates that copter allows, I am not as bothered by this. I think it can be an issue for users if their flight controllers have issues logging at the fast rate. I know my F7s were unable to keep up at 400hz logging rate. Thus it was very useful to be able to use 100hz so that I could allow the F7 to keep up with the logging.

allows for systemID to reduce XY gains in pos controlled modes

enable as an AUX switch

lthall · 2025-02-09T23:27:17Z

libraries/AC_AttitudeControl/AC_PosControl.cpp

@@ -679,6 +679,10 @@ void AC_PosControl::update_xy_controller()
    // add velocity feed-forward scaled to compensate for optical flow measurement induced EKF noise
    vel_target *= ahrsControlScaleXY;

+    if (_xy_control_scale_factor > 0) {


Why was this added?

@lthall
@tridge added this to be able to scale the velocity and position inputs of the position controller while using System ID feature in a position controlled mode. Currently he set the scaler to 0.1 to reduce the position and velocity feedback while system ID is done.

Cool.
We should change this to match the pattern in Attitude Control where we set it to 1.0 by default.

If this is a "Control" scale factor we should also update the accel_target like we do with the ahrsControlScaleXY.
accel_target *= ahrsControlScaleXY;

tridge added the VTOL-Plane label Feb 6, 2025

tridge requested a review from bnsgeyer February 6, 2025 05:07

tridge force-pushed the pr-sysid-quadplane branch from ac21f5a to 1a23f2d Compare February 6, 2025 05:08

tridge force-pushed the pr-sysid-quadplane branch from 1a23f2d to 322c3f1 Compare February 6, 2025 06:54

tridge mentioned this pull request Feb 6, 2025

Plane: system ID for quadplane - backport to 4.5 #29239

Open

IamPete1 reviewed Feb 6, 2025

View reviewed changes

Hwurzburg added the WikiNeeded needs wiki update label Feb 6, 2025

tridge force-pushed the pr-sysid-quadplane branch 2 times, most recently from a8fd6ec to d9853b6 Compare February 7, 2025 00:49

bnsgeyer requested changes Feb 7, 2025

View reviewed changes

tridge force-pushed the pr-sysid-quadplane branch from d9853b6 to 5699218 Compare February 7, 2025 06:06

tridge and others added 3 commits February 10, 2025 09:49

RC_Channel: added SYSTEMID AUX switch

eeb9df1

AC_AttitudeControl: added set_xy_control_scale_factor()

d1f22a5

allows for systemID to reduce XY gains in pos controlled modes

Plane: add system ID to quadplane

6c272da

enable as an AUX switch

tridge force-pushed the pr-sysid-quadplane branch from 5699218 to 6c272da Compare February 9, 2025 22:50

lthall reviewed Feb 9, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plane: added systemID as an aux function #29237

Plane: added systemID as an aux function #29237

tridge commented Feb 6, 2025

timtuxworth commented Feb 6, 2025

tridge commented Feb 6, 2025

IamPete1 Feb 6, 2025

tridge Feb 7, 2025

bnsgeyer Feb 7, 2025

IamPete1 Feb 6, 2025

timtuxworth commented Feb 6, 2025

bnsgeyer Feb 7, 2025

tridge Feb 7, 2025

bnsgeyer Feb 7, 2025

tridge Feb 7, 2025

lthall Feb 8, 2025

bnsgeyer Feb 8, 2025 •

edited

Loading

lthall Feb 9, 2025

bnsgeyer Feb 9, 2025

lthall Feb 10, 2025

Plane: added systemID as an aux function #29237

Are you sure you want to change the base?

Plane: added systemID as an aux function #29237

Conversation

tridge commented Feb 6, 2025

timtuxworth commented Feb 6, 2025

tridge commented Feb 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timtuxworth commented Feb 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnsgeyer Feb 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnsgeyer Feb 8, 2025 •

edited

Loading