Arksine / moonraker

Web API Server for Klipper
https://moonraker.readthedocs.io
GNU General Public License v3.0
1.02k stars 392 forks source link

Allow default metadata parser timeout to be configurable #862

Closed moggieuk closed 1 week ago

moggieuk commented 1 month ago

Hi, I'm the author of Happy Hare - a MMU extension to Klipper. Happy Hare adds a couple of pieces of functionality to Moonraker, one of which extends the metadata parsing to find and substitute placeholders in the gcode. E.g. to assess the set of Tools used in a multi-color print. The issue is that this adds a little extra processing time and the hard coded default of 10s is insufficient. (I know that if "object processing" is enabled the default jumps to 300s which is a current workaround).

This PR adds the ability to optionally specify the default (minimum) timeout using an extra optional parameters under file_manager:

[file_manager]
default_metadata_parser_timeout: 30

Which replaces the old default if specified. This can also be used to push object_processing timeout higher if really necessary...

Finally, I increased the code default to 20s because it seems to make more sense in my testing for large gcode files even without the added Happy Hare option. This could remain 10 with the exposed config if necessary.

Side note: If the metadata parsing does fail, then there are bugs in other tools like KlipperScreen (I have a PR to fix as well) that will cause massive amount of errors being written to syslog which will usually lead to Timer to Close errors with Klipper.

Arksine commented 1 month ago

Thanks. I'm okay with adding an option to change the default. The commits (or the PR) need to be signed off indicating acceptance of the DCO before merging.

FWIW, it shouldn't be necessary to change the default time. Regardless of the file's size, the amount of the file processed is capped by the metadata parser. Even on original Pi hardware it should complete well under 10 seconds. If it takes longer it suggests significant I/O issues or some other process starving metadata.py of CPU time.

moggieuk commented 3 weeks ago

Thanks. I re-opened PR with a single commit message in (DCO) form required.

Arksine commented 1 week ago

Thanks.