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

SET_GCODE_VARIABLE does not work with non-numeric values #4816

Closed
pedrolamas opened this issue Oct 19, 2021 · 6 comments
Closed

SET_GCODE_VARIABLE does not work with non-numeric values #4816

pedrolamas opened this issue Oct 19, 2021 · 6 comments

Comments

@pedrolamas
Copy link
Contributor

I've noticed that SET_GCODE_VARIABLE fails when used with strings.

Take the following example:

[gcode_macro TEST]
variable_v: ''
gcode:
  SET_GCODE_VARIABLE MACRO=TEST VARIABLE=v VALUE='value'

Calling the above macro will result in the following output:

$ TEST
Unable to parse 'value' as a literal
Unable to parse 'value' as a literal

Looking at the code, the issue seems to be related to this specific bit:

try:
literal = ast.literal_eval(value)
except ValueError as e:
raise gcmd.error("Unable to parse '%s' as a literal" % (value,))

@cmidgley
Copy link

Thanks, @pedrolamas for submitting a new ticket. As mentioned in #4107, I also found the same problem (I have a bit of additional supporting information in that post as well).

@cmidgley
Copy link

This issue keeps coming up for me. In my latest use case, I want to have a MESSAGE macro that sends a message toM117 and RESPOND. I would like it to save the message, and then have a MESSAGE_PUSH macro that sends a different message, and then a MESSAGE_POP macro that restores the prior message. This way, I can have a macro do something that shows a message, but then return to the original message when it is done (for example, during a PAUSE/RESUME or operations like homing and leveling).

The obvious way to do this would be to simply save the message in a gcode variable, and later reference it when MESSAGE_POP is used. For example (hand-entered code follows, so likely has errors/syntax issues):

[gcode_macro MESSAGE]
gcode:
  {% set msg = params.MSG %}
  M117 {msg}
  RESPOND "{msg}"
  SET_GCODE_VARIABLE macro=MESSAGE variable=last_message value="{msg}"

[gcode_macro MESSAGE_PUSH]
gcode:
  {% set msg = printer["gcode_macro MESSAGE"].last_message ~ "(" ~ params.MSG ~ ")" %}
  M117 {msg}
  RESPOND "{msg}"

[gcode_macro MESSAGE_POP]
gcode:
  MESSAGE msg="{printer['gcode_macro MESSAGE'].last_message}"

I have several other cases where I want functionality such as this, such as managing the color of the lights on the printer, or tracking the name of the filament currently in each extruder. Since Klipper can only store a single number (not strings, not lists) I am unable to do these types of operations. I've also tried using SAVE_VARIABLE (persists to disk) but it also fails with non-numeric values.

Perhaps there is another way around this issue? I've tried every semi-obvious path, but no luck so far.

On a side note, I have been using Marlin/Repetier for many years, and Klipper totally blows them out of the water. I am now converting all my printers to Klipper. Fantastic / amazing project, thank you!

@wlhlm
Copy link
Contributor

wlhlm commented Oct 23, 2021

The code tries to interpret the value given to SET_GCODE_VARIABLE as a python literal, so if you want to give it a string it needs to be written like a python string literal (enclosed in quotes):
@pedrolamas

  SET_GCODE_VARIABLE MACRO=TEST VARIABLE=v VALUE='"value"'

Similar in case jinja2 expansion is involved:
@cmidgley

SET_GCODE_VARIABLE macro=MESSAGE variable=last_message value='"{msg}"'

Granted, it is a bit of a fiddly endeavor, especially when nested quotes are involved.

@pedrolamas
Copy link
Contributor Author

Ah, does make sense considering the source code!!

Thanks for pointing that out @wlhlm, and considering this I will check if we can add something in the documentation to state such behavior!

@cmidgley
Copy link

Thanks @wlhlm! That nailed the problem. Not obvious, but makes sense and works great. This issue should be closed, but for any future readers interested, here is a working full-stack based message push/pop. Perhaps this can help others with handling some of the oddities of Jinja2 / Klipper syntax and list management...?

[gcode_macro MESSAGE]
description: Output message on console and display (MSG="required-message")
variable_msg_stack: [""]
gcode:
    {% if params.MSG is not defined %}
        { action_respond_info("MESSAGE macro called without MSG parameter; ignored") }
    {% else %}
        # add message to top entry on stack
        {% set msg = params.MSG|default("") %}
        {% set _ = msg_stack.pop() %}
        {% set msg_stack = msg_stack + [msg] %}
        SET_GCODE_VARIABLE macro=MESSAGE variable=msg_stack value="{msg_stack}"
        
        # output message to display and console
        MESSAGE_UPDATE
    {% endif %}

[gcode_macro MESSAGE_PUSH]
description: Push the message stack, and optionally display a message (MSG="optional-message")
gcode:
    {% set msg = params.MSG|default("") %}

    # push the stack down one level
    {% set msg_stack = printer["gcode_macro MESSAGE"].msg_stack %}
    {% set msg_stack = msg_stack + [""] %}
    SET_GCODE_VARIABLE macro=MESSAGE variable=msg_stack value="{msg_stack}"

    # output message if we have one
    {% if params.MSG is defined %}
        MESSAGE msg="{msg}"
    {% endif %}
    

[gcode_macro MESSAGE_POP]
description: Pop the message stack, restoring the prior message
gcode:
    {% set msg_stack = printer["gcode_macro MESSAGE"].msg_stack %}
    {% if msg_stack|length > 1 %}
        # pop the message off the stack
        {% set _ = msg_stack.pop() %}
        SET_GCODE_VARIABLE macro=MESSAGE variable=msg_stack value="{msg_stack}"

        # update the displays
        MESSAGE_UPDATE
    {% else %}
        { action_respond_info("MESSAGE_POP called with no remaining MESSAGE_PUSH items; ignored") }
    {% endif %}

[gcode_macro MESSAGE_RESET]
description: Resets the message stack (recommended in PRINT_START to repair unmatched MESSAGE_PUSH/MESSAGE_POP)
gcode:
    SET_GCODE_VARIABLE macro=MESSAGE variable=msg_stack value="['']"

[gcode_macro MESSAGE_UPDATE]
description: Internal macro used to display the current message from the stack (MSG="required-message")
gcode:
    # get message from the stack, eliminating any blank messages
    {% set msg_stack = printer["gcode_macro MESSAGE"].msg_stack|reject("eq", "")|list %}
    {% if msg_stack|length > 0 %}
        {% set msg = msg_stack[msg_stack|length - 1] %}
    {% else %}
        {% set msg = "" %}
    {% endif %}

    # display, including prior messages if we have any
    {% if msg_stack|length > 1 %}
        {% set _ = msg_stack.pop() %}
        {% set prior_msgs = msg_stack|reverse|join(", ") %}
        M117 {msg} ({prior_msgs})
        RESPOND MSG="{msg} ({prior_msgs})"
    {% else %}
        M117 {msg}
        {% if msg != "" %}
            RESPOND MSG="{msg}"
        {% endif %}
    {% endif %}

@pedrolamas
Copy link
Contributor Author

I've checked the existing documentation for SET_GCODE_VARIABLE and noticed this bit:

The provided VALUE is parsed as a Python literal.

This explains that the VALUE will be Python evaluated, so if we want it to be considered a string, it needs to be quoted as described above.

Given this, I am closing this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants