PID controllers cleanup (less pid controllers)

[borisbstyle]

Pid controller cleanup. New default PID controller is pidRewrite

1- Rewrite
2- Luxfloat

Let me know if anybody disagrees?

[ledvinap]

PID0 is default controller, it is used and tuned by lots of users. Removing it abruptly is very bad idea.

Maybe it is possible to change default to multiwii23 now and decide what to do after things settle ..
Providing equations to migrate coefficients may help too ...

[MJ666]

In my view PID0 and PID3 perform quite similar except for the yaw behavior. The tuning values are relative similar. Users which don't do much tuning will have likely an positive experience with the same settings (may need to reduce roll and pitch P abbot 20%). Users with highly tuned copters likely are not using PID0 anymore since a longer time and will not be affected by this change (except to set the proper number which is than changed for PID3 and PID5).

@borisbstyle we also need to adjust documentation accordingly. But this additional work could be done if we get agreement here.

In general in my view the will help to clean up and reduce complexity for the users without real negative effect. May be @hydra could share also his opinion here.

[borisbstyle]

Yes I agree with MJ666. There are really just a few pilots using PID0 and I tested some of my old tunes on PID0 with PID3 and it's pretty much the same except I get much faster yaw.
I think we should not keep the legacy for too long. This will make people choices easier and maintaining will become less difficult.
This will also free up some space to be able to implement new things and especially with the CMJCU size issues this would be helpfull.

[dkisselev]

I'm not so sure about "few". Remember that the people that you see here and on other forums are probably not representative of the population.

I've happily used PID0 for the past 6 months because it worked perfectly without any tuning for me, and when I tried any other PID, I found that it flew horribly. So as a noob, I (knowingly) stuck with it because it let me focus on actually learning to fly. I would absolutely hate it if I updated to 1.10 and found that my perfectly good PID was removed with no warning, because my day of fun flying suddenly turned into a day of frustrating tuning because I didn't get a chance to read up on it.

I switched to luxfloat and got it all tuned up now (literally today actually, because I did a rebuild that didn't fly as well on PID0), so this wouldn't affect me anymore, but I imagine it will still affect newcomers.

The other issue is that we and everyone else have referred to the PID's by number, so changing the numbers can be pretty confusing with all the existing posts and documentation out there.

My vote would be to go through a deprecation phase. Set the default PID for new setups to 3 as suggested above, and re-work the configurator to make the soon-to-be-removed options look like bad options. Leave it that way for a release or two, then actually remove those bad controllers with a future release after users have a chance to actually learn how to tune PID's and switch over.

[SteveAmor]

As @MJ666 has said, if this does happen, please be sure to update documentation - eg. docs/PID tuning.md

[borisbstyle]

@dkisselev
Did you ever try PID3 on yur current PID0(default) tune?
Only number for PID5 will be changed, which will move to PID3.

@SteveAmor
Of course this will all be documented once it will go into master. I usually always add things to documentation after some collaboration.

[MJ666]

@dkisselev I think if you use PID0 without any tuning (what I also did at the beginning) you will find PID3 working only better and you will have an positive experience. Did you ever tried this? May be its a bit unusual at the beginning the copter will then really respond on yaw what he ins not rely doing on PID0 and you need to get used of this. Everything else should work as well as in PID0.

[Zero9r]

With my experience in other areas I have come to the conclusion that with any major change you just need to rip the bandage off as quickly and fast as possible. There will always be an argument to keep pid0 and what not. But for the sake of space and moving the builds forward I say it just needs to be done. It may effect a very small group, but I don't see it being to big a deal. If you moved to CF then you know you are going to have to make some changes and tuning. If they are not up to it then they should of just stuck with what ever GUI there board originally uses.

[hydra]

I think when removing old pid controllers that keeping the pid controller numbers as-is is fine. Renumbering will just cause confusion.

Disabling old ones in next gui release sounds sensible and leaving them available for exisiting users until 1.11.0. And making one of the others the default in 1.11.0 too

[MJ666]

This sounds like a plan. We even can map than the nonexistent PID controllers automatically to the new default one in the newer firmware so if someone configure PID3 or PID4 in the GUI or via the CLI it will be changed to PID0 (actual PID3) during saving. This will allow to keep the GUI backward compatible for a while and user of the new firmware will have the proper configuration for the future also.

[borisbstyle]

@hydra That makes sense. So basicallly some pid controllers will be removed, but still selectable in cli etc? cli allows only ranges. That means that pid controller 4 for example would still be selectable. So ramapping would be needed?

@MJ666
I think remapping during save can also cause confusion?

[MJ666]

@borisbstyle I think somewhere we need to make the cut. If the PID controller is removed you need to be sure it can not be chosen in the configuration anymore.

There are a few options I can see.

• We can build intelligence into the CLI and GUI to avoid this configuration depending on the firmware version.
• We still can allow to configure this and map this to the default PID internally but keep the value in the configuration.
• Or we can for the proposed solution and change configuration to use the default PID controller if an remove one would be chosen.

I my view the last one is less impacting and the simples way to do. It will also best for going forward to new versions and leave legacy behind. Finally documentation and release notes should reflect this changes.

[borisbstyle]

@MJ666 In the configurator this is all very easy. You can do API check fucntion and depending on that select and show available pid controllers and name mappings.
But in the cli there is no warning or visual representation why your chosen PID controllers setting is constantly automatically being changed to seomething else.
This is also a cli implementation limitation, where you only can select single range of configurable parameters.

[MJ666]

@borisbstyle In the CLI we have many cases where values will not get saved and get altered if you make choices witch will not fit together (i.e. if you configure an serial Rx without have an serial port assigned first or if you are using two different receiver types like RX_PARALLEL_PWM and RX_SERIAL).

[ProDrone]

I think this is a modification that should be done in steps, over multiple releases.

What if, as a first step, we configure the PID controllers by name as Multiwii2.3, Rewrite, Luxfloat, Harakiri in CLI and Configurator? Just to make it impossible to choose a non-existing PID controller.

Then all we need is a fixed part in the documentation that states what the name is for PID controller nn in Cleanflight v1.9.0. People can use this to map info about PID settings from older user-posts that still refer to PID controller numbers (to prevent loss of knowledge).

[hydra]

Not a bad idea but would prefer NOT to have strings in the FC firmware that are only used for one purpose. If the upcoming configuration menus can use thr same strings then that would be acceptable. Would still prefer less FC code though.

[hydra]

MSP will still need code to set pid controller via number too.

[ProDrone]

Yes, MSP needs an additional messages to get the name, number mapping. Also required to stay in sync when internal mapping changes (should not require a client update). I agree that adding strings and extending MSP and CLI code is something we don't want, but this may be the price to pay to keep PID users happy (and when CLI is removed, this code will also be gone).

[hydra]

I'll dwell on the feedback some and make a descision soon.

[digitalentity]

I personally use Harakiri PID right from the start. It's stable, flyable on the defaults and already uses D-term LPF (aka PT1). I tried other controllers with my 450-sized quad, but Harakiri was the best one, although on bigger or smaller quads things might be different.

[MJ666]

I fly Harakiri (PID5) on my small multicopters and PID Rewrite (PID1) on the larger ones (160mm and bigger).

[orefalo]

I am so confused I though Luxfloat was ruling them all.

[dkisselev]

@orefalo The guys that like Luxfloat are just so happy with it that they love to scream it to the world (with good reason). Harakiri has also been very very good, so those that have good results on it have no reason to switch. (the controllers being removed are all significantly worse than both of those)

[orefalo]

oh! got it, the description is confusing - it sounds like if PID0-4 will be removed.

Agreed! should be merged

[Redshifft]

Leave them all just sort out those dodgy defaults in 1 & 2

[wanderzell]

I agree the firmware and documentation needs an update to clearly show which PID controllers are developed further and thus being recommended. This involves also to set new default.

But just removing the old ones introducing naming conflict is certainly not a good idea. There are lots of PID specs out in the wild that refer the controllers only by number. This is because how they are shown in UI and in the Docs - people have got impression that names change but numbers stay. And this is why we must not mix the numbers, if new controllers are added and old ones removed. If we decide to retire controller 0 and 4 then just retire them. And keep using 1, 2, 3 and 5.

I see the main problem in this transition is that there are numbers used to identify the controller and not the name.

Here's what I suggest.

Next major release:

• Introduce sticky name for each controller, use this in UI, CLI and docs. so literally - either pid_controller=2 and pid_controller=luxfloat would work
• Retire controllers 0 and 4 by documenting them as "deprecated", provide easy ways for people to switch (eg. some simple math how to convert PID0 values to PID1)

Major release after next:

• remove deprecated controllers
• start using only names. Keep numbers for API compatibility, if needed, but the main idea would be to make people using names when referring to controllers, therefore making sure that there will be no confusion about different controllers coming and going ever again.

[borisbstyle]

@hydra
Can you tell me what you think of this approach as intermediate situation? Its all rebased.

cli is now working with both pid controller number and name. It is always reporting the pid controller name. Configurator part should also be adjusted.

[digitalentity]

+1 to @borisbstyle latest commit. We still have the most used controllers. Also I see more and more people migrating over to lux after it was improved.

[borisbstyle]

@digitalentity On this way we can easy remove, add or shift pid controllers without affecting the enduser much.

I need to improve the set option to not be cap sensitive and it should be good. I guess we still can use integer in the MSP API on this way and just modify it in the configurator based on API version

[borisbstyle]

@hydra
What do you think of this cleanup?

[hydra]

No matter what we do. Some people will be unhappy. Unavoidable.

My preference, keep 1,2,3 delete the others.

We can always add other ones back later if we need them.

As for a default.. undecided.

[hydra]

rename to pidControllerNames please

[hydra]

new default: rewrite.
why?: easier to tune. stock settings work for more users.

[hydra]

ok, going to merge this and the cli naming PR now. see #1488

[hydra]

also did this:

cleanflight/cleanflight-configurator@ac78dbc

[hydra]

so, as I mentioned in IRC:

the INDEX of pid controllers is GONE from a users perspective
so no more 'PID3', etc
GUI doesn't have it, CLI doesn't have it

MW23, MWREWRITE and LUX are the OFFICIAL names