fix: allow macro prompt to remain open even if the events were more than 100 events ago

[ammmze]

Description

Previously we were only looking for a series of prompt action events within the last 100 events. However, if things are happening in the background while the prompt is still open, those events can keep racking up and eventually the prompt window will close (though klipperscreen still displays it).

I've now refactored this code so that it will internally keep an array of the prompt events and we will process through all the events at first, but we will set a checkpoint at the most recent processed event so that we don't go back and re-process ones that were already done. Effectively this means on first load sure we iterated through potentially a lot of events, but then after that we're really only processing through at most a few events. It is certainly less concise than the previous implementation, but I believe it is a more robust solution and perhaps a bit more performant. Previously every time we asked for this.macroPromptEvents it would iterate through the last 100 messages doing its filters and map. And that would get called numerous times in the scope of a single render. With this new process each time we call it (aside from the first load), it typically will only have at most a few items to filter through.

In addition, the part of the code that would parse out the message, because it would replace all double quotes, it meant that I could not do this:

RESPOND TYPE=command MSG="action:prompt_begin \"Question of the day\""
RESPOND TYPE=command MSG="action:prompt_button Foo Bar|RESPOND MSG=\"FOO BAR\""
RESPOND TYPE=command MSG="action:prompt_show"

If I tried to click that button I would get the following error:

Malformed command 'RESPOND MSG=FOO BAR'

I've updated the message parsing to effectively just strip off matching open and closing single or double quotes from the message. For example, all of the following would result in a message of Question of the day:

RESPOND TYPE=command MSG="action:prompt_begin \"Question of the day\""
RESPOND TYPE=command MSG="action:prompt_begin 'Question of the day'"
RESPOND TYPE=command MSG="action:prompt_begin Question of the day"

Here's a quick couple macros that demonstrate the bug. Execute the MAINSAIL_BUG_TEST macro and then click the button to Send 100. This will then execute 100 respond macros and the prompt dialog in mainsail will close without the user intending to close it.

[gcode_macro MAINSAIL_BUG_TEST]
gcode:
  RESPOND TYPE=command MSG="action:prompt_begin Question of the day"
  RESPOND TYPE=command MSG="action:prompt_button Send 100|MAINSAIL_BUG_TEST_MSG LOOP=100"
  RESPOND TYPE=command MSG="action:prompt_button Send 10|MAINSAIL_BUG_TEST_MSG LOOP=10"
  RESPOND TYPE=command MSG="action:prompt_button Send 1|MAINSAIL_BUG_TEST_MSG LOOP=1"
  RESPOND TYPE=command MSG="action:prompt_show"

[gcode_macro MAINSAIL_BUG_TEST_MSG]
gcode:
  {% set loop  = params.LOOP|default(10)|int %}
  {% for i in range(loop) %}
    RESPOND MSG="Iteration {i}"
  {% endfor %}

Related Tickets & Documents

None found

Mobile & Desktop Screenshots/Recordings

[optional] Are there any post-deployment tasks we need to perform?

[meteyou]

To be honest, while displaying a Macro prompt, there should not be any traffic in the console, because klippy should be in pause. i have to double-check your code how performant it works, because "the last 100 entries" was just a performance thing...

[ammmze]

To be honest, while displaying a Macro prompt, there should not be any traffic in the console, because klippy should be in pause. i have to double-check your code how performant it works, because "the last 100 entries" was just a performance thing...

Thank you for taking the time to review. While my example macro prompt was pretty simplified, I would counter and say that there are perfectly valid ways that klippy would not be paused while a prompt is displayed. Another example, FLAP (haven't used it, but just came across it), will trigger prompts for things like runout, and insert events. Once those prompts are displayed, klippy technically is not paused. They can then have those buttons run whatever macros they want and choose whether or not the prompt should close after the button is pressed. And to further explain what I am working on where I encountered this is in Happy Hare v3, i'm working on an interactive calibration procress that will walk the user through a series of prompts that are driven from a separate thread, which makes it easier to collect context along the way and make the the workflow more dynamic. Preview of that work can be found here.

Perhaps a compromise would be to add the slice back in, but with a higher value (5-10k). I just think 100 too little. That way there is a defined cap to the number of events processed on initial load. But I would still suggest keeping the rest of this refactor because it will reduce the number of events we iterate over in each render significantly.