[BUG] TMC2130 Sensorless Homing stall sensitivity values get corrupted

[hitgub31231]

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

Yes, and the problem still exists.

Bug Description

After Reboot sensitivity values are overwritten with (in my case) Value "63"
M502 does load the correct values.
But changing them in the Menu and storing them in Eeprom ist not possible.
After every reset, Values have to be changed manually again.
Had no problems in 2.1.2

Bug Timeline

No response

Expected behavior

Persistent storage of sensorless Homing sensivity Values in Eeprom.

Actual behavior

Not being stored correctly

Steps to Reproduce

Change X and Y Values in Menu
M500
Reset Printer
Values are not the same as the stored ones

Version of Marlin Firmware

2.1.x bugfix

Printer model

Ender 3

Electronics

09 steppers, BTT SKR 1.4, TMC2130

LCD/Controller

Reprap discount full graphic smart controller

Other add-ons

BLTOUCH

Bed Leveling

UBL Bilinear mesh

Your Slicer

Other (explain below)

Host Software

OctoPrint

Don't forget to include

• A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

No response

[hitgub31231]

Configuration.zip

[hitgub31231]

It seems, the error does only occur with negative Values, e.g. -1

[rondlh]

I can confirm this bug, if I set my sensorless homing sensitivity for TMC5160 to a negative value and store the values (M500), then after a reboot the Bump sensitivity is set to 63 for all axis tested (X, Y and Y2).
A value of 0 and 1 is restored correctly, a value of -1 is restored to 63.

[ellensp]

@rondlh did you use lcd or gcode to set sensitivity? wondering if its just a lcd menu issue

[rondlh]

@rondlh did you use lcd or gcode to set sensitivity? wondering if its just a lcd menu issue

I tried M914 directly.
M914 X-1
M914 --> X homing sensitivity: 127
My display(BTT TFT35 V3.0) reports 63, probably the display limits the value to 63.

[ellensp]

It seems to be in the tmc library

I added a bit of debugging

diff --git a/Marlin/src/feature/tmc_util.h b/Marlin/src/feature/tmc_util.h
index 99d9dc4bc1..029cb059bc 100644
--- a/Marlin/src/feature/tmc_util.h
+++ b/Marlin/src/feature/tmc_util.h
@@ -135,8 +135,11 @@ class TMCMarlin : public TMC, public TMCStorage<AXIS_LETTER, DRIVER_ID> {
     #if USE_SENSORLESS
       int16_t homing_threshold() { return TMC::sgt(); }
       void homing_threshold(int16_t sgt_val) {
+        SERIAL_ECHOLN(PSTR("Setting "), AXIS_LETTER, PSTR(" homing threshold to "), sgt_val," ",sgt_min," ", sgt_max);
         sgt_val = (int16_t)constrain(sgt_val, sgt_min, sgt_max);
+        SERIAL_ECHOLN(PSTR("Set homing threshold to "), sgt_val," ",sgt_min," ", sgt_max);
         TMC::sgt(sgt_val);
+        SERIAL_ECHOLN(PSTR("Homing threshold set to "), TMC::sgt());
         TERN_(HAS_MARLINUI_MENU, this->stored.homing_thrs = sgt_val);
       }
       #if ENABLED(SPI_ENDSTOPS)

Sent

M914 X-1
SENDING:M914 X-1
Setting 88 homing threshold to -1 -64 63
Set homing threshold to -1 -64 63
Homing threshold set to 127

(88 is 'X' char, didn't bother to fix that, its only temporary debugging.)

So homing_threshold function got correct value
It got past the constraints
and was sent to the TMC::sgt library call.
but either the TMC::sgt set or get lost the sign.

[ellensp]

In the library, from TMCStepper/src/source/COOLCONF.cpp we have the following to set and get the sgt register

void TMC2130Stepper::sgt(	int8_t  B )	{ SET_REG(sgt);		}

int8_t TMC2130Stepper::sgt() {
	uint8_t raw = COOLCONF_register.sgt; // "int7_t"
	return (raw & 0x40) ? raw - 128 : raw;
}

The top set is ok

The bottom get is wrong.
Walking through it.
eg -1 in signed 7bit is stored as 01111111 signed 8bit (127 in decimal) the code checks for the signed bit at 0x40 (b01000000) and if found it subtracts 128. But 127-128 = -1 and is not possible as variable raw is uint8_t

The following code works, replacing the -128 with setting the sign bit directly.

int8_t TMC2130Stepper::sgt() {
	uint8_t raw = COOLCONF_register.sgt; // "int7_t"
	return (raw & 0x40) ? raw | 0x80 : raw;
}

now the results with debugging gives correct results

M914 X-1
SENDING:M914 X-1
Setting 88 homing threshold to -1 -64 63
Set homing threshold to -1 -64 63
Homing threshold set to -1

This bug was introduced in MarlinFirmware/TMCStepper@d7e12dd

[rondlh]

In the library, from TMCStepper/src/source/COOLCONF.cpp we have the following to set and get the sgt register

void TMC2130Stepper::sgt( int8_t B ) { SET_REG(sgt); }

int8_t TMC2130Stepper::sgt() {
uint8_t raw = COOLCONF_register.sgt; // "int7_t"
return (raw & 0x40) ? raw - 128 : raw;
}

The following code works, replacing the -128 with setting the sign bit directly.

int8_t TMC2130Stepper::sgt() {
uint8_t raw = COOLCONF_register.sgt; // "int7_t"
return (raw & 0x40) ? raw | 0x80 : raw;
}

Confirmed, this solved the issue for me, thanks!

[ellensp]

Oh this bug broke a bunch of drivers as many inherit from TMC5130Stepper