simplefoc / Arduino-FOC

Arduino FOC for BLDC and Stepper motors - Arduino Based Field Oriented Control Algorithm Library
https://docs.simplefoc.com
MIT License
1.94k stars 510 forks source link

initFOC: remove unnecessary delay #370

Closed Schnilz closed 5 months ago

Schnilz commented 5 months ago

Hey, thanks for writing simpleFOC. It is amazing how easy you made it conquer the otherwise complex topic of motor control!

I looked into skipping the direction and zero calibration done in initFOC with the find_sensor_offset_and_direction.ino example. This worked nicely but the function still block for a whole second which i didn't get. From looking a the code i don't know why the delay is needed so i removed it, and my driver-code still works (but "boots" faster). If it is needed for the calibration or something, I would suggest moving it in the if statements so it gets skipped when not needed.

Thanks!

runger1101001 commented 5 months ago

Hey, thanks for your nice feedback, and thanks for the PR!

You're not the first to observe this, and also not the first to question it - I have done so myself.

These delays have been in the code since before I first started to help on it. Their purpose isn't completely clear to me.

The first delay, on line 154, could easily be added by the user if needed. My guess as it its purpose is that is allows the driver hardware, which was presumably enabled just before in motor.init(), has a chance to start up. 500ms is very long for this, in my experience most drivers have a wake up time measured in microseconds. But then I only work with little motors, it might be different for large inverters. In any case, the user could add the correct amount of delay themselves.

The second delay, in line 168, if I were to guess its purpose then it is to allow the motor to settle after the sensor alignment, before starting the current sense alignment. But there is already a delay in the sensor alignment function, having both is a little confusing. It's also not clear it's needed at this point, if there was no sensor alignment for example.

So I tend to agree with you that both can be removed, but its also clear that this will change the behaviour of SimpleFOC for all users, so even though the code change is tiny, I consider the impact to be large.

I'd like to check with Antun what he thinks about removing them.

askuric commented 5 months ago

Hi guys,

This is perfectly reasonable. I think that these delays are a remainder from the earlier code where the sensor and current alignement were implemented within initFOC and were not handled within their own functions.

I do agree with Richard's conclusions here and I am happy to merge this PR. We might consider also doing a pass over the other delays in the library to potentially reduce them. Especially since some of them have been set in the original Arduino UNO based code.

runger1101001 commented 5 months ago

Then lets merge it :-)