Skip to content

[WIP] Implement INDI control law with supporting infrastructure#25216

Open
rohan908 wants to merge 34 commits intoPX4:mainfrom
acp-lab:pr-indi
Open

[WIP] Implement INDI control law with supporting infrastructure#25216
rohan908 wants to merge 34 commits intoPX4:mainfrom
acp-lab:pr-indi

Conversation

@rohan908
Copy link
Copy Markdown

@rohan908 rohan908 commented Jul 14, 2025

Solved Problem

INDI Rate Control Implementation for Multicopters

Solution

New INDI Rate Control Module

  • Added: src/modules/mc_indi_rate_control/ - Complete INDI rate control implementation
    • MulticopterINDIRateControl.cpp/hpp - Main INDI controller with adaptive effectiveness
    • ActuatorEffectivenessRotorsINDI.cpp/hpp - Rotor-specific actuator effectiveness matrix
    • mc_indi_rate_control_params.c - Parameter definitions for INDI tuning
    • module.yaml - Module configuration and parameter metadata

Core INDI Features

  • Adaptive Effectiveness Matrix: Dynamic calculation of G1 and G2 matrices based on current rotor speeds
  • Inertia Compensation: Incorporates vehicle inertia matrix for accurate torque calculations
  • Low-Pass Filtering: ESC status filtering for smooth control signals
  • Parameter-Based Configuration: Configurable rotor positions, thrust coefficients, and moment coefficients

New Parameters

  • Metric Geometry Group:
    • MET_ROTOR${i}_CT - Thrust coefficients (N/(rad/s)²)
    • MET_ROTOR${i}_KM - Moment coefficients (m)
    • MET_ROTOR${i}_IZZ - Rotor moments of inertia (kg·m²)

Integration Changes

  • Modified: ROMFS/px4fmu_common/init.d/rc.mc_apps - Added conditional INDI module startup
  • Updated: Airframe configurations to support INDI rate control

Testing & Validation

  • Publish outputs for debugging to UROB topics!
  • Extra variables published in vehicle_thrust_setpoint for debugging as well as the adaptive G matrices published to indi_adaptation_status
  • To know if it is working the indi_torque_setpoint should follow the measured_torque closely

Context

Implemented from this paper: https://sihaosun.github.io/publication/comparativestudy/
And this paper: https://pure.tudelft.nl/ws/files/9188985/Adaptive_Incremental_Nonlinear_Dynamic_Inversion_or_Attitude_Control_of_Micro_Aerial_Vehiclesa.pdf
Equations:
image

@rohan908 rohan908 marked this pull request as draft July 14, 2025 17:38
@rohan908 rohan908 changed the title Pr indi [WIP] Pr indi Jul 14, 2025
@dagar
Copy link
Copy Markdown
Member

dagar commented Jul 14, 2025

I've played with implementing this a few times, so I'd be very happy if we can bring it in experimentally. What would really help make that possible is if it can be carried as an optional alternate rate controller (eg mc_indi_rate_control) so that there's no potential cost or downside to regular users.

@rohan908
Copy link
Copy Markdown
Author

I've played with implementing this a few times, so I'd be very happy if we can bring it in experimentally. What would really help make that possible is if it can be carried as an optional alternate rate controller (eg mc_indi_rate_control) so that there's no potential cost or downside to regular users.

I've found some crucial errors in my current implementation that kind of forces this to be a seperate module (specifically the non-metric affectiveness matrix in the control allocator). This will be fixed!

Copy link
Copy Markdown
Member

@Jaeyoung-Lim Jaeyoung-Lim left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

High level, you are modifying commander and the existing mc_rate_control module to implement indi control.

I would rather separate this out as a separate module (e.g. mc_indi_rate_control) so that you are less tangled with the rest of the system (such as commander) and it is less safety critical by modifying the existing rate controller.

On the allocation side, we can now use the control allocator as a library. My suggestion would be to do the allocation inside the new module, without touching the control allocator.

@julianoes julianoes changed the title [WIP] Pr indi [WIP] Implement INDI control law with supporting infrastructure Jul 15, 2025
@rohan908 rohan908 requested a review from Jaeyoung-Lim July 17, 2025 22:00
Copy link
Copy Markdown
Member

@Jaeyoung-Lim Jaeyoung-Lim left a comment

Choose a reason for hiding this comment

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

Thank you for the adjustments.

I couldn't go through the code thoroughly yet, but here are some of my initial comments.

param set-default MET_ROTOR3_PY -0.174
param set-default MET_ROTOR3_CT 0.0001047
param set-default MET_ROTOR3_KM -0.0005
param set-default MET_ROTOR3_IZZ 2.649858234714004e-05
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need to define this in addition to the CA_ parameters?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

the CT, KM for CA_ are not in metric. The IZZ is not defined in CA_. And with regards to the positioning, I can get rid of those, the CA_ params seem to give accurate enough results and are in metric.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the downside of just using Ct and Km in CA_ and use them as metric parameters? Defining additonal parameters will use a lot of flash space that is not necesssary.

float32[48] g1_matrix # G1 matrix elements (3 rows * 16 columns)

# G2 matrix (gyroscopic part) - 3x16 matrix flattened
float32[48] g2_matrix # G2 matrix elements (3 rows * 16 columns)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it necessary to log these huge matrices to figure out what is going on?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

hm... I would suggest leaving it in for debugging the adaptive effectiveness matrix (bc its essentially only possible through manual tuning and I assume this feature would only be used experimentally). And it would only be published to when INDI_ADAPT_EN is enabled (==1).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These would be quite expensive to log at a high rate. I would at least remove them by default.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ok! What is the best practice to do that? Just comment them out?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes

vehicle_thrust_setpoint_s vehicle_thrust_setpoint{};
//vehicle_torque_setpoint_s vehicle_torque_setpoint{};

_thrust_setpoint.copyTo(vehicle_thrust_setpoint.xyz);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does the indi not track linear acceleration?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No, in the implementations I've seen so far for multicopters, the INDI has been only for the body torque.

Comment on lines +298 to +300
_vehicle_torque_setpoint.xyz[0] = PX4_ISFINITE(torque_setpoint(0)) ? torque_setpoint(0) : 0.f;
_vehicle_torque_setpoint.xyz[1] = PX4_ISFINITE(torque_setpoint(1)) ? torque_setpoint(1) : 0.f;
_vehicle_torque_setpoint.xyz[2] = PX4_ISFINITE(torque_setpoint(2)) ? torque_setpoint(2) : 0.f;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems a bit strange. So you are incrementing on top of the rate controller output, rather than incrementing against the previous torque setpoint?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It does seem strange at first but I'm basing this off of Professor Sihao Sun's work. From equation 28 we see that the desired angular acceleartion is essentially the same implementation as the current attitude controller -> attitude rate controler but without the I term. Then, in equation 33, that desired angular rate is subtracted by the filtered angular acceleration from the gyro (which is exactly what the output of the unmodified rate controller is!). The main assumption I'm using here is that the PD controller constants act as a pseudo-replacement for the moment of inertia matrix (Iv), but after talking with my colleagues they think that is a fine assumption (though I would love to hear your thoughts). Then, I add the filtered body torque from the indi_torque_setpoint to that output as done in equation 33.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm also going to push a new commit that cleans up the prints and all the residual debugging mess I've left behind in the code, including this awful code block that you've pointed out

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then, in equation 33, that desired angular rate is subtracted by the filtered angular acceleration from the gyro (which is exactly what the output of the unmodified rate controller is!).

I don't think this is correct. The desired angular acceleration is substracted by the filtered angular acceleration measured from the gyro. The filtered angular acceleration is NOT the same as the output of the unmodified rate controller, unless the torque is perfectly being tracked. The reason iNDI controller is robust is because it uses measurements to correct it's commands. I think this part of the implementation should be fixed.

@rohan908 rohan908 requested a review from Jaeyoung-Lim July 21, 2025 16:48
@rohan908 rohan908 marked this pull request as ready for review July 22, 2025 20:33
@DronecodeBot
Copy link
Copy Markdown

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-dev-call-july-23-2025-team-sync-and-community-q-a/46617/2

@Pedro-Roque Pedro-Roque self-requested a review July 23, 2025 15:09
@mrpollo mrpollo requested a review from MaEtUgR July 23, 2025 15:13
@mrpollo
Copy link
Copy Markdown
Contributor

mrpollo commented Jul 23, 2025

@MaEtUgR would you mind giving this a quick review? thanks!

Copy link
Copy Markdown
Member

@Pedro-Roque Pedro-Roque left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @rohan908 ! Here go a few comments on my side.

Comment on lines +6 to +10
float32[3] g1_term # G1 term of torque setpoint about X, Y, Z body axis (normalized)
float32[3] g2_term # G2 term of torque setpoint about X, Y, Z body axis (normalized)
float32[3] pid_xyz # PID torque setpoint about X, Y, Z body axis (normalized)
float32[3] indi_torque_xyz # INDI torque setpoint about X, Y, Z body axis (normalized)
float32[3] measured_body_torque_xyz # measured body torque about X, Y, Z body axis (normalized)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this should go here. If you need all these terms, then you should have a different message definition specific for INDI. Furthermore, measured_torque shouldn't belong in a torque setpoint message.

_param_handles[i].axis_z = param_find(buffer);
}

snprintf(buffer, sizeof(buffer), "MET_ROTOR%u_CT", i);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will be confusing for the user I believe, as they are the same as CA_..._CT/KM. I'd use those parameters and specify to the user that these should be metric.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If I replace the parameters of CA_... to be metric then the Control Allocator would not work, which is why I made them seperate. I could hypothetically have a parameter for maxRotSpeed and then use that to divide the CA_...CT/KM params? What do you suggest?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think a maximum rotor speed, if you need that value, should be fine to have. I think that's less ambiguous/duplicating than the current implementation.

I still would like to check what exactly fails in ControlAllocator if you use those variables as metric, but don't have time for it now. You could only set metric values in case your flag is enabled, for instance.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rohan908 Why do you care about the control_allocator module? IMO, you should not run it when doing iNDI

Comment on lines +1 to +2
# INDI Adaptation Status Message
# Tracks changes in G matrices during INDI adaptation
Copy link
Copy Markdown
Contributor

@hamishwillee hamishwillee Jul 24, 2025

Choose a reason for hiding this comment

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

Suggested change
# INDI Adaptation Status Message
# Tracks changes in G matrices during INDI adaptation
# INDI Adaptation Status message
#
# Tracks changes in G matrices during INDI adaptation.

Comment on lines +4 to +5
uint64 timestamp # time since system start (microseconds)
uint64 timestamp_sample # timestamp of the data used for adaptation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doc standard now single space before after comment, metadata prefix [] for units, [@range low,high], [@enum constant_prefix], [@frame frame_name]. No full stop unless there is multiple sentences.

Suggested change
uint64 timestamp # time since system start (microseconds)
uint64 timestamp_sample # timestamp of the data used for adaptation
uint64 timestamp # [us] Time since system start
uint64 timestamp_sample # [us] Timestamp of the data used for adaptation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI the docs standard is now documented in https://docs.px4.io/main/en/uorb/uorb_documentation

Comment on lines +14 to +16
float32[16] rotor_speeds # Current rotor speeds (rad/s)
float32[16] rotor_speed_dots # Rotor speed derivatives (rad/s^2)
float32[3] angular_accel_delta # Angular acceleration delta (rad/s^2)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
float32[16] rotor_speeds # Current rotor speeds (rad/s)
float32[16] rotor_speed_dots # Rotor speed derivatives (rad/s^2)
float32[3] angular_accel_delta # Angular acceleration delta (rad/s^2)
float32[16] rotor_speeds # [rad/s] Current rotor speeds
float32[16] rotor_speed_dots # [rad/s^2] Rotor speed derivatives
float32[3] angular_accel_delta # [rad/s^2] Angular acceleration delta

Comment on lines +23 to +24
bool adaptation_enabled # Whether adaptation is currently enabled
uint8 num_actuators # Number of actuators being adapted
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
bool adaptation_enabled # Whether adaptation is currently enabled
uint8 num_actuators # Number of actuators being adapted
bool adaptation_enabled # True if adaptation is currently enabled
uint8 num_actuators # Number of actuators being adapted

@github-actions github-actions bot added the stale label Aug 24, 2025
@mrpollo mrpollo removed the stale label Sep 12, 2025
@github-actions
Copy link
Copy Markdown

This PR was identified as stale and it will be closed in 30 days unless any activity is detected.

@github-actions
Copy link
Copy Markdown

This pull request has been automatically closed due to 120 days of inactivity. If you would like to continue, please feel free to reopen it or submit a new PR.

@github-actions github-actions bot closed this Jan 13, 2026
@Jaeyoung-Lim Jaeyoung-Lim reopened this Jan 13, 2026
@github-actions github-actions bot removed the stale label Jan 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants