[BUG] EVENT_GCODE_TOOLCHANGE_ALWAYS_RUN does not work

[GenaUser]

Did you test the latest bugfix-2.0.x code?

Yes, and the problem still exists.

Bug Description

When the line "#define EVENT_GCODE_TOOLCHANGE_ALWAYS_RUN" is activated, the T0 and T1 commands cause the printer to be reset or the printer freeze (printer busy).

When the line is deactivated, the extra G-code works as before (only after homing).

Bug Timeline

10/19/22

Expected behavior

Be able to use the additional G code to be executed with tool change commands T0 and T1 all time. Even if the homing is not done.

Actual behavior

No response

Steps to Reproduce

Enable #define EVENT_GCODE_TOOLCHANGE_ALWAYS_RUN

Version of Marlin Firmware

2000902

Printer model

No response

Electronics

No response

Add-ons

No response

Bed Leveling

No response

Your Slicer

No response

Host Software

No response

Additional information & file uploads

No response

[thinkyhead]

The EVENT_GCODE_TOOLCHANGE_ALWAYS_RUN option can be considered experimental since there are likely some scenarios where we want to skip the tool-change G-code. Since the normal behavior of T0 / T1 is only slightly affected by the new option, let's take a quick look at the difference. The current code for T command looks like this…

  tool_change(tool_index
    #if HAS_MULTI_EXTRUDER
      ,  TERN(PARKING_EXTRUDER, false, tool_index == active_extruder) // For PARKING_EXTRUDER motion is decided in tool_change()
      || parser.boolval('S')
    #endif
  );

Breaking this down, for PARKING_EXTRUDER the no_move value is the default false so that shouldn't be affected. If the tool is the same, the no_move flag is set, which is sensible. And finally, the S flag can also set no_move.

The new option only affects calls to tool_change where no_move ends up being set, including calls that pass true explicitly, or when tool_change itself sets no_move because homing is needed.

So then, what is the difference in T0 / T1 behavior that could be causing a lockup? Do you still see a lockup if you send G28 just before you send a T0 / T1 command?

To go further in the analysis, it would be helpful to know your EVENT_GCODE_TOOLCHANGE_T0 and EVENT_GCODE_TOOLCHANGE_T1 settings. Maybe they are just not suitable for use with the new option.

[GenaUser]

Thank you @thinkyhead for your explanations.
The "EVENT GCODE_TOOL CHANGE ALWAYS_RUN" option causes a reset or a "Busy processing, please wait ..." message with T0 or T1.
G28 also causes a printer freezing with the message "Busy processing, please wait ..."

The head of my printer uses a single motor with a change between T0 and T1 by a pendulum driven by 2 motors ("I" and "J") :

#define EVENT_GCODE_TOOLCHANGE_T0 "G28 J \ n"
#define EVENT_GCODE_TOOLCHANGE_T1 "G28 I \ n"

The use of 2 motors with G28, makes it possible to ensure a physical positioning of the head rather than 1 motor using G28 in one direction and a displacement length in the other. It is a prevention for a possible step loss.
(The problem is the same with only 1 axis)

Examples:
https://youtu.be/S5_BkCK7rr4
https://youtu.be/I6jBb85iDac

I tested the Extra G-code without G-code, and the problems disappeared.
"G28 J" and "G28 I" do not seem to be suitable for the function. A loop problem with G28?
When the "EVENT GCODE_TOOL CHANGE ALWAYS_RUN" option is disabled, G28 operates normally. Perhaps in this case, the taking into account of the offsets, and therefore of the extra G-codes are only taken into account after this general homing, which does not create any redundancy.
If so, is it possible to solve the problem?

EVENT_GCODE_TOOLCHANGE_T0 and T1 are very useful (thanks to DerAndere1), but the fact that it is dependent on homing reduce the possibilities and may be the cause of this problem.

In the future, the ideal for this function would perhaps be to approach what Repetier V2 offers.
It is possible to define two G-code "TOOL_EXTRUDER(ToolExtruder1,.....,startScript, endScript)" for all extruders (not only for T0 and T1) without any particular restriction of use.

[DerAndere1]

G28 I \ n and G28 J \ n is not valid RS 274 Gcode syntax. I guess your config looks similar to this:

#define LINEAR_AXES 5
#define AXIS4_NAME 'A'
#define AXIS5_NAME 'B'

In that case, you can probably fix the issue by changing to valid G-codes:

#define EVENT_GCODE_TOOLCHANGE_T0 "G28 A"
#define EVENT_GCODE_TOOLCHANGE_T1 "G28 B"

Note, that the word to reference an axis corresponds to AXIS*_NAME and also note that line breaks are usually not required. Line breaks (correct \n without space) are used only to separate two or more Gcode commands. With the corrected event gcode, you can probably disable EVENT_GCODE_TOOLCHANGE_ALWAYS_RUN.

[GenaUser]

#define AXIS4_NAME 'A' and #define AXIS5_NAME 'B' are well identified.
I tested "G28 A" and "G28 B" with the same result:
If EVENT_GCODE_TOOLCHANGE_ALWAYS_RUN is disabled, tools are synchronized with the T0 and T1 commands (using G-code), but only when all axes are homed.
If EVENT_GCODE_TOOLCHANGE_ALWAYS_RUN is enabled, printer freezes or reset with T0, T1 or G28

[GenaUser]

The malfunction comes from a loop created with G28.
I tested #define EVENT_GCODE_TOOLCHANGE_T0 "" (without g-code),
and #define EVENT_GCODE_TOOLCHANGE_T1 "G28 A"
The problem disappears for T1. It changes the tool even if the homing is not done.

To confirm the origin, I tested this:
In G28.cpp, if "tool_change (0, true);" (line 315) and "tool_change (old_tool_index, TERN (PARKING_EXTRUDER ,! Pe_final_change_must_unpark, DISABLED (DUAL_X_CARRIAGE)));" (line 493) are disabled, T0 and T1 work as expected with "#define EVENT_GCODE_TOOLCHANGE_ALWAYS_RUN".

Is it possible to put an index for not activat "EVENT_GCODE_TOOLCHANGE_T0" and "EVENT_GCODE_TOOLCHANGE_T1" during G28 homing?

[DerAndere1]

All this is potentially unsafe and untested. but you can try something like this:

Marlin/Marlin/src/gcode/calibrate/G28.cpp

Line 315 in de6dfc9

tool_change(0, true);

change that to:

    #if defined(EVENT_GCODE_TOOLCHANGE_T0) || defined(EVENT_GCODE_TOOLCHANGE_T1)
    // don't change tool during processing of EVENT_GCODE_TOOLCHANGE_T[0,1] to prevent busy loop
      if (!processing_event_gcode_toolchange) tool_change(0, true);
    #else
      tool_change(0, true);
    #endif

---

Marlin/Marlin/src/gcode/calibrate/G28.cpp

Line 493 in de6dfc9

tool_change(old_tool_index, TERN(PARKING_EXTRUDER, !pe_final_change_must_unpark, DISABLED(DUAL_X_CARRIAGE))); // Do move if one of these

Change that to:

  #if defined(EVENT_GCODE_TOOLCHANGE_T0) || defined(EVENT_GCODE_TOOLCHANGE_T1)
    if (!processing_event_gcode_toolchange) 
      tool_change(old_tool_index, TERN(PARKING_EXTRUDER, !pe_final_change_must_unpark, DISABLED(DUAL_X_CARRIAGE)));
  #else
    tool_change(old_tool_index, TERN(PARKING_EXTRUDER, !pe_final_change_must_unpark, DISABLED(DUAL_X_CARRIAGE)));   // Do move if one of these
  #endif

---

In https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.0.x/Marlin/src/module/tool_change.h#L55
add the following:

#if (defined(EVENT_GCODE_TOOLCHANGE_T0) || defined(EVENT_GCODE_TOOLCHANGE_T1)) &&  HAS_MULTI_HOTEND
  bool processing_event_gcode_toolchange = false;
#endif

---

Marlin/Marlin/src/module/tool_change.cpp

Lines 1311 to 1319 in de6dfc9

	#ifdef EVENT_GCODE_TOOLCHANGE_T0
	if (new_tool == 0)
	gcode.process_subcommands_now(F(EVENT_GCODE_TOOLCHANGE_T0));
	#endif
	#ifdef EVENT_GCODE_TOOLCHANGE_T1
	if (new_tool == 1)
	gcode.process_subcommands_now(F(EVENT_GCODE_TOOLCHANGE_T1));
	#endif

change that to:

      #ifdef EVENT_GCODE_TOOLCHANGE_T0
        if (new_tool == 0) {
          #if HAS_MULTI_HOTEND
            processing_event_gcode_toolchange = true;
          #endif
          gcode.process_subcommands_now(F(EVENT_GCODE_TOOLCHANGE_T0));
          #if HAS_MULTI_HOTEND
            processing_event_gcode_toolchange = false;
          #endif
        }
      #endif

      #ifdef EVENT_GCODE_TOOLCHANGE_T1
        if (new_tool == 1) {
          #if HAS_MULTI_HOTEND
            processing_event_gcode_toolchange = true;
          #endif
          gcode.process_subcommands_now(F(EVENT_GCODE_TOOLCHANGE_T1));
          #if HAS_MULTI_HOTEND
            processing_event_gcode_toolchange = false;
          #endif
       }
      #endif

[GenaUser]

Thanks for your proposition.
After changes I got compilation errors.
I made the following changes to undo some of them (I'm not sure if these good method):

• ")" Expected G28.cpp line 315
  "#if defined (EVENT_GCODE_TOOLCHANGE_T0 || EVENT_GCODE_TOOLCHANGE_T1)"
  becomes
  "#if defined (EVENT_GCODE_TOOLCHANGE_T0) || (EVENT_GCODE_TOOLCHANGE_T1)"

• ")" Expected G28.cpp line 498
  "#if defined (EVENT_GCODE_TOOLCHANGE_T0 || EVENT_GCODE_TOOLCHANGE_T1")
  becomes
  "#if defined (EVENT_GCODE_TOOLCHANGE_T0) || (EVENT_GCODE_TOOLCHANGE_T1)"

• "The function call is not allowed in a constant expression, identifier expected, the expression must have the type integral or enum", tool_change.h line 56
  "#if defined ((EVENT_GCODE_TOOLCHANGE_T0 || EVENT_GCODE_TOOLCHANGE_T1)) && HAS_MULTI_HOTEND becomes
  "#if defined (EVENT_GCODE_TOOLCHANGE_T0) || (EVENT_GCODE_TOOLCHANGE_T1) && HAS_MULTI_HOTEND "

• "The argument of type "const __FlashStringHelper *" is incompatible with the parameter of type "char *"" tool_change.cpp line 1317 and 1329
  "gcode.process_subcommands_now (F (EVENT_GCODE_TOOLCHANGE_T0));"
  and
  "gcode.process_subcommands_now (F (EVENT_GCODE_TOOLCHANGE_T1));"
  become
  "gcode.process_subcommands_now (PSTR (EVENT_GCODE_TOOLCHANGE_T0));"
  and
  "gcode.process_subcommands_now (PSTR (EVENT_GCODE_TOOLCHANGE_T1));"

There is still an error that I cannot cancel:
":" token \ "" G28 A \ "" is not valid in preprocessor expressions ",
Configuration_adv.h, line" #define EVENT_GCODE_TOOLCHANGE_T1 "G28 A"

---

Tentatively, I made a change that seems to work.
This is probably not a "clean" solution, as I am not programmer.

In G28.cpp
I added the following lines to create an index at the start and end of the block:

Line 22
int Extra_var = 0;

Change of state of the variable at the start of the block
Line 214 after "void GcodeSuite :: G28 () {"
extern int Extra_var;
Extra_var = 1;

Change of state of the variable at the end of the block
Line 552 after "#endif"
Extra_var = 0;

In tool_change.cpp
I modified the following lines:

Line 1309 addition of
extern int Extra_var;

Line 1312
if (new_tool == 0)
become
if ((new_tool == 0) && (Extra_var == 0))

Line 1317
if (new_tool == 1)
become
if ((new_tool == 1) && (Extra_var == 0))

I have applied this index in other blocks of G28.cpp in order to possibly secure other configurations.

[DerAndere1]

oops. I edited my above suggestion to at least add some mising define operators. But if your own solution already works for you, use it. You may want to rename your variable Extra_var to homing_in_process and its type to bool. But I guess neither yours nor my proposal is a general solution. they both may have a problem for other configurations with PARKING_EXTRUDER: Lets go through your logic and lets assume a situation where tool 1 is the active tool and you send a G28 Gcode manually. The firmware will call G28() which sets Extra var = 1, then calls

Marlin/Marlin/src/gcode/calibrate/G28.cpp

Line 315 in 4dd1f09

tool_change(0, true);

as it tries to temporarily change to tool 0. but this call to tool_chage(0,true) will fail to move your toolchanger: That is because Extra_var is still 1, thus the required EVENT_GCODE_TOOLCHANGE_T0 is not executed during this attemted toolchange.

[GenaUser]

Thanks for your advices.
I would like to try your solution, but I still can't compile.
In tool_chane.ccp;
"cgcode.process_subcommands_now (F (EVENT_GCODE_TOOLCHANGE_T0));" and "gcode.process_subcommands_now (F (EVENT_GCODE_TOOLCHANGE_T1));" generate an incompatibility problem.

If I modify by: "gcode.process_subcommands_now_P (PSTR (EVENT_GCODE_TOOLCHANGE_T0));" and "gcode.process_subcommands_now_P (PSTR (EVENT_GCODE_TOOLCHANGE_T1)) ;" I have an incompatibility with "G28 A" and G28 B ".

I followed your advice for my solution:
I converted the variable to boolean, and changed its name.
I also reduced the area of ​​action of his influence by changing in G28.cpp:

• Line 315
  tool_change (0, true);
  to
  homing_in_process = 1;
  tool_change (0, true);
  homing_in_process = 0;

• and line 493
  tool_change (old_tool_index, TERN (PARKING_EXTRUDER,! pe_final_change_must_unpark, DISABLED (DUAL_X_CARRIAGE)));
  to
  homing_in_process = 1;
  tool_change (old_tool_index, TERN (PARKING_EXTRUDER,! pe_final_change_must_unpark, DISABLED (DUAL_X_CARRIAGE)));
  homing_in_process = 0;

I deleted the other variables that I had added previously at the beginning and at the end of the block.
It works, and testing doesn't seem to reveal any issues.