Migrating default PID gains and filter settings to platform defaults and Kconfig

[matejkarasek]

Until now, PID values have been hardcoded, and could only be changed via permanent_parameters, or in a custom fork.
This PR enables using platform-specific PID gain values and filter settings.

• PID gains are now defined for each platform individually in the [specific_patform]_defaults.h
• defaults for low pass filter settings are in platform_defaults.h, and can be overridden inside [specific_patform]_defaults.h for each platform if needed
• defines PID_FILTER_ALL and IMPROVED_BARO_Z_HOLD moved to Kconfig

[matejkarasek]

Not sure what is going wrong with the build checks, all platforms build fine locally... Any ideas?

[krichardsson]

After a very quick look (that is it might be completely wrong) I think the problem is that the unit tests can not include the platform_defaults.h file. You probably have to add it to the unit test config

You can run the unit tests with tb test

I will have mote time to look at the PRs next week

[knmcguire]

Same here:) Please check what is causing the CI to fail

[matejkarasek]

After a very quick look (that is it might be completely wrong) I think the problem is that the unit tests can not include the platform_defaults.h file. You probably have to add it to the unit test config

You can run the unit tests with tb test

I will have mote time to look at the PRs next week

Thanks for the pointer @krichardsson, will check it out!

[matejkarasek]

So running tb test in docker passes all the tests.
But tb build fails at the same place as the CI.

The unit test config seems to already include the folder where platform_defaults.h is included, so it must be something else 🤔

crazyflie-firmware/tools/test/gcc.yml

Line 56 in b370e20

- 'src/platform/interface/'

[krichardsson]

The problem is not the unit tests, but building the python bindings. You can run them with tb test_python.

I think you need to add "src/platform/interface", to the include section of bindings/setup.py

[krichardsson]

The call to commanderGetSetpoint() in attitude_pid_controller.c is also causing problems, in general it feels a bit "backwards". Maybe you can pass in the value of setpoint.attitudeRate.yaw as a parameter to the function instead?

[matejkarasek]

The build errors are fixed, thanks for suggesting the correct solution :)

The call to commanderGetSetpoint() in attitude_pid_controller.c is also causing problems, in general it feels a bit "backwards". Maybe you can pass in the value of setpoint.attitudeRate.yaw as a parameter to the function instead?

I have removed the feedforward gain (where this is needed) from the current pull request as it can be treated separately.

How would you feel about updating the entire PID structure, such that it includes an extra feedforward gain, and everything is handled by the pidUpdate(...) function?
Next to yaw control, we use feed forward gains also in the velocity controller:

crazyflie-firmware/src/modules/src/position_controller_pid.c

Lines 242 to 243 in b370e20

	attitude->pitch = -runPid(state_body_vx, &this.pidVX, setpoint->velocity.x, DT) - kFFx*setpoint->velocity.x;
	attitude->roll = -runPid(state_body_vy, &this.pidVY, setpoint->velocity.y, DT) - kFFy*setpoint->velocity.y;

[matejkarasek]

The last 2 commits include the feedforward gain in the PID structure, how do you feel about this solution?

[krichardsson]

I think it looks good!
The feed forward addition to the the PIDs is a nice feature

[matejkarasek]

Ok, great to hear!

[krichardsson]

Nice!

[krichardsson]

Merged. Thanks!