[BUG] `TMCMarlin::set_pwm_thrs()` sets `0` if driver is not powered by VMOT #20397

rubienr · 2020-12-07T09:27:02Z

Bug Description

When TMC2130 stepper driver is not powered by VMOT (i.e. PSU_CONTROL feature is enabled and PSU is off),
TMCMarlin::set_pwm_thrs(const uint32_t) will call TMC::TPWMTHRS(0) because this->microsteps() returns the readout of the unpowered driver's internal processor, not a shadowed register (see tmc_util.h:116 and all subsequent implementations of TMCMarlin::set_pwm_thrs(const uint32_t thrs)).

The rough call tree:

TMCMarlin::set_pwm_thrs(const uint32_t)
  -> uint16_t TMCStepper::microsteps() 
    -> uint8_t TMC2130Stepper::mres() { CHOPCONF_t r{0}; r.sr = CHOPCONF(); return r.mres; }
      -> uint32_t TMC2130Stepper::CHOPCONF() { return read(CHOPCONF_register.address); } // attempts to read from the driver
  -> TMC::TPWMTHRS(_tmc_thrs(this->microsteps() /* == 0 if unpowered */, thrs, planner.settings.axis_steps_per_mm[AXIS_ID]));
    -> constexpr uint16_t _tmc_thrs(const uint16_t msteps /* == 0 */, const uint32_t thrs, const uint32_t spmm)
         {return 12650000UL*msteps /* msteps == 0 */ /(256*thrs*spmm);} // returns 0

TMC::TPWMTHRS(uint32_t) will cache and send the threshold to driver. The next read with TMCStepper::TPWMTHRS() will return the cached value (0 in the unpowered case).

Configuration Files

Enable #define PSU_CONTROL, enable at least one TMC driver by #define *_DRIVER_TYPE TMC2130 and #define HYBRID_THRESHOLD.

configuration-issue-20397-2.0.7.2.tar.gz

Marlin Version: 2.0.x branch as of eb254ef (7th Dec. 2020)

Steps to Reproduce

start Marlin (assuming a non 0 hybrid threshold value is stored in flash)
make sure PSU is off
trigger TMCMarlin::set_pwm_thrs(const uint32_t) for example by M913 X100
enable PSU M80
re-initialize stepper drivers with current settings M122 I S0
M913 will report 0

Expected behaviour:
1.-3.
4. enable PSU M80
5. M913 will report the hybrid threshold as stored in flash

or

1.-3.
4. enable PSU M80
5. re-initialize stepper drivers with current settings M122 I S0
6. M913 will report the hybrid threshold as stored in flash

Actual behaviour:
1.-5.
6. M913 will report 0

Probably Related

#19364 (PR: Use stored values for TMC StealthChop)
#19313 (Issue: [BUG] M502/M500 when TMC drivers do not have VMOT results in incorrect values being stored in EEPROM)

A Detailed Example

; ---------------------------

; marlin freshly started

; ensure correct hybrid threshold values are stored in flash
M80
M502
M500

; ---------------------------

; re-boot Marlin
; connecting with OctoPrint

Send: M115 ; firmware info (sent by OctoPrint automatically)
Recv: FIRMWARE_NAME:Marlin 2.0.7.2 (Dec  7 2020 15:35:21) SOURCE_CODE_URL:https://github.com/MarlinFirmware/Marlin PROTOCOL_VERSION:1.0 MACHINE_TYPE:Ender-5 Plus - modified EXTRUDER_COUNT:2 UUID:cede2a2f-41a2-4748-9b12-c55c62f367ff
Recv: Cap:SERIAL_XON_XOFF:0
Recv: Cap:BINARY_FILE_TRANSFER:0
Recv: Cap:EEPROM:1
...

; ---------------------------

Send: M504 ; validate EEPROM settings (sent by OctoPrint automatically)
Recv: DIGIPOTS Loading
Recv: DIGIPOTS Loaded
Recv: echo:EEPROM OK
Recv: ok P63 B15

; ---------------------------

Send: M501 ; restore settings from EEPROM (sent by OctoPrint automatically)
Recv: DIGIPOTS Loading
Recv: DIGIPOTS Loaded

  ; with debug output I see that
  ; * tmc_init() is called for each axis with default values form Confiugration(_adv).h
  ;   example of local variables in set_pwm_thrs() for an arbitrary axis:
  ;     this->microsteps()==0 // readout is 0 because TMC is not supplied by VMOT
  ;     thrs==50
  ;
  ; * MarlinSettings::_load() refreshes the threshold and calls set_pwm_thrs()
  ;   example of local variables  in set_pwm_thrs() for an arbitrary axis:
  ;     this->microsteps()==0  // readout is 0 because TMC is not supplied by VMOT
  ;     thrs==50
  ;
  ; with microsteps==0 the conversion _tmc_thrs() returns 0 and thus TMC::TPWMTHRS() stores 0 to the shadow register in the TMC library

Recv: DIGIPOTS Loading
Recv: DIGIPOTS Loaded
Recv: echo:V82 stored settings retrieved (774 bytes; crc 26328)
Recv: Unified Bed Leveling System v1.01 inactive
...
Recv: echo:; Z-Probe Offset (mm):
Recv: echo:  M851 X-26.50 Y47.50 Z-2.06
Recv: echo:; Stepper driver current:
Recv: echo:  M906 X1100 Y1000 Z800
Recv: echo:  M906 I1 Z800
Recv: echo:  M906 T0 E800
Recv: 
Recv: echo:; Hybrid Threshold:

  ; with debug output I see that for M913 get_pwm_thrs() is called 
  ; * local variables in get_pwm_thrs() are always as follows:
  ;     this->microsteps()==0 // because TMC is not supplied by VMOT
  ;     TPWMTHRS()==0         // bcause previously set to 0
  
Recv: echo:  M913 X0 Y0 Z0
Recv: echo:  M913 I1 Z0
Recv: echo:  M913 T0 E0
Recv: 
Recv: echo:; StallGuard threshold:
Recv: echo:  M914 X3 Y8
Recv: echo:; Driver stepping mode:
Recv: echo:  M569 S1 X Y Z
Recv: echo:  M569 S1 I1 Z
Recv: echo:  M569 S1 T0 E
...

; ---------------------------
; sending commands manually

; ---------------------------

Send: M913 ; report hybrid threshold speed
Recv: X stealthChop max speed: 0
Recv: Y stealthChop max speed: 0
Recv: Z stealthChop max speed: 0
Recv: Z2 stealthChop max speed: 0
Recv: E stealthChop max speed: 0

; ---------------------------

Send: M80 ; turn power supply on (VMOT)

; ---------------------------

Send: M913 ; report hybrid threshold speed
Recv: X stealthChop max speed: 0
Recv: Y stealthChop max speed: 0
Recv: Z stealthChop max speed: 0
Recv: Z2 stealthChop max speed: 0
Recv: E stealthChop max speed: 0

; ---------------------------

Send: M122 I S0 ; re-initialize stepper drivers with current settings
...
Recv: PWM thresh.	0	0	0	0	0
Recv: [mm/s]
Recv: -	-	-	-	-
...
Recv: Driver registers:
Recv: 		X	0x80:12:00:00
Recv: 		Y	0x80:11:00:00
Recv: 		Z	0x80:19:00:00
Recv: 		Z2	0x80:19:00:00
Recv: 		E	0x80:19:00:00
Recv: 
Recv: 
Recv: Testing X connection... OK
Recv: Testing Y connection... OK
Recv: Testing Z connection... OK
Recv: Testing Z2 connection... OK
Recv: Testing E connection... OK


; ---------------------------

Send: M913 ; report hybrid threshold speed
Recv: X stealthChop max speed: 0
Recv: Y stealthChop max speed: 0
Recv: Z stealthChop max speed: 0
Recv: Z2 stealthChop max speed: 0
Recv: E stealthChop max speed: 0

; ---------------------------

Send: M501 ; restore settings from EEPROM
Recv: DIGIPOTS Loading
Recv: DIGIPOTS Loaded

  ; with debug output I see that
  ; * tmc_init() is called for each axis with default values form Confiugration(_adv).h
  ;   example of local variables in set_pwm_thrs() for an arbitrary axis:
  ;     this->microsteps()==8 // corret readout
  ;     thrs==50
  ;
  ; * MarlinSettings::_load() refreshes the threshold and calls set_pwm_thrs()
  ;   example of local variables  in set_pwm_thrs()  for an arbitrary axis:
  ;     this->microsteps()==8  // correct readout
  ;     thrs==50

Recv: DIGIPOTS Loading
Recv: DIGIPOTS Loaded
Recv: echo:V82 stored settings retrieved (774 bytes; crc 26328)
...
Recv: echo:; Hybrid Threshold:

  ; with debug output I see that for M913 get_pwm_thrs() is called 
  ; * example of local variables in get_pwm_thrs() for an axis:
  ;     this->microsteps()==8 // correct readout
  ;     TPWMTHRS()==197       // reasonable value

Recv: echo:  M913 X50 Y50 Z20  ; expected values
Recv: echo:  M913 I1 Z20
Recv: echo:  M913 T0 E101
Recv: 
Recv: echo:; StallGuard threshold:
Recv: echo:  M914 X3 Y8
Recv: echo:; Driver stepping mode:
Recv: echo:  M569 S1 X Y Z
Recv: echo:  M569 S1 I1 Z
Recv: echo:  M569 S1 T0 E
Recv: echo:; Linear Advance:
Recv: echo:  M900 T0 K0.17
Recv: echo:  M900 T1 K0.17
Recv: echo:; Filament load/unload lengths:
Recv: echo:  M603 T0 L65.00 U50.00
Recv: echo:  M603 T1 L65.00 U50.00
Recv: echo:; Tool-changing:
Recv: echo: Z2.00
Recv: echo:; Filament runout sensor:
Recv: echo:  M412 S1

; ---------------------------

Send: M913
Recv: X stealthChop max speed: 50
Recv: Y stealthChop max speed: 50
Recv: Z stealthChop max speed: 20
Recv: Z2 stealthChop max speed: 20
Recv: E stealthChop max speed: 101

The text was updated successfully, but these errors were encountered:

ManuelMcLure · 2020-12-07T17:29:03Z

I've been discussing this on Discord with @rubienr and agree that this is a bug, This was something that slipped through the cracks when I looked at #19364 - the hybrid threshold is stored in a shadow register in TMCStepper, but the micro steps are not and those are used to initialize the shadow register hence the bug.

ManuelMcLure · 2020-12-07T19:04:10Z

The micro step setting is indirectly shadowed on the TMCStepper side as part of CHOPCONF for re-initializing the drivers, but reading it always goes to the driver, which is the design pattern for TMCStepper. On the Marlin side we don't keep a shadow version of the micro step setting, probably because it can only be set in Configuration_adv.h (no gcode or EEPROM storage) so it was not considered worth the RAM.
Unfortunately at the point where we update the hybrid threshold we don't have access to the original value for micro steps that was passed in to tmc_init() so it looks like to fix this we will have to add extra storage on the Marlin side.

rubienr · 2020-12-07T22:27:14Z

Tested with bugfix-2.0.x as of 9b82a5c and observed the same behaviour as described above.

Configuration Files

configuration-issue-20397-bugfix-2.0.x.tar.gz

ManuelMcLure · 2020-12-07T23:44:22Z

I'm pretty sure this is not specific to the 2130 drivers and will occur with all TMC drivers.

ClockeNessMnstr · 2020-12-27T22:39:37Z

Not sure if this is part of the same thing or maybe expected behavior for some reason but I was also getting '0' set by "M913 E5" for my TMC2209 on my extruder which had microstepping disabled / = 0.
it worked when I set microsteps to 4. I was able to set M913 E5.
Other motors worked set at 16.

Seems to set '0' if microsteps is set to 0 for the driver. I don't quite understand the firmware enough to know if this is the same behavior mentioned regarding microsteps on the driver.

github-actions · 2021-01-27T02:09:27Z

This issue has had no activity in the last 30 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 7 days.

rubienr · 2021-01-29T16:38:41Z

If needed I can offer my printer for remote debugging/flashing.

github-actions · 2021-03-07T02:05:21Z

This issue has had no activity in the last 30 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 7 days.

github-actions · 2021-05-16T02:09:51Z

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.

rubienr changed the title ~~[DRAFT] [BUG] TMCMarlin::set_pwm_thrs() sets 0 if driver is not powered by VMOT~~ [BUG] TMCMarlin::set_pwm_thrs() sets 0 if driver is not powered by VMOT Dec 7, 2020

sjasonsmith added the F: Trinamic label Dec 24, 2020

github-actions bot added the stale-closing-soon label Jan 27, 2021

github-actions bot removed the stale-closing-soon label Jan 30, 2021

github-actions bot added the stale-closing-soon label Mar 7, 2021

github-actions bot closed this as completed Mar 17, 2021

github-actions bot locked and limited conversation to collaborators May 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] `TMCMarlin::set_pwm_thrs()` sets `0` if driver is not powered by VMOT #20397

[BUG] `TMCMarlin::set_pwm_thrs()` sets `0` if driver is not powered by VMOT #20397

rubienr commented Dec 7, 2020 •

edited

Loading

ManuelMcLure commented Dec 7, 2020

ManuelMcLure commented Dec 7, 2020

rubienr commented Dec 7, 2020 •

edited

Loading

ManuelMcLure commented Dec 7, 2020

ClockeNessMnstr commented Dec 27, 2020 •

edited

Loading

github-actions bot commented Jan 27, 2021

rubienr commented Jan 29, 2021

github-actions bot commented Mar 7, 2021

github-actions bot commented May 16, 2021

[BUG] TMCMarlin::set_pwm_thrs() sets 0 if driver is not powered by VMOT #20397

[BUG] TMCMarlin::set_pwm_thrs() sets 0 if driver is not powered by VMOT #20397

Comments

rubienr commented Dec 7, 2020 • edited Loading

Bug Description

Configuration Files

Steps to Reproduce

Probably Related

A Detailed Example

ManuelMcLure commented Dec 7, 2020

ManuelMcLure commented Dec 7, 2020

rubienr commented Dec 7, 2020 • edited Loading

Configuration Files

ManuelMcLure commented Dec 7, 2020

ClockeNessMnstr commented Dec 27, 2020 • edited Loading

github-actions bot commented Jan 27, 2021

rubienr commented Jan 29, 2021

github-actions bot commented Mar 7, 2021

github-actions bot commented May 16, 2021

[BUG] `TMCMarlin::set_pwm_thrs()` sets `0` if driver is not powered by VMOT #20397

[BUG] `TMCMarlin::set_pwm_thrs()` sets `0` if driver is not powered by VMOT #20397

rubienr commented Dec 7, 2020 •

edited

Loading

rubienr commented Dec 7, 2020 •

edited

Loading

ClockeNessMnstr commented Dec 27, 2020 •

edited

Loading