simplefoc / Arduino-FOC

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

[BUG] Crash in STM32 driver initialization with multiple motors #437

Open dekutree64 opened 1 month ago

dekutree64 commented 1 month ago

This bit of code in stm32_mcu.cpp reads off the end of the pinTimers array if there is more than one motor. It doesn't always crash, which made it very difficult to narrow down where the problem was.

int scoreCombination(int numPins, PinMap* pinTimers[]) {
  // check already used - TODO move this to outer loop also...
  for (int i=0; i<numTimerPinsUsed; i++) {
    if (pinTimers[i]->peripheral == timerPinsUsed[i]->peripheral
        && STM_PIN_CHANNEL(pinTimers[i]->function) == STM_PIN_CHANNEL(timerPinsUsed[i]->function))
      return -2; // bad combination - timer channel already used
  }
  ...

I think this is the proper fix for it, but hopefully whoever wrote the original can verify that the logic is correct.

  // check already used - TODO move this to outer loop also...
  for (int i=0; i<numPins; i++) {
    for (int j=0; j<numTimerPinsUsed; j++) {
      if (pinTimers[i] != NP && pinTimers[i]->peripheral == timerPinsUsed[j]->peripheral
          && STM_PIN_CHANNEL(pinTimers[i]->function) == STM_PIN_CHANNEL(timerPinsUsed[j]->function))
        return -2; // bad combination - timer channel already used
    }
  }
runger1101001 commented 1 month ago

ooh, thanks for spotting this! Definitely a bug :-(

I think we will switch to my new "HAL only" PWM driver for all the STM32 MCUs (including Arduino Portenta) in our next release.

In the "HAL only" branch, I think the error is already fixed: https://github.com/runger1101001/Arduino-FOC/blob/4d8fa4a6c15c3bce15d536235e0516851359fb34/src/drivers/hardware_specific/stm32/stm32_searchtimers.cpp#L19

runger1101001 commented 1 month ago

Fixed in https://github.com/simplefoc/Arduino-FOC/pull/430