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

Allow default metadata parser timeout to be configurable #862

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

moggieuk
Copy link
Contributor

@moggieuk moggieuk commented May 27, 2024

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.

Copy link

@mikeytdisco mikeytdisco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried this change and this change fixes the issue I was having.

@Arksine
Copy link
Owner

Arksine commented Jun 3, 2024

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.

Allow default metadata parser timeout to be configurable

Signed-off-by: Paul Morgan [email protected]
@moggieuk
Copy link
Contributor Author

moggieuk commented Jun 9, 2024

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

@moggieuk moggieuk reopened this Jun 9, 2024
@Arksine Arksine merged commit 4d6037c into Arksine:master Jun 23, 2024
2 checks passed
@Arksine
Copy link
Owner

Arksine commented Jun 23, 2024

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants