Closed nanoparticle closed 7 months ago
Hey @nanoparticle, how do you feel about this? There was a SimpleFOC release today, including the changes to the init() code that you wanted... how about this code? is it ready to merge to the drivers?
I haven't had time to do any real testing on this code beyond seeing if it is a feasible method of sensing. How much testing should it go through before being merged?
Hey @nanoparticle do we want to merge this? It's still in draft status...
For the drivers repository, the bar is not set very high - you don't have to test much. If it runs in some of your own setups it is enough... each folder in the drivers repository has its own README, so we can add a warning note for users to let them know it isn't tested much...
Also, did you see this one: https://github.com/simplefoc/Arduino-FOC/pull/197
It's a similar implementation, I think...
This gets my stamp of approval! But I would rather it be placed in the main SimpleFOC sensors folder rather than drivers.
I recommend adding a way to auto-calibrate the center values without having to run a separate program and copy them over by hand. I did it by moving the centerA and centerB arguments to init instead of the constructor, and adding a second version of init which takes a FOCMotor*. But this requires calling motor.init before sensor.init so it can use setPhaseVoltage. It doesn't cause any problems, but if Antun prefers the sensor init to always go before motor init, then we could add a hook for custom sensor calibration routines. It would be useful for digital halls too (the zero offset found by alignSensor works for trapezoid120, but sinePWM and SmoothingSensor need manual tuning).
I've adapted the center search code from XieMaster's version that runger linked above, with a modification to the angle calculation:
if (!motor->enabled) {
SIMPLEFOC_DEBUG("LinearHall::init failed. Call after motor.init, but before motor.initFOC.");
return;
}
pinMode(pinA, INPUT);
pinMode(pinB, INPUT);
int rawA, rawB, minA, maxA, minB, maxB;
rawA = minA = maxA = centerA = analogRead(pinA);
rawB = minB = maxB = centerB = analogRead(pinB);
// move one mechanical revolution forward
for (int i = 0; i <= 2000; i++)
{
float angle = _3PI_2 + _2PI * i * pp / 2000.0f;
motor->setPhaseVoltage(motor->voltage_sensor_align, 0, angle);
rawA = analogRead(pinA);
if (rawA < minA)
minA = rawA;
if (rawA > maxA)
maxA = rawA;
centerA = (minA + maxA) / 2;
rawB = analogRead(pinB);
if (rawB < minB)
minB = rawB;
if (rawB > maxB)
maxB = rawB;
centerB = (minB + maxB) / 2;
_delay(2);
}
//establish initial reading for rollover handling
electrical_rev = 0;
prev_reading = atan2f(rawA - centerA, rawB - centerB);
}
Upon further investigation, it looks like ADC configuration is a major problem for this sensor approach. As it is, I'm getting about 250 microseconds per analogRead on STM32G031, which is too slow for real world use. Current sense apparently requires an elaborate HAL configuration sequence for every supported platform to get good ADC performance, and separate ones for each STM32 series. That would be a maintenance nightmare to add that many files just for this one sensor. What to do?
Hey @dekutree64 , you're absolutely right, and we're keenly aware of this problem. The ultimate plan looks like this:
Yes, ADC sampling is too slow, so this linear Hall angle feedback method is more suitable for use in lower-speed projects such as pan/tilts. Only a 1-pole pair magnet and two linear Halls need to be installed to replace expensive magnetic encoders.
I have just merged deku's implementation to the drivers repository dev branch, it will be part of the next release... do you all agree we can close this PR (which is still in draft status) in favor of the merged implementation?
Thanks so much to everyone for pulling this over the finish line 👍
Not entirely sure where the driver files should go, or how exactly the includes are structured, but the basic functionality is here and superficially tested.