MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.14k stars 19.2k forks source link

[BUG] TMC2209 Sensorless homing fails if PSU_DEFAULT_OFF is enabled #21527

Closed Pyro-Fox closed 2 years ago

Pyro-Fox commented 3 years ago

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

Yes, and the problem still exists.

Bug Description

When I uncomment PSU_DEFAULT_OFF in my configuration, sensorless homing is now failing. Without changing ANYTHING else, if I comment it out again, my X and Y home correctly.

Note 1: Since the default behavior is to Home X before Y, I enabled HOME_Y_BEFORE_X as a test and confirmed both axes fail at sensorless homing in this configuration. Note 2: Since I'm using AUTO_POWER_CONTROL, I tried this both with and without sending an M80 first. The result was the same.

Bug Timeline

New

Expected behavior

I expect my axes to move to the physical limit of the rail.

Actual behavior

It crashes and starts BRRRRRRRRR'ing until the printer is reset.

Steps to Reproduce

  1. Upload firmware.bin with PSU_DEFAULT_OFF enabled to MicroSD and insert into SKR 1.3
  2. Press reset button to complete the upgrade and wait for LCD to load
  3. [Optional] Select Switch Power On from the LCD menu
  4. Select Auto Home from the LCD menu

When PSU_DEFAULT_OFF is enabled, the axis will crash. When it's disabled, it hits the end softly as expected.

Version of Marlin Firmware

built from bugfix-2.0.x (commit dd76a50)

Printer model

Custom FolgerTech FT-5

Electronics

BTT SKR 1.3 w/TMC2209

Add-ons

No response

Your Slicer

No response

Host Software

No response

All modified files: Marlin config for PSU_DEFAULT_OFF issue.zip

EEPROM:

Send: M503
Recv: echo:  G21    ; Units in mm (mm)
Recv: echo:  M149 C ; Units in Celsius
Recv: 
Recv: echo:; Filament settings: Disabled
Recv: echo:  M200 S0 D1.75
Recv: echo:; Steps per unit:
Recv: echo: M92 X80.00 Y80.00 Z400.00 E409.00
Recv: echo:; Maximum feedrates (units/s):
Recv: echo:  M203 X300.00 Y300.00 Z5.00 E25.00
Recv: echo:; Maximum Acceleration (units/s2):
Recv: echo:  M201 X3000.00 Y3000.00 Z100.00 E10000.00
Recv: echo:; Acceleration (units/s2): P<print_accel> R<retract_accel> T<travel_accel>
Recv: echo:  M204 P3000.00 R3000.00 T3000.00
Recv: echo:; Advanced: B<min_segment_time_us> S<min_feedrate> T<min_travel_feedrate> J<junc_dev>
Recv: echo:  M205 B20000.00 S0.00 T0.00 J0.01
Recv: echo:; Home offset:
Recv: echo:  M206 X0.00 Y0.00 Z0.00
Recv: echo:; Unified Bed Leveling:
Recv: echo:  M420 S0 Z10.00
Recv: 
Recv: Unified Bed Leveling System v1.01 inactive
Recv: 
Recv: echo:; Active Mesh Slot: -1
Recv: echo:; EEPROM can hold 7 meshes.
Recv: 
Recv: echo:; Material heatup parameters:
Recv: echo:  M145 S0 H180.00 B70.00 F0
Recv: echo:  M145 S1 H200.00 B80.00 F0
Recv: echo:; PID settings:
Recv: echo:  M301 P22.20 I1.08 D114.00
Recv: echo:  M304 P13.24 I0.76 D154.38
Recv: echo:; Z-Probe Offset (mm):
Recv: echo:  M851 X19.64 Y40.33 Z0.00
Recv: echo:; Stepper driver current:
Recv: echo:  M906 X800 Y800 Z800
Recv: echo:  M906 I1 Y800
Recv: echo:  M906 T0 E940
Recv: 
Recv: echo:; StallGuard threshold:
Recv: echo:  M914 X100 Y100
Recv: echo:  M914 I1 Y100
Recv: echo:; Driver stepping mode:
Recv: echo:  M569 S1 X Y Z
Recv: echo:  M569 S1 I1 Y
Recv: echo:  M569 S1 T0 E
Recv: echo:; Linear Advance:
Recv: echo:  M900 K0.22
Recv: ok
ellensp commented 3 years ago

@teemuatlut could you possibly take a look at this, since your the TMC driver expert.

Pyro-Fox commented 3 years ago

Wanted to add that I'm also seeing this happen if my printer power has been off for some time.

I wonder if there's a Trinamic check that's failing in the background 🤔? I just remembered that, when PSU_DEFAULT_OFF is enabled, the LCD would usually read "TMC CONNECTION ERROR".

I know this is grasping at straws, but I went looking for what generates that error and found test_tmc_connection() in tmc_util.cpp. Then, while checking all the references to that, I found this code in MarlinCore.cpp that looks loosely related to this issue (currently lines 1461 to 1463):

  #if HAS_TRINAMIC_CONFIG && DISABLED(PSU_DEFAULT_OFF)
    SETUP_RUN(test_tmc_connection(true, true, true, true));
  #endif
slowbro commented 3 years ago

When you say..

It crashes and starts BRRRRRRRRR'ing until the printer is reset.

Do you mean the axis crashes & starts making that sound from stalling, or marlin kills the printer & starts buzzing/beeping?

Edit: After looking around a bit, I'm mostly stumped. The only other things that call test_tmc_connection (seemingly the only place that triggers the TMC error) is in the M122 gcode. Related (enabled) configs are TMC_DEBUG and MONITOR_DRIVER_STATUS. Can you try disabling one or both of those and see if the issue persists? I doubt that would be it, as I don't see anything in the code calling GCodeSuite::M122 directly...

Pyro-Fox commented 3 years ago

When you say..

It crashes and starts BRRRRRRRRR'ing until the printer is reset.

Do you mean the axis crashes & starts making that sound from stalling, or marlin kills the printer & starts buzzing/beeping?

I meant the first axis will crash and keep stalling until the homing timeout kicks in and Marlin stops the printer.

There could be a bug in the StallGuard tech itself not sending the homing pin trigger when starting from on off state too, but I really don't know how to test for that.

Regarding turning off TMC_DEBUG, I'll try that tonight and report back.

slowbro commented 3 years ago

@Pyro-Fox following up - did you have time to test TMC_DEBUG? No worries if not, just running through my old notifications!

Pyro-Fox commented 3 years ago

@Pyro-Fox following up - did you have time to test TMC_DEBUG? No worries if not, just running through my old notifications!

I just confirmed that commenting out MONITOR_DRIVER_STATUS and TMC_DEBUG did not change the behavior at all.

slowbro commented 3 years ago

Alright, thanks for following up. I don't have the hardware available to test PSU control so I'm not entirely sure where to go from here.

I know this is grasping at straws, but I went looking for what generates that error and found test_tmc_connection() in tmc_util.cpp. Then, while checking all the references to that, I found this code in MarlinCore.cpp that looks loosely related to this issue (currently lines 1461 to 1463):

  #if HAS_TRINAMIC_CONFIG && DISABLED(PSU_DEFAULT_OFF)
    SETUP_RUN(test_tmc_connection(true, true, true, true));
  #endif

I think this may have something to do with it, since this is the first code that 'talks' to the TMC drivers on setup. If it's skipped, perhaps they are not enabled/started up when the psu-on command happens, leading to the communication error. Perhaps we can fudge it for now and try setting that when the psu-on command is sent. There's a couple things I want to try..

diff --git a/Marlin/src/feature/power.cpp b/Marlin/src/feature/power.cpp
index fb2f1312e0..0356b4830e 100644
--- a/Marlin/src/feature/power.cpp
+++ b/Marlin/src/feature/power.cpp
@@ -121,7 +121,10 @@ void Power::power_on() {
     PSU_PIN_ON();
     safe_delay(PSU_POWERUP_DELAY);
     restore_stepper_drivers();
-    TERN_(HAS_TRINAMIC_CONFIG, safe_delay(PSU_POWERUP_DELAY));
+    #if HAS_TRINAMIC_CONFIG && ENABLED(PSU_DEFAULT_OFF)
+      safe_delay(PSU_POWERUP_DELAY);
+      test_tmc_connection();
+    #endif
     #ifdef PSU_POWERUP_GCODE
       GcodeSuite::process_subcommands_now_P(PSTR(PSU_POWERUP_GCODE));
     #endif

and

diff --git a/Marlin/src/feature/power.cpp b/Marlin/src/feature/power.cpp
index fb2f1312e0..dc6ce92307 100644
--- a/Marlin/src/feature/power.cpp
+++ b/Marlin/src/feature/power.cpp
@@ -120,7 +120,7 @@ void Power::power_on() {
   if (!powersupply_on) {
     PSU_PIN_ON();
     safe_delay(PSU_POWERUP_DELAY);
-    restore_stepper_drivers();
+    reset_stepper_drivers();
     TERN_(HAS_TRINAMIC_CONFIG, safe_delay(PSU_POWERUP_DELAY));
     #ifdef PSU_POWERUP_GCODE
       GcodeSuite::process_subcommands_now_P(PSTR(PSU_POWERUP_GCODE));

Could you try both of those patches individually on your printer? I'd be interested in the serial output from the first one, too, as it looks like the test_tmc_connection() code was recently re-written to output information.

Pyro-Fox commented 3 years ago

Heads up. I promise I havent forgotten about this issue and am hoping to test your patch in the next couple days. I just have a couple prints I'm trying to finish first before I start playing with my firmware again.

slowbro commented 3 years ago

No worries, I totally understand that :) Whenever you get the chance, take your time!

shadowhackingcorp commented 3 years ago

Newbie to GitHub and ex-s/w engineer suffering with this. My setup starts with the PSU on and is fine until the PSU times out and goes off. When the PSU comes back up again the homing doesn't want to play.

Just had a look in trainamic.cpp and noticed that reset_trinamic_drivers() has a chunk at the end restoring the homing thresholds. Should this also be done in restore_trinamic_drivers() or should that be taken care of in the individual stepper.push()'s?

shadowhackingcorp commented 3 years ago

FWIW, I hived off the chunk I found in reset_trinamic_drivers (#if USE_SENSORLESS . . . #endif) into a local function and called it from both reset and restore and this solves it for me.

However, dunno if that's the "correct" solution or whether it needs applying to other drivers so daren't actually do more than stick my oar in here tentatively! And I think my copy of the code is probably an iteration or two old.

Cheers, Scott

slowbro commented 3 years ago

Thanks for the update. Yes, that seems like a reasonable change. The push() calls are re-setting all the defined settings for the drivers, but it doesn't seem to include sensorless parameters. Moving it to its own function seems good to me.

shadowhackingcorp commented 3 years ago

Go me! I've no idea how to go about changing stuff to be included in a release, so I can't do much at the mo. If I get time after work I'll post up my changes.

teemuatlut commented 3 years ago

https://github.com/teemuatlut/TMCStepper/commit/2e7624ed003aae66323a47e32ed731aa63683642 I added the missing calls. Let me know if it works and I can make a release tag.

thinkyhead commented 3 years ago

Thanks, Teemu…. I recall that we had added calls to make sure the TMC settings are restored when the PSU is turned on, so if the library is updated it should take care of the issue. I'll update the required version in the INI files once that is tagged. Thanks!

teemuatlut commented 3 years ago

I kinda had hoped to know first if the update messes up anything but I suppose it'll be fine. You can always blacklist a release 😄 You won't need to make any changes to the ini files as PIO should see it as a minor release and target it accordingly.

thinkyhead commented 2 years ago

@teemuatlut — We got all those changes done, right? I'm reviewing issue backlog for the first time in a long while, and I can't remember!

github-actions[bot] commented 2 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.