Proposal: Active "dirty" files

General Discussion
1

When Klipper detects modified or added files in it’s code, it reports the Klipper version as dirty. While this can help to catch certain manufacturer-broken Klipper versions, I think this has the unintended side effect of making it hard to get support when even a single file is added (not modified).

While some extras are fairly simple to uninstall and reinstall, more extensive extras, like Happy Hare, are fairly involved to uninstall and reinstall, while commenting out the related lines in printer.cfg is very straightforward and easily reversible.

Currently, the klippy.log prints out the following information on startup (taken from the KB article):

Git version: 'v0.12.0-439-g1fc6d214-dirty'
Untracked files: klippy/extras/autotune_tmc.py, klippy/extras/gcode_shell_command.py, klippy/extras/motor_constants.py
Modified files: klippy/chelper/serialqueue.c
Branch: master
Remote: origin
Tracked URL: https://github.com/Klipper3d/klipper

I propose to add another two lines to show which extras modules are actually active and which are inactive. Since inactive extras (ones without a section in printer.cfg) are never imported or executed, they don’t have any effect on Klipper’s functionality.

Active untracked extras: klippy/extras/autotune_tmc.py, klippy/extras/motor_constants.py
Inactive untracked extras: klippy/extras/gcode_shell_command.py

The new proposed startup lines:

Git version: 'v0.12.0-439-g1fc6d214-dirty'
Untracked files: klippy/extras/autotune_tmc.py, klippy/extras/gcode_shell_command.py, klippy/extras/motor_constants.py
Active untracked extras: klippy/extras/autotune_tmc.py, klippy/extras/gcode_shell_command.py, klippy/extras/motor_constants.py
Inactive untracked extras: klippy/extras/gcode_shell_command.py
Modified files: klippy/chelper/serialqueue.c
Branch: master
Remote: origin
Tracked URL: https://github.com/Klipper3d/klipper

Here, the only active untracked extras are TMC autotune, which can be commented out in the config. Since gcode_shell_command is inactive, it can have no affect on Klipper.

Feedback on this idea is appreciated.

2 Likes
2

Can you elaborate on what the benefit would be?

In my opinion:

  • “Dirty” sounds a bit harsh. The Linux kernel calls it tainted, which is not really much nicer. Its explanation can be found here: Tainted kernels — The Linux Kernel documentation
  • The flags give an early warning that something has been modified. What and with which consequences remain to be evaluated.
  • Someone who can read the klippy.log is very well able to distinguish between the modifications and also able to assess if issues and modifications are related.
  • There is only one single extra that is deemed generally harmless, and this is gcode_shell_command.py. However, even this can be used to crash Klipper by executing not-so-harmless shell commands.
  • The proposed differentiation just for one single extra and to avoid situations like MCU 'mcu' shutdown: Move queue overflow Nr.2 - #5 by hcet14 seems not justified.

Just my 2 cents.

3

My post to @3dcoded

was the first thing I was thinking about, when reading the OP.

Ok, even you state this is most of the times harmless. But, it might be harmful.

To avoid posts like this, I would support an FR from 3dcoded. But, I would suggest 3dcoded to reduce it just to “gcode_shell_command.py”. Then there is no crash of Klipper and just a warning in klippy.log about “gcode_shell_command.py” without stopping Klipper.

Just my 2ct.

4

Here’s my list of untracked files that I would like to see allowed:
[‘klippy/extras/autotune_tmc.py’, ‘klippy/extras/gcode_shell_command.py’, ‘klippy/extras/led_effect.py’, ‘klippy/extras/motor_constants.py’, ‘klippy/extras/z_calibration.py’]

5

Thank you all for your replies.

My goal with this isn’t to create a whitelist of “good” extras. It’s instead to allow for disabling extras and reflecting that in the klippy.log.

The issue with this is that TMC autotune has been traced to some stepper errors before. LED effect is known to cause TTC as well. And any of these can be updated at any time, so there’s no way to know if they’re “good” extras.

Maybe this example will illustrate my idea better. Say you have TMC autotune setup (config taken from README).

# printer.cfg
[autotune_tmc stepper_x]
motor: ldo-42sth48-2004mah
[autotune_tmc stepper_y]
motor: ldo-42sth48-2004mah

[autotune_tmc stepper_z]
motor: ldo-42sth48-2004ac
[autotune_tmc stepper_z1]
motor: ldo-42sth48-2004ac
[autotune_tmc stepper_z2]
motor: ldo-42sth48-2004ac
[autotune_tmc stepper_z3]
motor: ldo-42sth48-2004ac

[autotune_tmc extruder]
motor: ldo-36sth20-1004ahg

It’s significantly easier to just comment out these lines in your printer.cfg than it is to remove autotune_tmc.py and motor_constants.py, and both have the same result.

I’ve looked into Klippy’s code and extras are never imported (no code can be executed) unless their corresponding section is present in the printer.cfg (or included files).

Currently, whenever the klippy.log says v0.13.....-dirty, it’s pretty hard to get help if there’s a single added file. My idea is that by adding an “Inactive extras” entry to the klippy.log, if that list is empty, then extras can’t have an affect on Klipper.

This would not interfere with:

  • Modified files
  • Klipper (MCU code)
  • Non-extra files (e.g. kinematics)

Hopefully that explains it a bit better.

6

I misunderstood your intention, sorry. I also would like to take my “like” back.

I meant that.

There is still no good idea for https://klipper.discourse.group/t/new-proposal-for-klipper-extension-support (let’s say, I’m not aware of).

7

Alas, we can’t be sure of that. A code modification could alter the behavior of the code, even across “RESTART” commands, which means the config file itself isn’t authoritative.

That aside, in my opinion, the goal of the code modification checks is to provide accurate information in the log. It’s up to the people reviewing the log to determine how they use that information. Lots of people here (and elsewhere) will continue to research issues if they feel they can still provide meaningful assistance with the reported modifications. I’m not sure what advantage there would be in not reporting some modifications.

Cheers,
-Kevin

2 Likes