not enough piece of code

[lonelymyp]

when copying the original code
an important piece of code was lost in the procedure
int32_t A4950_move(int32_t stepAngle, uint32_t mA)
in file A4950.c

//scale sine result by current(mA)
dacSin=((int32_t)mA*(int64_t)abs(sin))/SINE_MAX;
//scale cosine result by current(mA)
dacCos=((int32_t)mA*(int64_t)abs(cos))/SINE_MAX;

[makerbase-mks]

I have open file, and check it.

[lonelymyp]

When you copied the code from the original project, you forgot to copy an important piece of code.

Without this piece, the pid algorithm works poorly. position correction current is not proportional and accuracy drops

[dzid26]

What important piece is missing? Scaling down by SINE_MAX ?

[martinbogo]

Hey -- did you make a PR from this issue? I suggest you edit the code, allow the fork, then submit the PR. AFTER you test the code.

[dzid26]

@martinbogo What is not working?

[Quas7]

Just to clarify, what I think is noted above.

In the original code from misfittech, the dacCos is scaled two times. First by SINE_MAX and then by 3300 (full scale current in mA)
In the MKS code, it is only scaled to the max current of 3300 and SINE_MAX scaling is missing.

code from misfit:

	//scale sine result by current(mA)
	dacSin=((int32_t)mA*(int64_t)abs(sin))/SINE_MAX;
	//scale cosine result by current(mA)
	dacCos=((int32_t)mA*(int64_t)abs(cos))/SINE_MAX;

	//convert value into DAC scaled to 3300mA max
	dacCos=(int32_t)((int64_t)dacCos*(DAC_MAX))/3300;
        //convert value into DAC scaled to 3300mA max
	dacSin=(int32_t)((int64_t)dacSin*(DAC_MAX))/3300;

EDIT: I see an improvment by adding the /SINE_MAX scaling and started a pull request.

[Quas7]

Looking at the sineTable in sine.c I now wonder, why it is limited to 511 (~9bit) as there is a 12bit input from the A1333 encoder and the PWM to the A4950 also has a higher resolution.

Misfittech also has much more resolution in the sineTable(s) but has also an ATMEL ARM which might be a bit beefier than the STM32F1 used here.
https://github.com/Misfittech/nano_stepper/blob/master/firmware/stepper_nano_zero/sine.cpp
and set its SINE_MAX accoringly higher with
https://github.com/Misfittech/nano_stepper/blob/master/firmware/stepper_nano_zero/sine.h
#define SINE_MAX ((int32_t)(32768L))

Mechaduino is using 10bit.
https://github.com/jcchurch13/Mechaduino-Firmware/blob/master/Mechaduino/Mechaduino/Parameters.cpp

Comparing to bigtreetech S42B.
They also use a 10 bit sineTable for the same uC. So, the STM32F1 microcontroller seems to be capable of taking at least one additional bit. ;)

[dzid26]

@Quas7 The effective resolution is limited by timer max frequency 72Mhz. If it counts to 32767, it will be updated every 72Mhz/32767= ~2kHz. Slow.

With 512, it is ~140kHz.
Now, the main loop by default runs at 8kHz (although I run it at 16kHz, for slightly smoother operation), so the timer should be at least as fast as the main loop. 2kHz is too slow.
Going faster than the main loop should reduce latency because you wait very little time for the timer to trigger after setting a value.
You would think 140kHz is overkill, but on the other hand, why would we need more precise current. It's not like the drivers are following the current perfectly anyway.
Also remember, there is low pass filter (capacitor and resistor) that convert PWM into smooth voltage reference, So the faster the frequency, the smoother the voltage ref.

So I tried 32767 and the motor was sluggish (not catching up with main loop) and noisy (too slow for the low pass filter).
Then I tried 1023, and couldn't tell a difference between 511.

Then I tried 255 and it actually made a little less hissing than it was with 511 or 1023.

Then I tried 127 and the high pitch noise disappeared completely! I am going to stick to that because I don't feel any performance difference. It feels as smooth as on 511 or 1023.

"511 counter" peaks around -43dB@18kHz:

(16kHz is background noise).

"127 counter" peaks around -63dB@18kHz.

Significantly under the red line from the previous maximum..

EDIT1: Original question was about the sine range, not timer range. I was talking about timer range. While sine range and timer range are related, there is no reason for sine wave not to be a bit greater range (unless we want to use int8) and then divided at the end when calculating vrefX and vrefY for the timer threshold.

EDIT2: Perhaps adding dither, described by ST, would solve noisy harmonics(?) and allow to maintain the resolution of the final timer counting.

[Quas7]

@dzid26 thanks for the nice background informations.

But I got a bit confused where you found the RC filter for the motion PWM signals.
Signals IN1,2,3,4 reflect the motion signals to the A4954 stepper motor driver (wrongly named "A9450" in the schematic) and as far as I see, they are connected directly to between uC and the driver.

Or did you maybe mean the VREF signals as there is actually an RC low pass in the schematic?
But VREF is only limiting the max current of the drivers and the sine table should not impact there (I hope...).

VREF
The maximum value of current limiting is set by the selection of RSx and the voltage at the VREFx pin in each channel. [...]
(And another topic but the VREF function likely has a flaw, as I see max current limits breached during calibration/re-test - it is obiously getting burning hot and clipping my bench supply current limit of 2A @15v.)

Anyway, I agree that we would have to stay out of the audible spectrum with >25kHz for the stepper PWM.
And I also agree, that even when using 128-ustepping we should still be fine with a 512-sine table (4x resolution) with regards to the microstep error as there are much bigger sources for errors than quantization in the complete system. ;)

[dzid26]

@Quas7
I have learnt that the sine wave is indeed controlled via Vref.
IN1,2,3,4 is only used to select the phase.

So the current control loop is really delegated to the A4954 chip. The chopping frequency of the chip is somewhere between 20-40kHz based on this

This control scheme is a little different from what you would see in FOC controllers, where normally MCU controls the current according to sensed current (which here is not possible, as there is no op-amps nor a connection to measure shunt voltage). Surprisingly it works really well.

I imagine that the only time the real current would be far from the requested current is at high speed - when BEMF prevents current flow. Unfortunately, we wouldn't know that this is occurring, so it's likely that in this case the current distribution between phases is not optimal. (Perhaps BEMF could be modeled to compensate for that).

Otherwise, especially at low speed, the "current limit" method for shaping the current signal works nicely.

[Quas7]

Thanks again for the nice details of this concept! That explains, why I could net get my head around the datasheet. ;)

Regarding the overcurrent.
I observed this only during calibration and, if I remember correctly, also in cal-testing.
I just assume, that calibration is done in open-loop and somewhere the Vref limit is getting ignored or set to 3300mA max current limit.