Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add linear hall position sensor driver #12

Closed
wants to merge 1 commit into from

Conversation

nanoparticle
Copy link

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.

@runger1101001
Copy link
Member

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?

@nanoparticle
Copy link
Author

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?

@runger1101001
Copy link
Member

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...

@runger1101001
Copy link
Member

Also, did you see this one: simplefoc/Arduino-FOC#197

It's a similar implementation, I think...

@dekutree64
Copy link
Contributor

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);
}

@dekutree64
Copy link
Contributor

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?

@runger1101001
Copy link
Member

Hey @dekutree64 , you're absolutely right, and we're keenly aware of this problem. The ultimate plan looks like this:

  • the ADC drivers for the different hardwares will be refactored and abstracted by something called "ADCEngine". ADCEngine will have different capability levels on the different hardwares. On simpler hardwares it may only support the inline current sensing of one motor. But on complex hardware (like STM32, Renesas) the goal is to make it support multiple motors, multiple ADCs, current sensing, voltage sensing and other ADC use in parallel
  • So in future it should be possible for the LinearHallSensor to request some ADC channels from the ADCEngine, and either get them or get an err - not supported. Those ADC channels would be faster than using analogRead() so your use-case should be possible.
  • There's lots of other use cases like VBUS sensing, OC protection, Voltage sense, etc which would also need this functionality

@XieMaster
Copy link

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.

dekutree64 added a commit to dekutree64/Arduino-FOC that referenced this pull request Jan 14, 2024
Changes compared to the original pull request in the drivers repository simplefoc/Arduino-FOC-drivers#12
1. Added a version of init which turns the motor one revolution to find the center values of the sensors.
2. Moved the calls to analogRead into a weakly bound function ReadLinearHalls so it can be overridden with custom ADC code on platforms with poor analogRead performance.
3. Commented out the pinMode calls in init, which makes it possible to pass in ADC channel numbers for custom ReadLinearHalls to use without having to remap them every update.
4. Changed to use the much faster _atan2 function that was added to foc_utils recently.
5. Added examples.
dekutree64 added a commit to dekutree64/Arduino-FOC-drivers that referenced this pull request Feb 14, 2024
Changes compared to the original pull request simplefoc#12

1. Added a version of init which turns the motor one revolution to find the center values of the sensors.
2. Moved the calls to analogRead into a weakly bound function ReadLinearHalls so it can be overridden with custom ADC code on platforms with poor analogRead performance.
3. Commented out the pinMode calls in init, which makes it possible to pass in ADC channel numbers for custom ReadLinearHalls to use without having to remap them every update.
4. Changed to use the much faster _atan2 function that was added to foc_utils recently.
@runger1101001
Copy link
Member

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?

@nanoparticle
Copy link
Author

Thanks so much to everyone for pulling this over the finish line 👍

SwapnilPande pushed a commit to Every-Flavor-Robotics/Arduino-FOC-drivers that referenced this pull request Aug 8, 2024
Changes compared to the original pull request simplefoc#12

1. Added a version of init which turns the motor one revolution to find the center values of the sensors.
2. Moved the calls to analogRead into a weakly bound function ReadLinearHalls so it can be overridden with custom ADC code on platforms with poor analogRead performance.
3. Commented out the pinMode calls in init, which makes it possible to pass in ADC channel numbers for custom ReadLinearHalls to use without having to remap them every update.
4. Changed to use the much faster _atan2 function that was added to foc_utils recently.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants