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

Changed SAVE_CONFIG to update include configs #153

Merged
merged 1 commit into from
Feb 17, 2024

Conversation

MasturMynd
Copy link
Contributor

@MasturMynd MasturMynd commented Feb 16, 2024

Proceeding under my own merits for #151, regarding #149

This update provides two changes to how autosave data is handled via klipper. The first is that included configs are now updated properly. In order to do this, the second change, a dedicated backups directory has been introduced; the importance of which is covered in further detail below. The backups directory will be created in the default configs directory for the printer.

The choice to make autosaving includes the danger option rather than the alternate backups directory is two-fold. Changing the path that a backup is saved to won't have any negative impact given that the path is pulled from klippy itself, then appended with the directory name. Additionally, code complexity and failure risk were considered for both options.

I've done my best to provide short but descriptive comments so the code can be easily understood.

More detailed information about the updates:

First is the desire to process included files during SAVE_CONFIG. As I'm sure is known, SAVE_CONFIG in its current state will only apply autosave data to printer.cfg. This results in conflicts when something such as PID values are stored in a separate config file. The proposed changes will sift through all of the config files used to instruct machine operation for conflicts with autosave data and comment them out.

The second change, the config_backups directory, was brought along out of necessity. During testing, @lraithel15133 was using a machine that had all of his additional config files in a single folder. Then, a [include foldername/*.cfg] was used for simplicity and to ensure that any new config additions were automatically loaded. Since the naming scheme that klipper uses for backing up configs retains the .cfg extension, the backups saved to the foldername directory were loaded via the include block resulting in error from _disallow_include_conflicts. Rather than change the naming format that klipper uses for backup files and potentially causing issues downstream with mainsail / fluid, the decision was made to add a directory specific for backup configs.

@rogerlz rogerlz changed the title Changed SAVE_CONFIG to update inclide configs Changed SAVE_CONFIG to update include configs Feb 17, 2024
@rogerlz
Copy link
Contributor

rogerlz commented Feb 17, 2024

@MasturMynd thank you

@rogerlz rogerlz merged commit 09b9319 into KalicoCrew:master Feb 17, 2024
2 checks passed
@MasturMynd MasturMynd deleted the autosave-includes branch February 25, 2024 13:22
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.

2 participants