Klipper3d / klipper

Klipper is a 3d-printer firmware
GNU General Public License v3.0
9.55k stars 5.33k forks source link

FYSETC TMC5160 v1.2 - diag0_stall homing #3150

Closed trevjonez closed 4 years ago

trevjonez commented 4 years ago

Kevin:

I've just been updating one of my machines to use some new drivers and the latest klipper code. What I am finding is that the default of using diag1_stall is not appropriate for these step sticks.

I've been able to workaround by modifying this line to reference "diag0_stall" instead.

Would you be open to a pull request that exposes a configuration option to customize which register to use? It seems only the 2130 and 5160 could be impacted by such a change. Though I am doubtful any 2130 driver layout would need this change or you would have likely seen this issue before.

Or is the timing of homing override gcode timing appropriate that I could use a SET_TMC_FIELD to set the value immediately preceding a G28 command? Skimming the extras files made it seem like any value I set would be overridden by handle_homing_move_begin

https://wiki.fysetc.com/S5160_V1.2/ https://github.com/FYSETC/TMC-Stepstick-Divers/blob/master/hardware/S5160/V1.2%203020/FYS_TMC5160_V1.2.pdf

Notes for SEO:

Similar to other 5160 stepsticks in order to get sensorless homing to function some modifications to the driver need to be made. The FYSETC 5160 v1.2 driver exposes the CLK pin via pin3 on connector P2. For sensorless homing to work you need to remove that pin and connect it to ground (pin 1 on connector P6). Additionally you will need to add a pin to the board on pin 2 of connector P1 in order to connect the diag0 signal to the MCU.

This was for use with an FYSETC S6 V1.2

All done, it looks like this: 00100lrPORTRAIT_00100_BURST20200803093630154_COVER

config for one of the machines setup with this is here: https://github.com/trevjonez/klipper-configs/blob/MK3-Bear-2/fysetc_s6_bear.cfg

klipper-gitissuebot commented 4 years ago

Hi @trevjonez,

It did not look like there was a Klipper log file attached to this ticket. The log file has been engineered to answer common questions the Klipper developers have about the software and its environment (software version, hardware type, configuration, event timing, and hundreds of other questions).

Unfortunately, too many people have opened tickets without providing the log. That consumes developer time; time that would be better spent enhancing the software. If this ticket references an event that has occurred while running the software then the Klipper log must be attached to this ticket. Otherwise, this ticket will be automatically closed in a few days.

For information on obtaining the Klipper log file see: https://github.com/KevinOConnor/klipper/blob/master/docs/Contact.md

The log can still be attached to this ticket - just add a comment and attach the log to that comment.

Best regards, ~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

KevinOConnor commented 4 years ago

Adding support for sensorless homing with diag0 seems like a useful addition to me. FWIW, I'd make a separate virtual pin (eg, "tmc5160_mydriver:virtual_diag0_endstop") that activates the alternative diag0 line.

-Kevin

trevjonez commented 4 years ago

I don't think I am grasping how an additional virtual pin can solve this. Is this something that can all be handled in the printer.cfg by that mechanism? Or is the thinking to support more correct naming?

Here is my current config:

# FYSETC S6 v1.2

[stepper_x]
step_pin: PE11
dir_pin: PE10
enable_pin: !PE12
step_distance: .0025
endstop_pin: tmc5160_stepper_x:virtual_endstop
position_endstop: 0
position_max: 250
homing_speed: 50
homing_retract_dist: 0

[tmc5160 stepper_x]
spi_bus: spi1
cs_pin: PE7
microsteps: 32
interpolate: True
run_current: .3
diag1_pin: ^!PB14
stealthchop_threshold: 300
driver_SGT: 1

Other than it being named diag1_pin it doesn't seem like the naming is even important, just that PB14 is the signal used for the virtual_endstop. Or are you saying, I could use a virtual pin that is an indirection to PB14 as a way to detect what register to set in tmc.py?

Do you think there would be any consequences to just setting both diag0_stall and diag1_stall in tmc.py and leaving the rest as is? I would expect this would be lowest friction if it wouldn't cause any misbehavior of hardware? IIRC the einsy and mmu2 board tie both 2130 diag pins to one mcu pin.

Or what if the config key was diag0_pin OR diag1_pin as the pin label, then vary the register used in tmc.py based on which was used in the cfg file?

KevinOConnor commented 4 years ago

Ah, I hadn't thought enough about it I guess.

Or what if the config key was diag0_pin OR diag1_pin as the pin label, then vary the register used in tmc.py based on which was used in the cfg file?

That sounds good to me.

-Kevin

ETE-Design commented 3 years ago

Is this the same for TMC5161, I guess the answer is yes?

trevjonez commented 3 years ago

This spec sheet says on the first page "Interface-compatible to TMC5160" so I am thinking yes you would configure it just like a 5160 and klipper would just work.

Though looking at the fysetc stepsticks the pinout looks corrected so you shouldn't need to remove or bridge the CLK pin to ground like I had to on the 5160's (5161 the pin is not connected to anything on the driver). It looks like the encoder pins are not exposed and the SPI pins are double sided to work with boards that need to do SPI via flying wires. Though you will have to add a pin to connect diag0 to your MCU. Overall looks far more user friendly, good job to fysetc I'd say.

https://wiki.fysetc.com/S5160_V1.2/ https://wiki.fysetc.com/S5161_V1.1/

ETE-Design commented 3 years ago

@trevjonez So only thing needed is to solder diag0, and no changes in Klipper needed anymore right?

trevjonez commented 3 years ago

@trevjonez So only thing needed is to solder diag0, and no changes in Klipper needed anymore right?

I believe so yes