Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pid-profiles v2 #162

Merged
merged 2 commits into from
Feb 22, 2024
Merged

pid-profiles v2 #162

merged 2 commits into from
Feb 22, 2024

Conversation

rogerlz
Copy link
Contributor

@rogerlz rogerlz commented Feb 21, 2024

  • PID-Profiles

  • added documentation

  • I forgot to add

  • Black formatting

  • adding pid_profile tests

  • Fixed Remove_Profile

  • Updated doc

  • Small fix to doc

  • readme changes


@rogerlz
Copy link
Contributor Author

rogerlz commented Feb 21, 2024

@Zeanon fyi! we had to revert the pid-profile commits due to the error below
I got a bit too excited and forgot to test PID_CALIBRATE.

Internal error on command:"PID_CALIBRATE"
Traceback (most recent call last):
  File "/home/pi/klipper/klippy/gcode.py", line 290, in _process_commands
    handler(gcmd)
  File "/home/pi/klipper/klippy/gcode.py", line 199, in func
    return origfunc(self._get_extended_params(params))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pi/klipper/klippy/extras/pid_calibrate.py", line 47, in cmd_PID_CALIBRATE
    Kp, Ki, Kd = calibrate.calc_pid()
                 ^^^^^^^^^^^^^^^^^^^^
  File "/home/pi/klipper/klippy/extras/pid_calibrate.py", line 293, in calc_pid
    temp_diff = temp_diff + self.peaks[-i][1] - self.peaks[-i - 1][1]
                            ~~~~~~~~~~^^^^
IndexError: list index out of range
Internal error on command:"PID_CALIBRATE"
Internal Error on WebRequest: gcode/script
Traceback (most recent call last):
  File "/home/pi/klipper/klippy/webhooks.py", line 281, in _process_request
    func(web_request)
  File "/home/pi/klipper/klippy/webhooks.py", line 478, in _handle_script
    self.gcode.run_script(web_request.get_str("script"))
  File "/home/pi/klipper/klippy/gcode.py", line 310, in run_script
    self._process_commands(script.split("\n"), need_ack=False)
  File "/home/pi/klipper/klippy/gcode.py", line 290, in _process_commands
    handler(gcmd)
  File "/home/pi/klipper/klippy/gcode.py", line 199, in func
    return origfunc(self._get_extended_params(params))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/pi/klipper/klippy/extras/pid_calibrate.py", line 47, in cmd_PID_CALIBRATE
    Kp, Ki, Kd = calibrate.calc_pid()
                 ^^^^^^^^^^^^^^^^^^^^
  File "/home/pi/klipper/klippy/extras/pid_calibrate.py", line 293, in calc_pid
    temp_diff = temp_diff + self.peaks[-i][1] - self.peaks[-i - 1][1]
                            ~~~~~~~~~~^^^^
IndexError: list index out of range

@Zeanon
Copy link
Contributor

Zeanon commented Feb 21, 2024

Oh
As said, I can't really test with danger klippers
I'll take a look at it later and try to find out what's causing the issue
I'm sorry :(

@rogerlz
Copy link
Contributor Author

rogerlz commented Feb 21, 2024

Oh As said, I can't really test with danger klippers I'll take a look at it later and try to find out what's causing the issue I'm sorry :(

Nothing to be sorry for! It happens. I just wanted to let you know.
I will test it too later today.

@Zeanon
Copy link
Contributor

Zeanon commented Feb 21, 2024

Oh As said, I can't really test with danger klippers I'll take a look at it later and try to find out what's causing the issue I'm sorry :(

Nothing to be sorry for! It happens. I just wanted to let you know. I will test it too later today.

the interesting thing is: I didnt change that code, which confuses me a bit
did you check whether it happens without the pid profiles as well?
From what I can tell, didnt change anything that should affect cald_pid, so I'm a bit at a loss here

@Zeanon
Copy link
Contributor

Zeanon commented Feb 21, 2024

I realized one thing I actually forgot

    def update_smooth_time(self, write_to_profile):
        return

    def get_profile(self):
        return {'name': 'autotune'}

    def get_type(self):
        return 'autotune'

Those three methods should be at the end of ControlAutoTune ro make the methods consistend across all Controls
Actually this might be the cause of the error:
(or at least I ahve a theory what is causing the error:
When setting the temp for autotune, we get an error, thus the except of the try catch block catches in, sets back the old control and peaks now has no values thus causing the index out of range

@Zeanon
Copy link
Contributor

Zeanon commented Feb 21, 2024

I managed to debug and fix it.
While doing that I also fixed a typo and readded the message that save_config will update the config.

https://github.com/Zeanon/danger-klipper/blob/master/klippy/extras/pid_calibrate.py

* PID-Profiles

* added documentation

* I forgot to add

* Black formatting

* adding pid_profile tests

* Fixed Remove_Profile

* Updated doc

* Small fix to doc

* readme changes

---------

Co-authored-by: Rogerio Goncalves <rogerlz@gmail.com>
@rogerlz
Copy link
Contributor Author

rogerlz commented Feb 21, 2024

nice! It works :)

11:38 PM
PID_CALIBRATE heater=extruder TARGET=210
11:39 PM
sample:1 pwm:0.9000 asymmetry:2.3120 tolerance:n/a
11:39 PM
sample:2 pwm:0.6595 asymmetry:1.9820 tolerance:n/a
11:40 PM
sample:3 pwm:0.4745 asymmetry:0.4235 tolerance:n/a
11:40 PM
sample:4 pwm:0.4409 asymmetry:0.1787 tolerance:0.4591
11:40 PM
sample:5 pwm:0.4272 asymmetry:0.1439 tolerance:0.2322
11:41 PM
sample:6 pwm:0.4157 asymmetry:0.5714 tolerance:0.0588
11:41 PM
sample:7 pwm:0.3742 asymmetry:-0.1283 tolerance:0.0668
11:42 PM
sample:8 pwm:0.3832 asymmetry:0.2627 tolerance:0.0530
11:42 PM
sample:9 pwm:0.3637 asymmetry:-0.1609 tolerance:0.0520
11:42 PM
sample:10 pwm:0.3752 asymmetry:-0.0061 tolerance:0.0195
11:42 PM
PID parameters for 210.00°C: pid_Kp=13.915 pid_Ki=1.215 pid_Kd=39.832
Heater: extruder
Tolerance: 0.0200
Profile: default
The SAVE_CONFIG command will update the printer config file
with these parameters and restart the printer.

@rogerlz rogerlz requested a review from bwnance February 21, 2024 23:45
@Zeanon
Copy link
Contributor

Zeanon commented Feb 21, 2024

Awesome
I shouldn't have overlooked those methods, sorry

@rogerlz
Copy link
Contributor Author

rogerlz commented Feb 22, 2024

Awesome I shouldn't have overlooked those methods, sorry

thank you again!

if you are on Discord, join the armchair-engineering server and talk to us in the #danger-klipper channel!

@Zeanon
Copy link
Contributor

Zeanon commented Feb 22, 2024

I am on the armchair DC already
My handle is LastZeanon :)

@rogerlz rogerlz merged commit b805222 into master Feb 22, 2024
2 checks passed
@rogerlz rogerlz deleted the pid-profile branch February 22, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants