-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Implemented #2132 + config.ini cleanup + bugfix/code optimization #2146
Conversation
👍🏻 like always!!! |
@pfn If you can include it later on, I agree also. I just mentioned it because digant was involved in the conversation regarding PID, and he mentioned that the timeout could be increased. |
I preferred the old emulate_m600 name, imo it should be a present tense action for key names. Also, this is pretty cumbersome for users as anyone with a custom configuration will have to go through and make sure everything is correctly modified. This sort of mass update would be nice if there were a clean way to migrate users' config. |
But it's also a feature like "terminal ACK", "notification M117" etc... which you decide to enable/disable in the Feature/UI menu. Also the verb can create confusion in some cases (e.g. invert axis means more that you have the possibility to change the printer axis direction while it is used by the TFT as a reference to bind the "move" buttons in the right way. |
IMPROVEMENTS:
Scope of those changes are also to force some people to finally update their config.ini starting from the most updated config.ini provided here instead of simply integrating new params in their old config.ini. This should avoid (I hope) to see so many bug reports referring to missing or old comments present in the config.ini file reporting wrong (old) values range etc...
Below a list of the renamed params in config.ini:
BUGFIXES:
fixes #2138
resolves #2132
PR STATE: ready to merge