sdtom commented 6 years ago

Version 1.1.9

Description

Sending this Sequence of commands:

T0
G28 X
T1

Causes the X axis to crash on the tool change. This seems identical to the old issue

3823. There is no clamping on soft stops for a non dual carriage axis (I can see in Marlin_main.cpp:12045 a developer added it for that case a month ago).

Steps to Reproduce

T0 G28 or G28 X T1

In config file have offsets set:

define HOTEND_OFFSET_X {0.0, 35.00}

Expected behavior: Extruder to not move on the toolchange

Actual behavior: hits X min hardstop

Additional Information

Debug Logs of the sequence:

> **SENT: G28**
> READ: >>> G28
> READ: Machine Type: Cartesian
> READ: Probe: NONE
> READ: echo:Active Extruder: 0
> Active Extruder: 0
> READ:   current_position=(0.00, 0.00, 0.00) : setup_for_endstop_or_probe_move
> READ: > endstops.enable(true)
> READ: Raise Z (before homing) to 5.00
> READ: >>> do_blocking_move_to(0.00, 0.00, 5.00)
> READ: <<< do_blocking_move_to
> READ: >>> homeaxis(X)
> READ: Home 1 Fast:
> READ: >>> do_homing_move(X, -360.00, [50.00])
> READ:   current_position=(0.00, 0.00, 5.00) : sync_plan_position
> READ: echo:busy: processing
> busy: processing
> READ: <<< do_homing_move(X)
> READ: Move Away:
> READ: >>> do_homing_move(X, 5.00, [50.00])
> READ:   current_position=(0.00, 0.00, 5.00) : sync_plan_position
> READ: <<< do_homing_move(X)
> READ: Home 2 Slow:
> READ: >>> do_homing_move(X, -10.00, 25.00)
> READ:   current_position=(0.00, 0.00, 5.00) : sync_plan_position
> READ: <<< do_homing_move(X)
> READ: >>> set_axis_is_at_home(X)
> READ: For X axis:
> READ:  home_offset = 0.00
> READ:  position_shift = 0.00
> READ:  soft_endstop_min = 0.00
> READ:  soft_endstop_max = 240.00
> READ: > home_offset[X] = 0.00
> READ:   current_position=(0.00, 0.00, 5.00) : 
> READ: <<< set_axis_is_at_home(X)
> READ:   current_position=(0.00, 0.00, 5.00) : sync_plan_position
> READ:   current_position=(0.00, 0.00, 5.00) : > AFTER set_axis_is_at_home
> READ: <<< homeaxis(X)
> READ:   current_position=(0.00, 0.00, 5.00) : sync_plan_position
> READ:   current_position=(0.00, 0.00, 5.00) : clean_up_after_endstop_or_probe_move
> READ: echo:Active Extruder: 0
> Active Extruder: 0
> READ: X:0.00 Y:0.00 Z:5.00 E:0.00 Count X:0 Y:0 Z:8000
> READ: <<< G28
> READ: ok
> **SENT: T1**
> READ: >>> gcode_T(1)
> READ:   current_position=(0.00, 0.00, 5.00) : 
> READ: Offset Tool XY by { 35.00, 0.00 }
> READ:   current_position=(35.00, 0.00, 5.00) : sync_plan_position
> READ:   destination=(0.00, 0.00, 5.00) : Move back
> READ: >>> do_blocking_move_to(0.00, 0.00, 5.00)
> READ: <<< do_blocking_move_to
> READ: echo:Active Extruder: 1
> Active Extruder: 1
> READ:   current_position=(0.00, 0.00, 5.00) : AFTER
> READ: <<< gcode_T
> READ: ok
[ConfigurationFiles.zip](https://github.com/MarlinFirmware/Marlin/files/2386922/ConfigurationFiles.zip)

sdtom commented 6 years ago

Video of G28 X then T1 https://youtu.be/MuU5E3V7TJA

thinkyhead commented 6 years ago

Thanks! This is a known issue. It might have been patched in the bugfix-1.1.x branch already. We'll follow up if the issue still exists there. For the time being, you may use T1 S1 to prevent the crash.

aharshac commented 6 years ago

Temporary fix.

Change inline void gcode_T(const uint8_t tmp_extruder) in Marlin_main.cpp

Replace

(tmp_extruder == active_extruder) || parser.boolval('S')

with

(tmp_extruder == active_extruder) || parser.boolval('S')
  || !all_axes_homed() || (tmp_extruder != 0 && current_position[X_AXIS] < X_MIN_POS)

Corresponding printer config:

#define USE_XMIN_PLUG
#define X_HOME_DIR  -1

#define X_MIN_POS 0
#define MANUAL_X_HOME_POS -40

// Tool 0 (left), Tool 1 (right)
#define SWITCHING_NOZZLE
#define SWITCHING_NOZZLE_SERVO_NR 0
#define SWITCHING_NOZZLE_SERVO_ANGLES { 0, 180 }   // Angles for E0, E1

#define HOTEND_OFFSET_X {0.0, 36.0}

InsanityAutomation commented 6 years ago

We can probably use the tool offsets to do the same thing here as I did for the idex setup.

thinkyhead commented 6 years ago

Ultimately, what is needed is logic added to the tool_change function to watch out for a tool change movement that would move the nozzle past MIN_X_POS or MAX_X_POS (and should check Y too), and make sure the move is constrained, also updating the current_position to wherever it ends up. (As opposed to when there is enough space, such that the new current_position corresponds exactly to the old one.)

Maxwellfire commented 5 years ago

This issue appears to be more than just on the tool change move itself.

As you said in 2016 here: https://github.com/MarlinFirmware/Marlin/issues/3823#issuecomment-221117295

The code appears to be doing many of the same things as a G92, but not actually updating the software limits.

I expect to do the following in 2 cases with:

Normal X axis (Not dual)
No bed leveling enabled
Software min endstops enabled and set to x = 0
Home position offset set to 0
Tool 1 offset x set to 18mm

Case 1 expected:

T0
G28 X
M114; Should report that our current position is x = 0
M211; Should report that soft limits are at x = 0
T1 S1; Doesn't move but changes current coordinate;
M114; Should report that our current position is x = 18
M211; Should report that soft limits are now at x = 18
G0 X0; Should ignore this command since we are at axis soft limit

Case 1 Actual:

T0
G28 X
M114; Reports that our current position is x = 0
M211; Reports that soft limits are at x = 0
T1 S1; Doesn't move but changes current coordinate;
M114; Reports that our current position is x = 18. GOOD
M211; Reports that soft limits are still now at x = 0. BAD
G0 X0; Slams axis into endstop since soft limits aren't updated. BAD

Case 2 actual and expected:

T0
G28 X
M114; Reports that our current position is x = 0
M211; Reports that soft limits are at x = 0
G92 X18; Doesn't move but changes current coordinate;
M114; Reports that our current position is x = 18
M211; Reports that soft limits are still now at x = 18
G0 X0;  Ignores this command since we are at axis soft limit

I'm not experienced enough with the codebase to determine what should be changed in tool_change() or Gcode_T(). I just wanted to point out that the issue appears to be more than just on the initial toolchange move. This seems like exactly what was supposed to be fixed by that earlier pull request. Is it possible that a later commit regressed on this issue?

I hope this helps. ~Max

boelle commented 5 years ago

@sdtom is this still an issue when using latest bugfix 2.0?

sdtom commented 5 years ago

have not tried but can this weekend. my workaround which is having slicer prepend an extra tool Change to the one that will be used first prior to homing has worked pretty well so far. will get back to you soon

boelle commented 5 years ago

remember to use bugfix 2.0 when testing :-D

sdtom commented 5 years ago

this is the first time using bugfix 2.0.x branch. after migrating my printer config over, I am having a 'worse' issue at about the exact point. Z access, immediately after homing Z successfully (around when tool change happens), the Z drive just runaway drives itself into the bed. So the Z goes down, touches off endstop, goes back up, goes down again at the slower speed.. after it stops on endstop momentarily / completing home operation.. just starts going down further. This is approx where the tool change happens in my gcode, i haven't nailed the tool change down as 100% the operation that makes it go haywire.

Reverting to old firmware w/ same gcode and everything is fine so not printer. I've kind of run out of time with looking at this today, but.. updated anyway

boelle commented 5 years ago

when time allows maybe attach the 2 config files as a zip file, it might be that @thinkyhead can use it and figure where things go wrong

thinkyhead commented 5 years ago

Whenever there are homing or leveling issues, we now ask everyone to follow a standard procedure to gather more information so that we're not just taking stabs in the dark. Here is the boilerplate:

Download Marlin bugfix-2.0.x to test with the latest code.
Enable DEBUG_LEVELING_FEATURE and M114_DETAIL and re-flash the firmware.
Connect to your printer from host software such as Cura, Printrun or Repetier Host.
Issue the command M111 S247 to enable maximum logging.
Perform a G28 to do your standard homing procedure.
Copy the log output into a .TXT file and attach it to your next reply.

From these logs we should hopefully get a better idea of what's going on with your machine.

sdtom commented 5 years ago

Fixed my problem, wasn't homing.. it was tool change.. I had transferred my hotend_offset_x as hotend_offset_Z--- user error :)

However, reporting on to this original error, it is still there.. no change in behavior in this sequence: Tested bugfix-2.0.x @ d71dc5 (latest per this morning)

Given in Configuration: #define HOTEND_OFFSET_X {0.0, 34.00} // (mm) relative X-offset for each nozzle

This G-Code:

T0
G28
T1

The T1 rams into the X min hardstop trying to shift for the tool change / ignoring that it's already at x pos 0 and shouldn't move any farther over

aharshac commented 5 years ago

I had this problem on a dual nozzle machine running a fork of Marlin 1.1.9 #7b594ee.

Attempting a tool change at home position (XMIN, YMAX, ZMAX) would result in the carriage crashing into the XMIN and ZMAX endstops.

Here's how I fixed the crashes.

Printer config:

Endstops USE_XMIN_PLUG USE_YMAX_PLUG USE_ZMAX_PLUG Home Offsets MANUAL_X_HOME_POS -40 MANUAL_Y_HOME_POS 395 MANUAL_Z_HOME_POS 400 X_MIN_POS 0 Y_MIN_POS 0 Z_MIN_POS 0 Hotend offsets HOTEND_OFFSET_X {0.0, 36.0} HOTEND_OFFSET_Y {0.0, 0.0} HOTEND_OFFSET_Z {0.0, -4.0} Dual nozzle configuration SWITCHING_NOZZLE SWITCHING_NOZZLE_SERVO_NR 0 SWITCHING_NOZZLE_SERVO_ANGLES { 0, 180 }

Modify void tool_change() in Marlin_main.cpp

void tool_change(const uint8_t tmp_extruder, const float fr_mm_s/*=0.0*/, bool no_move/*=false*/) {
  planner.synchronize();

  #if HAS_LEVELING
    // Set current position to the physical position
    const bool leveling_was_active = planner.leveling_active;
    set_bed_leveling_enabled(false);
  #endif

  #if ENABLED(MIXING_EXTRUDER) && MIXING_VIRTUAL_TOOLS > 1

    mixing_tool_change(tmp_extruder);

  #else // !MIXING_EXTRUDER || MIXING_VIRTUAL_TOOLS <= 1

    if (tmp_extruder >= EXTRUDERS)
      return invalid_extruder_error(tmp_extruder);

    #if HOTENDS > 1

      const float old_feedrate_mm_s = fr_mm_s > 0.0 ? fr_mm_s : feedrate_mm_s;

      feedrate_mm_s = fr_mm_s > 0.0 ? fr_mm_s : XY_PROBE_FEEDRATE_MM_S;

      if (tmp_extruder != active_extruder) {
        if (!no_move && axis_unhomed_error()) {
          no_move = true;
          #if ENABLED(DEBUG_LEVELING_FEATURE)
            if (DEBUGGING(LEVELING)) SERIAL_ECHOLNPGM("No move on toolchange");
          #endif
        }

        #if ENABLED(DUAL_X_CARRIAGE)

          #if HAS_SOFTWARE_ENDSTOPS
            // Update the X software endstops early
            active_extruder = tmp_extruder;
            update_software_endstops(X_AXIS);
            active_extruder = !tmp_extruder;
          #endif

          // Don't move the new extruder out of bounds
          if (!WITHIN(current_position[X_AXIS], soft_endstop_min[X_AXIS], soft_endstop_max[X_AXIS]))
            no_move = true;

          if (!no_move) set_destination_from_current();
          dualx_tool_change(tmp_extruder, no_move); // Can modify no_move

        #else // !DUAL_X_CARRIAGE
          <ADD>
          const float xdiff = hotend_offset[X_AXIS][tmp_extruder] - hotend_offset[X_AXIS][active_extruder],
                      ydiff = hotend_offset[Y_AXIS][tmp_extruder] - hotend_offset[Y_AXIS][active_extruder];

          if (current_position[X_AXIS] - xdiff <= MANUAL_X_HOME_POS)
            no_move = true;

          bool stop_z_crash = false;
          #if ENABLED(SWITCHING_NOZZLE)
            // Always raise by at least 1 to avoid workpiece
            const float zdiff = hotend_offset[Z_AXIS][active_extruder] - hotend_offset[Z_AXIS][tmp_extruder];
            float tmp_z_max = Z_MAX_POS - (zdiff < 0 ? -1 * zdiff : zdiff) - 1;
            // SERIAL_ECHOLNPAIR("tmp_z_max ", tmp_z_max);
            stop_z_crash = current_position[Z_AXIS] > tmp_z_max;
            // SERIAL_ECHOLNPAIR("stop_z_crash ", stop_z_crash);
          #endif
          </ADD>

          set_destination_from_current();
          #if ENABLED(PARKING_EXTRUDER) // Dual Parking extruder
            parking_extruder_tool_change(tmp_extruder, no_move);
          #endif

          #if ENABLED(SWITCHING_NOZZLE)
            // Always raise by at least 1 to avoid workpiece
            <DIFF>
            // const float zdiff = hotend_offset[Z_AXIS][active_extruder] - hotend_offset[Z_AXIS][tmp_extruder];
            if (!stop_z_crash) {
              current_position[Z_AXIS] += (zdiff > 0.0 ? zdiff : 0.0) + 1;
              planner.buffer_line_kinematic(current_position, planner.max_feedrate_mm_s[Z_AXIS], active_extruder);
            }
            </DIFF>
            move_nozzle_servo(tmp_extruder);
          #endif

          <RM>
          // const float xdiff = hotend_offset[X_AXIS][tmp_extruder] - hotend_offset[X_AXIS][active_extruder],
          //             ydiff = hotend_offset[Y_AXIS][tmp_extruder] - hotend_offset[Y_AXIS][active_extruder];
          </RM>

          #if ENABLED(DEBUG_LEVELING_FEATURE)
            if (DEBUGGING(LEVELING)) {
              SERIAL_ECHOPAIR("Offset Tool XY by { ", xdiff);
              SERIAL_ECHOPAIR(", ", ydiff);
              SERIAL_ECHOLNPGM(" }");
            }
          #endif

          // The newly-selected extruder XY is actually at...
          current_position[X_AXIS] += xdiff;
          current_position[Y_AXIS] += ydiff;

          // Set the new active extruder
          active_extruder = tmp_extruder;

        #endif // !DUAL_X_CARRIAGE

        #if ENABLED(SWITCHING_NOZZLE)
          // The newly-selected extruder Z is actually at...
          <DIFF>
          if (!stop_z_crash) 
            current_position[Z_AXIS] -= zdiff;
          </DIFF>
        #endif

        // Tell the planner the new "current position"
        SYNC_PLAN_POSITION_KINEMATIC();

        #if ENABLED(DELTA)
          //LOOP_XYZ(i) update_software_endstops(i); // or modify the constrain function
          const bool safe_to_move = current_position[Z_AXIS] < delta_clip_start_height - 1;
        #else
          constexpr bool safe_to_move = true;
        #endif

        // Raise, move, and lower again
        if (safe_to_move && !no_move && IsRunning()) {
          #if DISABLED(SWITCHING_NOZZLE)
            // Do a small lift to avoid the workpiece in the move back (below)
            current_position[Z_AXIS] += 1.0;
            planner.buffer_line_kinematic(current_position, planner.max_feedrate_mm_s[Z_AXIS], active_extruder);
          #endif
          #if ENABLED(DEBUG_LEVELING_FEATURE)
            if (DEBUGGING(LEVELING)) DEBUG_POS("Move back", destination);
          #endif
          // Move back to the original (or tweaked) position
          do_blocking_move_to(destination[X_AXIS], destination[Y_AXIS], destination[Z_AXIS]);
          #if ENABLED(DUAL_X_CARRIAGE)
            active_extruder_parked = false;
          #endif
        }
        #if ENABLED(SWITCHING_NOZZLE)
          else {
            // Move back down. (Including when the new tool is higher.)
            <DIFF>
            if (!stop_z_crash)
              do_blocking_move_to_z(destination[Z_AXIS], planner.max_feedrate_mm_s[Z_AXIS]);
            </DIFF>
          }
        #endif
      } // (tmp_extruder != active_extruder)

      planner.synchronize();

      #if ENABLED(EXT_SOLENOID) && !ENABLED(PARKING_EXTRUDER)
        disable_all_solenoids();
        enable_solenoid_on_active_extruder();
      #endif

      feedrate_mm_s = old_feedrate_mm_s;

      #if HAS_SOFTWARE_ENDSTOPS && ENABLED(DUAL_X_CARRIAGE)
        update_software_endstops(X_AXIS);
      #endif

    #else // HOTENDS <= 1

      UNUSED(fr_mm_s);
      UNUSED(no_move);

      #if ENABLED(MK2_MULTIPLEXER)
        if (tmp_extruder >= E_STEPPERS)
          return invalid_extruder_error(tmp_extruder);

        select_multiplexed_stepper(tmp_extruder);
      #endif

      // Set the new active extruder
      active_extruder = tmp_extruder;

    #endif // HOTENDS <= 1

    #if DO_SWITCH_EXTRUDER
      planner.synchronize();
      move_extruder_servo(active_extruder);
    #endif

    #if HAS_FANMUX
      fanmux_switch(active_extruder);
    #endif

    #if HAS_LEVELING
      // Restore leveling to re-establish the logical position
      set_bed_leveling_enabled(leveling_was_active);
    #endif

    SERIAL_ECHO_START();
    SERIAL_ECHOLNPAIR(MSG_ACTIVE_EXTRUDER, (int)active_extruder);

  #endif // !MIXING_EXTRUDER || MIXING_VIRTUAL_TOOLS <= 1
}

InsanityAutomation commented 5 years ago

Some degree of the above is needed. It's on my list to get to at some point.... That list is way too long lol thinkyhead just finished one of the things I had planned to get to! Day job is too busy ATM....

aharshac commented 5 years ago

@thinkyhead debug log for tool change

Recv: <<< G28
Recv: ok
[...]
Send: T1
Recv: echo:T1
Recv: >>> T(1)
Recv:   current_position=(-40.00, 395.00, 400.00) : BEFORE
Recv: Offset Tool XY by { 36.00, 0.00, -4.00 }
Recv:   current_position=(-4.00, 395.00, 396.00) : sync_plan_position
Recv:   destination=(-40.00, 395.00, 400.00) : Move back
Recv: >>> do_blocking_move_to(-40.00, 395.00, 400.00)
Recv: <<< do_blocking_move_to
[...]
Recv: echo:Active Extruder: 1
Recv:   current_position=(-40.00, 395.00, 400.00) : AFTER
Recv: <<< T()
Recv: ok
[...]
Send: T0
Recv: echo:T0
Recv: >>> T(0)
Recv:   current_position=(-40.00, 395.00, 400.00) : BEFORE
Recv: Offset Tool XY by { -36.00, 0.00, 4.00 }
Recv:   current_position=(-76.00, 395.00, 404.00) : sync_plan_position
Recv:   destination=(-40.00, 395.00, 400.00) : Move back
Recv: >>> do_blocking_move_to(-40.00, 395.00, 400.00)
Recv: <<< do_blocking_move_to
[...]
Recv: echo:Active Extruder: 0
Recv:   current_position=(-40.00, 395.00, 400.00) : AFTER
Recv: <<< T()
Recv: ok

InsanityAutomation commented 5 years ago

FYI, one fix went in at MRRF and another PR is open #14117 with a few more tweaks around IDEX. Can you test with that PR and let me know if you still have an issue?

Thanks

boelle commented 5 years ago

@thinkyhead Another one that needs to be closed. Sorry, I got bored.

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

MarlinFirmware / Marlin

G28 Homing then change to T1 yields crash on end stop #11845

Description

3823. There is no clamping on soft stops for a non dual carriage axis (I can see in Marlin_main.cpp:12045 a developer added it for that case a month ago).

Steps to Reproduce

define HOTEND_OFFSET_X {0.0, 35.00}

Additional Information