Possible bug with sine amplitude calculation when encoder frequency is negative  [SOLVED]

Post Reply
User avatar
catphish
Posts: 954
Joined: Fri Oct 08, 2021 11:02 pm
Location: Dorset, UK
Has thanked: 93 times
Been thanked: 179 times

Possible bug with sine amplitude calculation when encoder frequency is negative

Post by catphish »

I have been experiencing a mildly annoying problem with my sine firmware. Torque seems to *dramatically* increase when the vehicle is rolling backwards.

There are 2 ways to reproduce this problem (the second is a bit more scientific):

1) If the vehicle is stationary, and I apply a small amount of throttle, the car doesn't move. However, if the car is rolling backwards and I apply the same minimal amount of throttle, there is a *violent* forward torque, but only until the car is stationary again.

2) If i set regentravel=0, and ampmin=30, so that the inverter is consistently at ampnom=30 without having to touch anything, there is positive torque, but not enough to move the car forwards on the flat. However, if I apply this on an uphill, something odd happens, the car starts to roll backwards, then stops. Then starts to roll backwards again, then stops.

Instinctively, felt to me as if negative movement of the encoder is being treated as positive when calculating the output frequency and amplitude. I'm now attempting to look at the code, and it seems the reality is a little more complicated.

Code: Select all

void PwmGeneration::CalcNextAngleAsync(int dir)
{
   static uint16_t slipAngle = 0;
   uint16_t rotorAngle = Encoder::GetRotorAngle();

   frq = polePairRatio * Encoder::GetRotorFrequency() + fslip;
   slipAngle += dir * slipIncr;

   if (frq < 0) frq = 0;

   angle = polePairRatio * rotorAngle + slipAngle;
}
The next angle is in fact calculated correctly, as it only uses rotor position and requested slip.

HOWEVER... frq is calculated using Encoder::GetRotorFrequency() which is unsigned.

frq is then used here to calculate amplitude:

Code: Select all

uint32_t amp = MotorVoltage::GetAmpPerc(frq, ampNomLimited);
The result of all of this is that is the vehicle is traveling at -2Hz, and the requested slip is 2Hz, the output frequency will be 0Hz but the output amplitude will be based on 4Hz!

In my opinion, Encoder::GetRotorFrequency() should return a signed value, and voltage should be calculated more like this:

Code: Select all

frq = ABS(dir * polePairRatio * Encoder::GetRotorFrequency() + fslip);
I hope this makes sense! I wish I could simply fix this myself and submit a patch, but my only drive unit is in a vehicle on the road I'm not able to safely test experimental changes myself. Sorry!
User avatar
catphish
Posts: 954
Joined: Fri Oct 08, 2021 11:02 pm
Location: Dorset, UK
Has thanked: 93 times
Been thanked: 179 times

Re: Possible bug with sine amplitude calculation when encoder frequency is negative

Post by catphish »

I've written a patch for this: https://github.com/catphish/stm32-sine/ ... e8ce293214

It appears to work with by AB encoder and sine output, however there are two outstanding issues:

1) This may break FOC, and even if it doesn't, it means the behavior is different between the two. It would be good if someone who maintains FOC could look over this and see if the same change could be implemented in FOC for consistency.

2) The speed parameter is now signed, and reads negative when the motor is running in reverse. I like this but it may confuse existing CAN integrations. Perhaps an option should be added to choose between signed and unsigned speed display?

Attached are graphs generated by flicking the direction switch between FWD and REV and high and low throttle. I will be working in making this transition work smoothly and this patch is the first step in doing so.
Attachments
Screenshot at 2023-01-06 23-21-27.png
Screenshot at 2023-01-06 23-31-15.png
User avatar
johu
Site Admin
Posts: 5681
Joined: Thu Nov 08, 2018 10:52 pm
Location: Kassel/Germany
Has thanked: 153 times
Been thanked: 959 times
Contact:

Re: Possible bug with sine amplitude calculation when encoder frequency is negative

Post by johu »

The reason why there is no continuous transition from positive to negative frequency becomes apparent when working with Leaf or other OEM stuff: it becomes hard to NOT transition from forward regen to backward acceleration.
That's why I did this a bit more explicit with the additional direction specifier.

But I think carefully crafted this could be a more straight-forward way. Also we could leave speed absolute and fstat signed perhaps to not break CAN integration?
Support R/D and forum on Patreon: https://patreon.com/openinverter - Subscribe on odysee: https://odysee.com/@openinverter:9
User avatar
catphish
Posts: 954
Joined: Fri Oct 08, 2021 11:02 pm
Location: Dorset, UK
Has thanked: 93 times
Been thanked: 179 times

Re: Possible bug with sine amplitude calculation when encoder frequency is negative

Post by catphish »

johu wrote: Sat Jan 07, 2023 2:28 pm The reason why there is no continuous transition from positive to negative frequency becomes apparent when working with Leaf or other OEM stuff: it becomes hard to NOT transition from forward regen to backward acceleration.
That's why I did this a bit more explicit with the additional direction specifier.

But I think carefully crafted this could be a more straight-forward way. Also we could leave speed absolute and fstat signed perhaps to not break CAN integration?
I've just been testing this and it's apparent that this change has a knock-on effect on various other parts of the code. For example cruise control assumes that speed will be positive.

As you know, I am not a big fan of the concept of explicit direction states, however for now I think I will update this patch to create a separate new function to return the signed speed to avoid breaking *everything* just to fix one small issue. Perhaps later I'll get a bit more bold with it.
User avatar
catphish
Posts: 954
Joined: Fri Oct 08, 2021 11:02 pm
Location: Dorset, UK
Has thanked: 93 times
Been thanked: 179 times

Re: Possible bug with sine amplitude calculation when encoder frequency is negative

Post by catphish »

It turns out that the entire process of negative speed being returned was a bug.

GetSpeed() is still unsigned, only GetRotorFrequency() has changed. I've fixed the bug and speed is now always positive as usual, while rotor frequency is signed for internal use.

https://github.com/catphish/stm32-sine/ ... dd7b5e7dc7
User avatar
johu
Site Admin
Posts: 5681
Joined: Thu Nov 08, 2018 10:52 pm
Location: Kassel/Germany
Has thanked: 153 times
Been thanked: 959 times
Contact:

Re: Possible bug with sine amplitude calculation when encoder frequency is negative

Post by johu »

Two more things I imagine need work is the regenrampstr and FrequencyLimitCommand() as they rely on fstat being absolute
Support R/D and forum on Patreon: https://patreon.com/openinverter - Subscribe on odysee: https://odysee.com/@openinverter:9
User avatar
catphish
Posts: 954
Joined: Fri Oct 08, 2021 11:02 pm
Location: Dorset, UK
Has thanked: 93 times
Been thanked: 179 times

Re: Possible bug with sine amplitude calculation when encoder frequency is negative  [SOLVED]

Post by catphish »

johu wrote: Sat Jan 07, 2023 4:41 pm Two more things I imagine need work is the regenrampstr and FrequencyLimitCommand() as they rely on fstat being absolute
Thanks! I'd already spotted regenrampstr and that one's now fixed. I believe FrequencyLimitCommand() operates on the output frequency, not the input frequency, so I believe that is already handled.

I will test frequency limiting though before calling this final: https://github.com/catphish/stm32-sine/ ... 4641a8e563


On an loosely related note... because regen also operates on absolute speed, I believe it is important to check the motor direction before applying regen.

On the bench I did the following:
* Enable off throttle regen
* Spin up the motor
* Switch direction to reverse

The result is that the throttle calls for regen and because the motor is spinning. negative torque is applied, and it accelerated backwards. This patch should fix that (it does in my testing): https://github.com/catphish/stm32-sine/ ... a3217466bf
User avatar
catphish
Posts: 954
Joined: Fri Oct 08, 2021 11:02 pm
Location: Dorset, UK
Has thanked: 93 times
Been thanked: 179 times

Re: Possible bug with sine amplitude calculation when encoder frequency is negative

Post by catphish »

This change seems happy on the bench. I will need to wait for the weather improve and will test it in my car before creating a pull request.

While this patch definitely fixes a bug, I won't know until I test it in my car whether or not it achieves the goal of improving smoothness when accelerating gently while rolling backwards.

I would like to rule out this bug first, but if this doesn't solve the problem, I believe the next step will be to look a the MotorVoltage::GetAmpPerc function. This should be able to smoothly roll off the voltage from 1Hz down to 0Hz rather than the hard cutoff at 0.2Hz currently used.
Post Reply