PID-Profiles

[Zeanon]

It took me quite some time but I had to refine some stuff, smooth out switching between profiles and so on.
Code is working perfectly fine in my fork, though I might have missed something while transfering it so it should be tested (unfortunately I can't really test it since I would have to redo a whole printer as all my printers are running my fork)
Luckily @dans98 offered to test it so there's that

[bwnance]

thanks for the PR! this looks awesome. once dan and @rogerlz test i think it should be good - could you black format it? thanks :)

[Zeanon]

Of course, once I find out what that is XD
I still might have to look at a few things since some parameters in the PID algorithms differ compared to mainline Klipper so they might require some change in handling

[dans98]

I'm out of town till mid week, but after that I should be able to take a look at this.

[dans98]

I did a first pass code review tonight, i'll try and take a deeper look tomorrow.

[Zeanon]

Just a general info:
I have been running this code for about 6 months on my machines now and it worked really well
I did some minor modifications to smooth out overshoot when switching profiles but the code I pushed now has been tested on my fork for about 3 months
There is some over/undershoot when switching (which is to be expected) but I tried to reduce it to a minimum
If anyone has some ideas what could be changed, feel free to hit me up
The testing is also the main reason why I delayed making the PR for so long(I told Dan I would do it when he did the velocity PID pr on Klipper) but since it is quite a significant change I wanted to make sure it is stable and does not cause any unforseen problems since I don't want anyone to burn down their printer

[rogerlz]

thanks @Zeanon! It is a really nice feature, and I am personally interested in using it.
If you don't mind, I'd like to commit some additional stuff to this PR, like automated testing, adding the feature to the README, etc.

[Zeanon]

thanks @Zeanon! It is a really nice feature, and I am personally interested in using it.
If you don't mind, I'd like to commit some additional stuff to this PR, like automated testing, adding the feature to the README, etc.

Go for it
If you make some functional changes, let me know since I like to understand them :)
I'm always happy to learn or stand corrected if I forgot something or made a mistake

[YanceyA]

@Zeanon Do you mind providing a basic description of what PID profiles are, what they do and why a user would want to do this? I haven't quite caught onto why I need to have this feature in my life.

[Zeanon]

@Zeanon Do you mind providing a basic description of what PID profiles are, what they do and why a user would want to do this? I haven't quite caught onto why I need to have this feature in my life.

Yeah maybe I should improve the doc a bit
It allows you to create different PID values for different temperatures to improve accuracy if you tend to print different materials like abs and Pla which require vastly different temps

[dans98]

@Zeanon Do you mind providing a basic description of what PID profiles are, what they do and why a user would want to do this? I haven't quite caught onto why I need to have this feature in my life.

If you print different materials at different temps, you want different pid parameters.

For example on my printer If I calibrate for at 260c, but try and print at 220c. The plot of the hotend temp will look like a sin wave, because the parameters are to aggressive for the lower temp.

The opposite happens if I calibrate low and try and print high.

[YanceyA]

Thanks for the detail. This is a good feature for me as I have seen a small issue for my 255C PID tune when I need to run 305C for a ASA-CF material. As you feature allows, It would be great to have a few PID profiles to cover the range of temperatures.

At the most intensive case a user could create profiles at ~5 degree increments to have the best resolution for a PID model tuned to the specific print temperature?

As a side note, PID_v was an improvement for a 255C tune and print at 305C but still not perfect.

[Zeanon]

The 5° increments is exactly what I did
I just wrote a look up table for it

[rogerlz]

I pushed automated testing. It will fail due to the issue with PID_PROFILE REMOVE

[Zeanon]

@rogerlz I have fixed the stuff you pointed out and noticed another missing info in the docs (I added the load_clean and reset_target options later to smooth out the temps when switching profiles and apparently forgot to add them to the other commands as well, documentation is not my strong suit if I'm being honest)

Another thing I noticed yesterday: I also implemented a PID_PROFILES GET_VALUES HEATER=<heater_name> which was mainly for debugging but it is fleshed out.
Should we keep it or yeet it?

[Zeanon]

Another thing to think about imo:
Should I add all pid values to get_status or only the name as I am doing it rn?

Also is there a danger version of mainsail, as I also implemented support for the profiles into mainsail:

[rogerlz]

@rogerlz I have fixed the stuff you pointed out and noticed another missing info in the docs (I added the load_clean and reset_target options later to smooth out the temps when switching profiles and apparently forgot to add them to the other commands as well, documentation is not my strong suit if I'm being honest)

Another thing I noticed yesterday: I also implemented a PID_PROFILES GET_VALUES HEATER=<heater_name> which was mainly for debugging but it is fleshed out. Should we keep it or yeet it?

@Zeanon I would keep it. It might be useful to others too. Do you want to add it to the docs or happy as it is?
Would you please add a line to the README with this PR so users can see what features are in.

- [heater: PID-Profiles](https://github.com/DangerKlippers/danger-klipper/pull/145)

Should I add all pid values to get_status or only the name as I am doing it rn?

I'd say only the name to not be too verbose.

Also is there a danger version of mainsail,

I don't think so, but your change is great!! I loved it.
I don't have frontend dev skills to maintain a danger-mainsail, but I will talk to @bwnance

[Zeanon]

@rogerlz I have fixed the stuff you pointed out and noticed another missing info in the docs (I added the load_clean and reset_target options later to smooth out the temps when switching profiles and apparently forgot to add them to the other commands as well, documentation is not my strong suit if I'm being honest)
Another thing I noticed yesterday: I also implemented a PID_PROFILES GET_VALUES HEATER=<heater_name> which was mainly for debugging but it is fleshed out. Should we keep it or yeet it?

@Zeanon I would keep it. It might be useful to others too. Do you want to add it to the docs or happy as it is? Would you please add a line to the README with this PR so users can see what features are in.

- [heater: PID-Profiles](https://github.com/DangerKlippers/danger-klipper/pull/145)

Should I add all pid values to get_status or only the name as I am doing it rn?

I'd say only the name to not be too verbose.

Also is there a danger version of mainsail,

I don't think so, but your change is great!! I loved it. I don't have frontend dev skills to maintain a danger-mainsail, but I will talk to @bwnance

I actually already added the get_values to the doc but forgot a part of the command XD
If there are plans for a danger mainsail, I have a bunch of stuff I can do a PR for XD
My frontend dev skills are also quite limited, I just learned by modding mainsail to implement stuff for all the features I implemented into klipper

[Zeanon]

I forgot the readme
sorry XD

[rogerlz]

I forgot the readme sorry XD

no worries! I took the opportunity to shuffle it a bit

[YanceyA]

@Zeanon I'd really appreciate some more details on how to implement this. In no particular order here are a few questions:

• Is there a method or macro to create various PID tune profiles? Is the minimum viable method to run a number of PID tunes and save each manually?
• Do you have to SAVE_CONFIG to make the profiles available for loading? That is, if I run PID tunes, save the profiles, are they available for loading without save_config?
• Will save profile overwrite a profile with the same name? Or should the old profile be deleted first before saving with the same name?
  -There is no functionality to switch PID profiles automatically when temp changes? Say I have a pid profile for 200C, hotend is at 200C, and I want to increase the temperature to 300. I would need to load the 300C profile and then command the heater to go to 300C?
  -You mentioned a lookup table method... can you share your configs/macros for this?
  -Is the most practical method to use this a macro that will look at the extruder temp requested and automatically load the appropriate profile? Such as a macro in print start that looks at the extruder temp and loads the correct profile.

If you want I can help support writing some basic documentation to help users like me implement the feature.

[Zeanon]

@Zeanon I'd really appreciate some more details on how to implement this. In no particular order here are a few questions:

• Is there a method or macro to create various PID tune profiles? Is the minimum viable method to run a number of PID tunes and save each manually?
• Do you have to SAVE_CONFIG to make the profiles available for loading? That is, if I run PID tunes, save the profiles, are they available for loading without save_config?
• Will save profile overwrite a profile with the same name? Or should the old profile be deleted first before saving with the same name?
  -There is no functionality to switch PID profiles automatically when temp changes? Say I have a pid profile for 200C, hotend is at 200C, and I want to increase the temperature to 300. I would need to load the 300C profile and then command the heater to go to 300C?
  -You mentioned a lookup table method... can you share your configs/macros for this?
  -Is the most practical method to use this a macro that will look at the extruder temp requested and automatically load the appropriate profile? Such as a macro in print start that looks at the extruder temp and loads the correct profile.

If you want I can help support writing some basic documentation to help users like me implement the feature.

Basically pid_profiles behave the same as bed_mesh profiles
after you do a calibrate the profile will be stored and is the current loaded profile until you load another
if will be available to be loaded until a firmware restart occurs
if you do SAVE_CONFIG it will be written to the config and a profile with the name will be overwritten

you have to do a calibration for each profile (I started working on a way to calculate pid values for every temperature without having to a calibrate each time but it is extremely complex)

Concerning the LUT, take a look in here: https://github.com/LastZeanon/Klipper_LynxBot_Config/blob/master/LUT/pid.cfg
Loading a new profile every time you switch temp is not advised as switching profiles creates an over or undershoot which is not preventable
What I do is I have a table which stores which filament profile needs which PID profiles and if that information is not available I load the profile depending on the first_layer temp
Writing some info on how to use it best might be a good idea, I'll see what I can come up with

[YanceyA]

Awesome, thanks for that info and config reference. Exactly what i needed to know.

Curiously I wanted to see how linear the PID parameters were for your extruder. For your setup you could calculate PID parameters from a linear fit. That is, when an arbitrary extruder temp is set, you could calculate and load a suitable set of PID parameters. These linear fits could be calculated from only a few PID calibrations.

I'll try to do a similar analysis on my printer to see how the PID parameters are over a range of temps.