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

Feature request: Improve the way saved variables are managed #87

Open
fbeauKmi opened this issue Nov 4, 2023 · 3 comments
Open

Feature request: Improve the way saved variables are managed #87

fbeauKmi opened this issue Nov 4, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@fbeauKmi
Copy link
Contributor

fbeauKmi commented Nov 4, 2023

Currently, saved variables are stored at the end of the printer.cfg file in an odd structure. In my humble opinion, there are several areas for improvement.

  • Store these variables in a separate file from printer.cfg (without those ugly comments structure). Hopefully, this will simplify the structure. This file would simply be read after printer.cfg when loading the configuration.
  • Allow values to be overridden (as is alreadyin the rest of the configuration). Indeed, in the case of a distributed configuration, it is currently impossible to perform a SAVE_CONFIG, klipper returns an error, (I don't know the exact mechanism). -
  • So, commenting on configuration variables becomes obsolete

I hope I've made myself clear, thanks for your work!

@Piezoid Piezoid self-assigned this Nov 4, 2023
@Piezoid
Copy link
Contributor

Piezoid commented Nov 4, 2023

  • Allow values to be overridden (as is alreadyin the rest of the configuration). Indeed, in the case of a distributed configuration, it is currently impossible to perform a SAVE_CONFIG, klipper returns an error, (I don't know the exact mechanism).

In the current state, the saved values override the user values, if we simply remove the check.
That's what you want, right?

Maybe we can turn the error into a warning?

  1. Ideally the user should be warned when a overridden value is changed in the manually managed config: "The change to the parameter x has no effect as it is overridden by a saved value". However, this require tracking config changes,
  2. Warn after SAVE_CONFIG: "You might want to remove parameter x which is now overridden by a saved value",
  3. Warn every klippy restart,
  4. 2. but also logs overridden parameters at boot. This could be generalized to duplicate parameters across includes.

@fbeauKmi
Copy link
Contributor Author

fbeauKmi commented Nov 5, 2023

In the current state, the saved values override the user values, if we simply remove the check. That's what you want, right?
Yes, it is.

Maybe we can turn the error into a warning?

1. Ideally the user should be warned when a overridden value is changed in the manually managed config: "The change to the parameter x has no effect as it is overridden by a saved value". However, this require tracking config changes,

2. Warn after `SAVE_CONFIG`: "You might want to remove parameter x which is now overridden by a saved value",

3. Warn every klippy restart,

4. `2.` but also logs overridden parameters at boot. This could be generalized to duplicate parameters across includes.

TBH, i didn't think about warnings.
Is a setting as allow_saveconfig_to_override : true is possible/good idea ? with default value = false. When false you got the actual behavior, when true, no really need to warn as you know what you do.

@MasturMynd
Copy link
Contributor

  • Allow values to be overridden (as is alreadyin the rest of the configuration). Indeed, in the case of a distributed configuration, it is currently impossible to perform a SAVE_CONFIG, klipper returns an error, (I don't know the exact mechanism). -

Wanted to mention that this portion of the inquiry has been resolved via #153

@rogerlz rogerlz added the enhancement New feature or request label Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants