Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stepper driver startup current #400

Closed
dresco opened this issue Dec 4, 2023 · 11 comments
Closed

Stepper driver startup current #400

dresco opened this issue Dec 4, 2023 · 11 comments

Comments

@dresco
Copy link
Contributor

dresco commented Dec 4, 2023

Was just taking a closer look at driver_setup(), with respect to the stepper enable pins & startup current draw in the Trinamic drivers..

I'd updated the H7 driver in line with the recent F4 changes, but I don't believe this is working as intended.

I think the the DIGITAL_OUT() has to remain after the HAL_GPIO_Init(), else it has no effect on the physical pin state (at least that is what I'm observing on the H7).

What I see currently is that GPIO_Init() sets the pin output low (typically enabling the stepper driver), and it stays that way until deenergized later from grbl_enter().

With the DIGITAL_OUT line (including StepperEnable), moved back below HAL_GPIO_Init() - then at least the output would be set high almost immediately after the output is initialised?

(Appreciate this initial DIGITAL_OUT pays no attention to the $4 inversion setting, but presumably that's a whole different can of worms).


Recent update to the H7 driver below for reference;

--- a/Src/driver.c
+++ b/Src/driver.c
@@ -1943,12 +1943,13 @@ static bool driver_setup (settings_t *settings)
 
     for(i = 0 ; i < sizeof(outputpin) / sizeof(output_signal_t); i++) {
         if(outputpin[i].group != PinGroup_StepperPower) {
+
+            if(outputpin[i].group == PinGroup_MotorChipSelect || outputpin[i].group == PinGroup_MotorUART || outputpin[i].group == PinGroup_StepperEnable)
+                DIGITAL_OUT(outputpin[i].port, outputpin[i].bit, 1);
+
             GPIO_Init.Pin = outputpin[i].bit = 1 << outputpin[i].pin;
             GPIO_Init.Mode = outputpin[i].mode.open_drain ? GPIO_MODE_OUTPUT_OD : GPIO_MODE_OUTPUT_PP;
             HAL_GPIO_Init(outputpin[i].port, &GPIO_Init);
-
-            if(outputpin[i].group == PinGroup_MotorChipSelect || outputpin[i].group == PinGroup_MotorUART)
-                DIGITAL_OUT(outputpin[i].port, outputpin[i].bit, 1);
         }
     }
@terjeio
Copy link
Contributor

terjeio commented Dec 5, 2023

I set the output state early on purpose to avoid a glitch on the pin (going from high-Z input directly to high). It was done to avoid a current surge in the stepper drivers on startup. Writing to the output register before setting direction is allowed and the pin will be set to the state held in the output register when direction is set. From the PRM (7.3.10):

When the I/O port is programmed as output:
The output buffer is enabled:
– Open drain mode: A “0” in the Output register activates the N-MOS whereas a “1” 
in the Output register leaves the port in Hi-Z (the P-MOS is never activated)
– Push-pull mode: A “0” in the Output register activates the N-MOS whereas a “1” in 
the Output register activates the P-MOS

@terjeio
Copy link
Contributor

terjeio commented Dec 5, 2023

This is the issue that triggered the change.

@dresco
Copy link
Contributor Author

dresco commented Dec 5, 2023

This is the issue that triggered the change.

Thanks, yes that is exactly the scenario that had me looking closer at this...

Writing to the output register before setting direction is allowed and the pin will be set to the state held in the output register when direction is set. From the PRM (7.3.10):

I see, that makes total sense then. However, it doesn't seem to work on the H7... :(

Looking more closely, it seems the BSSR write doesn't work if not already in output mode, as the corresponding bit in ODR is not set, and the pin is subsequently driven low on HAL_GPIO_Init().

It does work if I set the relevant bit in ODR directly (instead of using DIGITAL_OUT). I wonder if this behaviour is the same for the F7?

@terjeio
Copy link
Contributor

terjeio commented Dec 5, 2023

I wonder if this behaviour is the same for the F7?

I'll check when I am back home, going tomorrow.

@dresco
Copy link
Contributor Author

dresco commented Dec 5, 2023

I'll check when I am back home, going tomorrow.

Thanks, fwiw this is what I have currently;

diff --git a/Src/driver.c b/Src/driver.c
index abee334..45e42df 100644
--- a/Src/driver.c
+++ b/Src/driver.c
@@ -1944,8 +1944,9 @@ static bool driver_setup (settings_t *settings)
     for(i = 0 ; i < sizeof(outputpin) / sizeof(output_signal_t); i++) {
         if(outputpin[i].group != PinGroup_StepperPower) {
 
+            // Set the initial state of the pin when it is enabled (note: need to use ODR instead of BSRR)
             if(outputpin[i].group == PinGroup_MotorChipSelect || outputpin[i].group == PinGroup_MotorUART || outputpin[i].group == PinGroup_StepperEnable)
-                DIGITAL_OUT(outputpin[i].port, outputpin[i].bit, 1);
+                outputpin[i].port->ODR |= 1 << outputpin[i].pin;
 
             GPIO_Init.Pin = outputpin[i].bit = 1 << outputpin[i].pin;
             GPIO_Init.Mode = outputpin[i].mode.open_drain ? GPIO_MODE_OUTPUT_OD : GPIO_MODE_OUTPUT_PP;

@terjeio
Copy link
Contributor

terjeio commented Dec 11, 2023

I wonder if this behaviour is the same for the F7?

Yes, it is. Your code above works.

@dresco
Copy link
Contributor Author

dresco commented Dec 11, 2023

Am also wondering whether there should be $ parameter settings for default current per axis (before any M906 changes), do you have any thoughts on this?

When I was looking at the Marlin code (looking for clues re the reported 5160 driver problems on the H7 board), I noticed that it sets a default current limit of 800mA for 5160 drivers.

Out of the box, the EZ5160 Pro driver modules have a default current setting that seems a bit sketchy without active cooling - according to their own product page.

image

My test board pulls a solid 1.8A @ 24V just turning a single NEMA17 motor in this default state..

M122
[TRINAMIC]
                      X
Driver          TMC5160
Set current           0
RMS current        2960
Peak current       4186
Run current       30/31
Hold current      15/31
...

@terjeio
Copy link
Contributor

terjeio commented Dec 11, 2023

Am also wondering whether there should be $ parameter settings for default current per axis

In addition to the $14x settings? Default values are set here.
Could it be that you need to reset driver/plugin settings with $RST=& since Set current is reported as 0?

@dresco
Copy link
Contributor Author

dresco commented Dec 11, 2023

In addition to the $14x settings? Default values are set here.
Could it be that you need to reset driver/plugin settings with $RST=& since Set current is reported as 0?

Hah! No, that is exactly what I was suggesting - I'd just not noticed it existed already 😊

They do always seem to revert to 0 on a settings reset though, will investigate that..

@dresco
Copy link
Contributor Author

dresco commented Dec 11, 2023

Hmm, they seem to be explicitly set to zero here, is that expected?

@terjeio
Copy link
Contributor

terjeio commented Dec 11, 2023

Oops, this may be an unintended side-effect from core changes I made earlier. The core defaults were initially for I2C potentiometers - I'll have to fix it...

@dresco dresco closed this as completed Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants