vlachoudis / bCNC

GRBL CNC command sender, autoleveler and g-code editor
GNU General Public License v2.0
1.54k stars 528 forks source link

Fix user button interpolation #1817

Closed jlirochon closed 1 year ago

jlirochon commented 1 year ago

Actual behavior

Harvie commented 1 year ago

I beleive users were actualy reporting scripting to be broken. https://github.com/vlachoudis/bCNC/wiki/Scripting

Maybe this might be related since the scripting uses % as well ???

eg.: #1661 #1289 #1304

jlirochon commented 1 year ago

Oh... Thx @Harvie I was not aware of this scripting feature !

After digging into the history, I believe that scripting feature stopped working at some point when switching to Python3. But some of the issues you linked seem more related to the moment the script is interpreted VS the moment the commands are executed by the CNC.

If we test outside of bCNC using the following test.ini :

[Buttons]
foo = %wait

Here is Python2 :

Python 2.7.18 (default, Mar  5 2023, 17:48:22)
[GCC Apple LLVM 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import ConfigParser
>>> config = ConfigParser.ConfigParser()
>>> config.read("test.ini")
['test.ini']
>>> config.get("Buttons", "foo")
'%wait'

Here is Python3 (like actual bCNC code) :

Python 3.9.7 (default, Sep 25 2021, 12:02:50)
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import configparser
>>> config = configparser.ConfigParser()
>>> config.read("test.ini")
['test.ini']
>>> config.get("Buttons", "foo")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/julien/.asdf/installs/python/3.9.7/lib/python3.9/configparser.py", line 799, in get
    return self._interpolation.before_get(self, section, option, value,
  File "/Users/julien/.asdf/installs/python/3.9.7/lib/python3.9/configparser.py", line 395, in before_get
    self._interpolate_some(parser, option, L, value, section, defaults, 1)
  File "/Users/julien/.asdf/installs/python/3.9.7/lib/python3.9/configparser.py", line 442, in _interpolate_some
    raise InterpolationSyntaxError(
configparser.InterpolationSyntaxError: '%' must be followed by '%' or '(', found: '%wait'

Here is a working Python3 example, where interpolation is disabled :

Python 3.9.7 (default, Sep 25 2021, 12:02:50)
[Clang 12.0.5 (clang-1205.0.22.9)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import configparser
>>> config = configparser.ConfigParser(interpolation=None)
>>> config.read("test.ini")
['test.ini']
>>> config.get("Buttons", "foo")
'%wait'

Adding interpolation=None seems to be the simplest fix to restore the initial scripting behavior. Interpolation of config variables won't be possible anymore. I don't know if someone was using this, or if it was an accidental feature. Merging this PR is not required.

Otherwise (requires merging this PR) :

  1. consider updating the docs to escape all the % in scripts (double %%). No new feature, no removed feature, scripting works again with some changes : %wait becomes %%wait
  2. consider changing the scripting prefix to something else, say @ or whatever... % was not a good choice from the start since this is the default interpolation character. Looks like Python2 was more indulgent but that was by luck. No new feature, no removed feature, scripting works again with some changes : %wait becomes @wait
  3. consider changing the interpolation character to $. This would allow accessing all config variables from other sections like this : ${CNC:feedmax_z} BUT this would require escaping all configs like $30=0 to $$30=0. More powerful interpolation, no removed feature, scripting works again as is, requires changes to configs using $
jlirochon commented 1 year ago

@Harvie I think this should be fixed, user buttons are barely usable.

If you are Ok to drop variable interpolation I can submit a one liner patch. I suspect interpolation was not consciously enabled, and was only a side effect. It could be re-enabled later, or we could provide an alternative way of accessing config variables in scripts.

Concerning the various tickets talking about %wait not working, I am able to reproduce this problem in my debugger. It doesn't wait at all. Unrelated to the interpolation. From quick look the thread actually executing commands doesn't care about the waiting state. I will try to fix this, but let's fix the interpolation first ?

jlirochon commented 1 year ago

I'm closing this one. #1829 is the new proposal.