[BUG] ESP32 current sensing readings not synced with PWM - Not reproducible

[byDagor]

Describe the bug
Sometime after SimpleFOC2.2 a bug was introduced that made the phase current reading bad, the symptoms were bad stability and noisy FOC Torque mode. The phase currents looked discontinued instead of a smooth sinusoidal wave.

Describe the hardware setup
For us it is very important to know what is the hardware setup you're using in order to be able to help more directly

• Motor: 5010 brushless
• Driver: Dagor Brushless controller
• MCU: ESP32
• Position sensor: AS5147
• Current sensing: 40x gain, 0.002mohm sense resistors, phases A and B
• Arduino IDE

Narrowed down the bug to this function:

// Read currents when interrupt is triggered
static void IRAM_ATTR mcpwm0_isr_handler(void*){
  // // high side
  // uint32_t mcpwm_intr_status = MCPWM0.int_st.timer0_tez_int_st;
  
  // low side
  uint32_t mcpwm_intr_status = MCPWM0.int_st.timer0_tep_int_st;
  if(mcpwm_intr_status){
    adc_buffer[0][adc_read_index[0]] = adcRead(adc_pins[0][adc_read_index[0]]);
    adc_read_index[0]++;
    if(adc_read_index[0] == adc_pin_count[0]) adc_read_index[0] = 0;
  }
  // low side
  MCPWM0.int_clr.timer0_tep_int_clr = mcpwm_intr_status;
  // high side
  // MCPWM0.int_clr.timer0_tez_int_clr = mcpwm_intr_status_0;
}

Different from the previous implementation, timer1 does not appear to be used. This produces discontinuous current measurements in a center aligned PWM 3 or 6 driver because all readings will only be synced to phase A.

I wrote a simple patch that works for my two phase system, it's MISSING the logic to understand if a third phase is being used.

// Read currents when interrupt is triggered
static void IRAM_ATTR mcpwm0_isr_handler(void*){
  // // high side
  // uint32_t mcpwm_intr_status = MCPWM0.int_st.timer0_tez_int_st;

  // low side
  uint32_t mcpwm_intr_status_0 = MCPWM0.int_st.timer0_tep_int_st;
  uint32_t mcpwm_intr_status_1 = MCPWM0.int_st.timer1_tep_int_st;
  //uint32_t mcpwm_intr_status_2 = MCPWM0.int_st.timer2_tep_int_st;

  if(mcpwm_intr_status_0 && currentState == 0){
    currentState = 1;
    adc_buffer[0][adc_read_index[0]] = adcRead(adc_pins[0][adc_read_index[0]]);
    adc_read_index[0]++;
    if(adc_read_index[0] == adc_pin_count[0]) adc_read_index[0] = 0;
  }else if(mcpwm_intr_status_1 && currentState == 1){
    currentState = 0;
    adc_buffer[0][adc_read_index[0]] = adcRead(adc_pins[0][adc_read_index[0]]);
    adc_read_index[0]++;
    if(adc_read_index[0] == adc_pin_count[0]) adc_read_index[0] = 0;
  }

  // low side
  MCPWM0.int_clr.timer0_tep_int_clr = mcpwm_intr_status_0;
  MCPWM0.int_clr.timer1_tep_int_clr = mcpwm_intr_status_1;
  //MCPWM0.int_clr.timer2_tep_int_clr = mcpwm_intr_status_2;
  // high side
  // MCPWM0.int_clr.timer0_tez_int_clr = mcpwm_intr_status_0;
  // MCPWM0.int_clr.timer1_tez_int_clr = mcpwm_intr_status_1;
  // MCPWM0.int_clr.timer2_tez_int_clr = mcpwm_intr_status_2;
}

And changing the following function to ensure we use timer1:

void _driverSyncLowSide(void* driver_params, void* cs_params){

  mcpwm_dev_t* mcpwm_dev = ((ESP32MCPWMDriverParams*)driver_params)->mcpwm_dev;
  mcpwm_unit_t mcpwm_unit = ((ESP32MCPWMDriverParams*)driver_params)->mcpwm_unit;

  // low-side register enable interrupt
  mcpwm_dev->int_ena.timer0_tep_int_ena = true;//A PWM timer 0 TEP event will trigger this interrupt
  mcpwm_dev->int_ena.timer1_tep_int_ena = true;//A PWM timer 0 TEP event will trigger this interrupt
  //mcpwm_dev->int_ena.timer2_tep_int_ena = true;//A PWM timer 0 TEP event will trigger this interrupt
  // high side registers enable interrupt
  //mcpwm_dev->int_ena.timer0_tep_int_ena = true;//A PWM timer 0 TEZ event will trigger this interrupt

  // register interrupts (mcpwm number, interrupt handler, handler argument = NULL, interrupt signal/flag, return handler = NULL)
  if(mcpwm_unit == MCPWM_UNIT_0)
    mcpwm_isr_register(mcpwm_unit, mcpwm0_isr_handler, NULL, ESP_INTR_FLAG_IRAM, NULL);  //Set ISR Handler
  else
    mcpwm_isr_register(mcpwm_unit, mcpwm1_isr_handler, NULL, ESP_INTR_FLAG_IRAM, NULL);  //Set ISR Handler
}

[askuric]

Hey @byDagor ,
I'm having trouble reproducing this behavior, what is the arduino-esp32 version that you're using?

[askuric]

So I've done some testing on this issue today and I'm having trouble understanding where the problem comes from and why this new code solves it. :D

Basically, as the phases A,B and C of the motor (both 3pwm and 6pwm) are synced, there is no need to do interrupt for every one of them separately, one is enough. Additionally, eve if you enable interrupts on all of them they are just gonna fire at the same time (but they are gonna fire the same interrupt calling the same function only once).
I've tested this in my code and both in case of enabling one or two phases the interrupt is called only once per pwm cycle, but the status flags mcpwm_intr_status_0 and mcpwm_intr_status_1 are both in 1 if both phase interrupts are enabled.

Scope testing

I've so far only tested on my scope.
So what I can see in my scope, both your new implementation and the old one have exactly the same behavior. Updating the same registers and at the same time.

Here is the comparison on by scope

• cyan line is the PWM at 0.5 duty cycle at 15kHz
• yellow is the call of the interrupt (twice per cycle) - just like an impulse when the mcpwm0_isr_handler is called.
• blue is the reading of the adcRead for phase current 1
• violet is the reading of the adcRead for phase current 2
  Old code:
  

New code:

Potential explanations

The adcRead function takes around 10us to read the current value which ic pretty terrible. For the pwm frequencies above 20kHz the low-side current sensing will not work well. So if you've been using it with more than that the current readings have probably been done in some part during the part of the PWM where the high side is on.

Here is the 20kHz PWM - for 0.5 duty cycle (the center) its already almost entering the high-side active part of the duty cycle - this is not good. For any non-zero phase voltage the adcRead will enter this period.

And 25kHz PWM, it is even worse. Even for 0 phase voltage the adcRead enters the high-side active PWM period.

I have no idea why the new code has solved the issue though.
This issue will need further investigation.