vedderb / bldc

The VESC motor control firmware
2.08k stars 1.31k forks source link

Fix noisy currents on Makerbase and Flipsky models #704

Closed casainho closed 3 months ago

casainho commented 5 months ago

This firmware filters the motor phase currents and makes the motor detection more stable and correct, and the motor is much more silent at startup and with more torque, compared to original VESC firmware. See here the changes I did: https://github.com/casainho/bldc/tree/VESCs_fix_noisy_currents-firmware_6.02

Credit: changes by github user korjaa (https://github.com/korjaa/bldc/commits/noise_study). See discussion here: https://forum.esk8.news/t/how-to-update-firmware-on-the-flipsky-75100-75200-foc-esc/61819/318

Was tested on my Lunyee 72V 2000W dual phase motor, installed on a micro scooter Fiido Q1S, using a Makerbase MKSESC_75_200_V2 and a 72V battery - more info here.

Lunyee 72V 2000W dual phase motor image

image

image

Fiido Q1S micro scooter with a Lunyee 72V 2000W dual phase motor installed on the rear wheel image

image

casainho commented 5 months ago

@makerbase-motor please look at this!!

Captaintoon12345 commented 5 months ago

oh oh oh looks great! it's true that my markerbase is noisy at low speed. Hardly waiting this commit!

kalvdans commented 5 months ago

Please rebase on top of master, to get a cleaner diff to review.

casainho commented 5 months ago

Please rebase on top of master, to get a cleaner diff to review.

This may be strange, but I don't know what the firmware code added does. I understand it changes the VESC motor core files and I don't know what this code does. This code was done by another user ( I do mention on the PR description) and I just keep it updated to VESC firmware V6.02. Also, I don't have time right now and in coming weeks.

I hope this PR make other users and developers, like @makerbase-motor and Flipsky, that their hardware is not good at least for 2kW motors like mine, and there is cahnges in VESC firmware that can improve the issue - but all this needs to be investigated, and I hope this manufactures invest to improve all this.

vedderb commented 5 months ago

This PR is simply unacceptable. It removes many of the config files required for building the firmware package and adds binary files that shouldn't be in the repository.

Regarding the noise issue, can you try the following and see if it solves the problem:

  1. Get the latest beta-version of VESC Tool and update the firmware.
  2. Do the motor configuration as usual.
  3. Try the "All Sensors Combined" current sample mode and see if it helps: bild
casainho commented 5 months ago

This PR is simply unacceptable. It removes many of the config files required for building the firmware package and adds binary files that shouldn't be in the repository.

I understand. But even more unacceptable, is the fact I don't know what the changes on the VESC core files do!! I did explain on previous message this. This is more to bring awareness to this issue and call the @makerbase-motor and Flipsky.

Regarding the noise issue, can you try the following and see if it solves the problem:

Thanks, I will do it. And I will try to understand on the code how this new way of measuring currents works and how that compares with changes done on this PR code.

I am happy to improve VESC!!

Captaintoon12345 commented 5 months ago

I will also try on mine to see if the nex version of VESC tool change something. Best regards

On Wed, Jan 31, 2024 at 11:56 PM Benjamin Vedder @.***> wrote:

This PR is simply unacceptable. It removes many of the config files required for building the firmware package and adds binary files that shouldn't be in the repository.

Regarding the noise issue, can you try the following and see if it solves the problem:

  1. Get the latest beta-version of VESC Tool and update the firmware.
  2. Do the motor configuration as usual.
  3. Try the "All Sensors Combined" current sample mode and see if it helps: bild.png (view on web) https://github.com/vedderb/bldc/assets/2311760/79234adf-8f41-4f8f-97e4-600d56211c03

— Reply to this email directly, view it on GitHub https://github.com/vedderb/bldc/pull/704#issuecomment-1920128859, or unsubscribe https://github.com/notifications/unsubscribe-auth/BBDNCRWXITM6EOC62NSTAGDYRLD23AVCNFSM6AAAAABCTEMYK2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRQGEZDQOBVHE . You are receiving this because you commented.Message ID: @.***>

-- Frédéric PIERRON

casainho commented 5 months ago

Regarding the noise issue, can you try the following and see if it solves the problem:

1. Get the latest beta-version of VESC Tool and update the firmware.

2. Do the motor configuration as usual.

3. Try the "All Sensors Combined" current sample mode and see if it helps:

I did some tests and you are correct, the new option All sensors combined get's ride of that noise at startup. Also, the other 2 options Longest Zero time and High current, makes that noise.

But there are still issues:

  1. the motor detection stills not detects a consistent value of Lq-Dq, so I decided to use the detected values by the firmware on this PR. Here an example of the values I use and the values detected by the last master branch firmware:

image

  1. seems there is a good torque at startup (maybe while use the hall sensors, but just after I miss a bit of torque while it return later at higher RPMs.

  2. at braking, mainly at lower speeds, probably when backing to hall sensors, there is sometimes some bad noise and vibrations.

I also tested the observer types, and the Ortega original is the best, on both firmwares I tested. The other other observer from the other developer, do not work well on this Makerbase 75200 and firmware.

So for now I will keep using the firmware on this PR.

TechAUmNu commented 4 months ago

This is just the fix that @ElwinBoots suggested on Discord, where it subtracts the average of the 3 currents. i.e. removes common mode noise.

jaykup26 commented 4 months ago

This is just the fix that @ElwinBoots suggested on Discord, where it subtracts the average of the 3 currents. i.e. removes common mode noise.

Is this what the new "All Sensors Combined" setting does? Replicates Elwin's changes as tracked by this commit?

https://github.com/korjaa/bldc/pull/1/files

ElwinBoots commented 4 months ago

First of all, good to see that you trying to find fixes for a badly performing hardware/software combination.

That is indeed exactly what I did, and effectively the result is that you get the full clarke transform. Drawbacks can occur at high duty cycle.

That something else apperently got worse in 6.05 versus 6.02 is interesting. Would need testing to find out what that is.

casainho commented 3 months ago

Regarding the noise issue, can you try the following and see if it solves the problem:

1. Get the latest beta-version of VESC Tool and update the firmware.

2. Do the motor configuration as usual.

3. Try the "All Sensors Combined" current sample mode and see if it helps:

I built yesterday the VESC Tool and set the options as you suggested.

As comparison, this values were measured using the code on this PR. This is the noise of the motor phase 1, running at 5000 RPM: image

And current on all the phase with command rotor_lock_openloop 5 10 45: image

And now the latest firmware from main:
Options: V0 only; All sensors combined
3A current This is the noise of the motor phase 1, running at 5000 RPM: image

And current on all the phase with command rotor_lock_openloop 5 10 45: image

So the final result is that the option All sensors combined, although improves, it still have more noise on the motor phases and I clearly get more noise at motor startup and acceleration / high currents at least. I just did a few shorts tests and it is far from good as the code on this PR.

@vedderb is there a way I can help on my side to get the improvement on this PR on the main?

vedderb commented 3 months ago

You have to compare the MC total current as that is what it used in the control loop. The reason that the other currents look better in the PR is that it ruins the raw sampling by applying compensation on them. In the sampled data, phase-samples should represent what is measured and MC total should represent what is used for control.

casainho commented 3 months ago

You have to compare the MC total current as that is what it used in the control loop. The reason that the other currents look better in the PR is that it ruins the raw sampling by applying compensation on them. In the sampled data, phase-samples should represent what is measured and MC total should represent what is used for control.

I did a few comparisons, using the _focopenloop 40 1500.

This is the firmware from this PR:

image

And this is from latest BLDC main branch firmware:

image

The PR firmware has a bit improved MC total. In practice, I don't get anymore the noise I was getting with previous VESC firmware 6.02 and everything seems good as expected!! So, I think this PR can be closed now.

But this PR firmware still have a big advantage! -- the FOC detection of R, L, etc, gives always constant values but that is not the case with latest BLDC main branch firmware, they always change and in big differences!!! @vedderb - could you think if this could be improved on the main branch? And many THANKS <3

TechAUmNu commented 3 months ago

The detection will be different if you run detection without 'All sensors combined' selected. This pr basically makes that the default, for normal fw you have to specifically select it. Maybe making it the default might be a good idea.

vedderb commented 3 months ago

Good point, didn't think of that. Yeah, might be a good idea to always use all sensors combined for detection as the duty cycle will be low then and we don't run into the issue where the measurement time becomes short.

vedderb commented 3 months ago

Another point: The sampling mode should be all sensors combined in the hw-file on hardware that has this kind of noise-issue. Then users don't have to spend time on finding that setting and detection should work well too.

casainho commented 2 months ago

The detection will be different if you run detection without 'All sensors combined' selected. This pr basically makes that the default, for normal fw you have to specifically select it. Maybe making it the default might be a good idea.

Unfortunately, BLDC main branch firmware still gives different detected motor values.... I can ride well with the firmware but the detection is not ok :-( I did a few tests to make sure that is the case.