Improving a homemade sensorless BLDC motor driver.

[Refrigerator]

Recently i finished making a homemade BLDC motor driver and it works quite well.
But when adding features i noticed that there's something not right with it.
The project was based on this project here: https://simple-circuit.com/arduino-sensorless-bldc-motor-controller-esc/
Except that i've changed most of the code because i'm using an Attiny88 instead of a Atmega328.
The Attiny only has two HW PWM outputs and in my project i've only used one of them.
To multiplex the single PWM output i connected the PWM pin though 2k resistors to other three GPIO pins, which operate as open collector outputs to control the PWM. But that's not the problem.
The problem comes with the BEMF sensing. At first i noticed that my button polling wasn't working and quickly realized that it's because my analog comparator interrupt is taking too long.
The A-Comp interrupt routine was taken straight from the project mentioned above and the cause of the interrupt taking too long is the "debouncing" part:

Code: [Select]

// Analog comparator ISR
ISR (ANALOG_COMP_vect) {
  // BEMF debounce
  for(i = 0; i < 10; i++) {
    if(bldc_step & 1){
      if(!(ACSR & 0x20)) i -= 1;
    }
    else {
      if((ACSR & 0x20))  i -= 1;
    }
  }
  bldc_move();
  bldc_step++;
  bldc_step %= 6;
}The way that the debouncing in the ISR works is that a for() loop cycles a certain number of times and if the output of the analog comparator is not as expected the loop is stalled. But, for me, the loop is almost permanently stalled and i want to fix this my way, because the borrowed code isn't working for me.

So in summary what i'm doing in my code right now:
1. Comparator is set to trigger on any edge*.
2. When it triggers the ISR is called.
3. The A-Comp interrupt is immediately disabled.
4. The "debouncing" bit takes place.
5. Outputs get commutated.
6. A-Comp inputs get changed.
7. A-Comp interrupt is enabled again.

*The reason why i trigger my interrupt on any comparator edge is that if noise appears then both falling and rising edges are close together, so it makes no difference in the end.

For my controller to work i need to find a way to reject noise. There are two main sources of noise.
First is the switching noise that appears at the beginning of the BEMF sensing period, and second is random noise in the form of voltage spikes.
In a Ti application note^[1] their switching noise was addressed by simply ignoring it for a set period. But this is problematic because the period needs to be shorter for higher speeds. And if i make the period proportional to speed then it's a positive feedback loop for more noise where: noise artificially increases speed measurement -> noise rejection period is decreased as result -> the probability of noise passing through is higher.
In the same application note the random noise was addressed by simply adding a capacitor in the volatge divider to make an RC filter.
This RC filter approach seems to be common among many other application notes, including Microchip, NXP etc.

I'm thinking for starters, if the output of the A-comp is not as expected then to just exit the ISR ASAP. But if all unexpected outputs are bad then all expected outputs are good? Nope. I think there also needs to be some delay after exit. Since for a given voltage spike there will be an opposing edge right after the first trigger. But this would only work for voltage spikes going the "wrong" way.

Ok let's take a step back. In the same Ti application note it is made clear that the BEMF zero crossing occurs 30^o before the switching event needs to happen. This means that i have 30^o of time to determine if my BEMF crossing was correct or not. I think that i could trigger two SW timers for this.
First timer would count the 30^o that i need to delay my switching by. And the second timer would count the length of the comparator pulse received.
If the comparator pulse is impossibly short, then the degree timer would be reset and maybe halted completely. This would reject both positive and negative spikes, making the above mentioned "some delay" unnecessary.
The thing is that, ultimately, the number of these false triggers will be highest near the zero crossing. Which is why i'm not sure i would halt the 30^o timer. And in the end i would be doing the same as the debouncing loop in the original code, except in a more roundabout way.

I'm thinking that, regardless of what the noise detection does, the 30^o timer should save the value of the A-comp for each degree. At the end of the 30^o period, if the result of the counted A-comp values is overwhelmingly as-expected (let's say above 90-95%) then it would be assumed that the zero crossing is valid and switching is permitted.
But then i'm not sure what to do if it's not. Maybe delay by 1^o for not more than 10^o until the result passes?
And if the result still doesn't pass, then assume that the motor is out of sync and disable all outputs? 
Also the whole counting degrees thing relies on speed measurement, which relies on accurately detecting the zero crossing so it's like a catch-22.

What do you guys think? Do you have any tips or maybe some links to any resources?


[1] https://www.ti.com/lit/an/spra498/spra498.pdf?ts=1659362622694&ref_url=https%253A%252F%252Fwww.google.com%252F

[thm_w]

I feel like filtering might help. JY02 for example is using R/CR filters, similar to the TI document you linked.
I don't think its ideal to sit around and debounce stall inside the ISR itself.
What min and max RPM are you expecting to operate at?

https://www.researchgate.net/figure/RC-Filter-and-Comparator-Connected-For-the-Improvement-of-the-Back-EMF_fig2_290085941
https://ww1.microchip.com/downloads/en/AppNotes/01083a.pdf

[Refrigerator]

I feel like filtering might help. JY02 for example is using R/CR filters, similar to the TI document you linked.
I don't think its ideal to sit around and debounce stall inside the ISR itself.
What min and max RPM are you expecting to operate at?

https://www.researchgate.net/figure/RC-Filter-and-Comparator-Connected-For-the-Improvement-of-the-Back-EMF_fig2_290085941
https://ww1.microchip.com/downloads/en/AppNotes/01083a.pdf

I have a driver with the JY02 and it didn't seem to cope with my motor very well. 
During testing IIRC i saw my lower gate switch at 270-ish Hz, so that comes out at about 810 ERPM.
So let's say my expected speed is 1k ERPM max.

Also while playing around i noticed that my a-comp ISR doesn't always trigger.
Currently i have a bit of code purely for testing the a-comp ISR and in this code:
1. A-comp ISR turns on LED.
2. Disables a-comp interrupt.
3. Clears SW timer.
4. Exits ISR.

At the same time the timer1 ISR does this:
1. If counter is below a certain value - increment
2. If counter is above a certain value:
   2. A. Turn LED off.
   2. B. Enable a-comp interrupt.

This code basically turns an LED on for a set period every time the comparator ISR triggers. But the ISR does not always trigger. 
The a-comp interrupt is only enabled when the LED is turned off but the interrupt is enabled far ahead of the next expected trigger.
Yet in the scope capture it can clearly be seen that some pulses are missing.
Now granted, my scope only has 2 channels so i can't probe the other 2 phases of the motor, to see if they're adding any noise or not.
But even then i would expect the comparator interrupt to trigger. Even if the trigger happens during the timer1 ISR there should still be a raised flag to trigger the a-comp ISR after the timer1 ISR is complete. 

[Refrigerator]

The BLDC motor is driven externally at a constant speed with a small geared induction motor.

[thm_w]

1krpm is not that bad.

Is that a scooter wheel hub motor or a hoverboard motor? It doesn't have built in hall sensors?

[Refrigerator]

1krpm is not that bad.

Is that a scooter wheel hub motor or a hoverboard motor? It doesn't have built in hall sensors?

The goal is to run a sensorless 250W mid-drive e-bike motor.

But the motor in the pic is a 350W hoverboard motor that i use for testing because it's convenient.

[Refrigerator]

A simple delay works without any missed pulses but i don't want delays in my code.

[Refrigerator]

I've isolated the timer1 interrupt as the culprit of the skipped pulses.
Not sure how to fix it at this point.
I've commented out the lines that disable the a-comp interrupt so there's no more disable-enable action taking place.
It seems to me like the timer 1 interrupt is blocking the comparator interrupt.

[Refrigerator]

Ok i found out what's up.
First i found this, which is expected:

All interrupts have a separate interrupt vector in the interrupt vector table. The interrupts have priority in accordance
with their interrupt vector position. The lower the interrupt vector address, the higher the priority.

Then i found this, which is expected:

Vector No. Program Address Source Interrupt Definition
1 0x000 RESET External/power-on/brown-out/watchdog reset
2 0x001 INT0 External interrupt request 0
3 0x002 INT1 External interrupt request 1
4 0x003 PCINT0 Pin change interrupt request 0
5 0x004 PCINT1 Pin change interrupt request 1
6 0x005 PCINT2 Pin change interrupt request 2
7 0x006 PCINT3 Pin Change Interrupt Request 3
8 0x007 WDT Watchdog time-out interrupt
9 0x008 TIMER1_CAPT Timer/Counter1 capture event
10 0x009 TIMER1_COMPA Timer/Counter1 compare match A
11 0x00A TIMER1_COMPB Timer/Counter1 compare match B
12 0x00B TIMER1_OVF Timer/Counter1 overflow
13 0x00C TIMER0_COMPA Timer/Counter0 compare match A
14 0x00D TIMER0_COMPB Timer/Counter0 compare match B
15 0x00E TIMER0_OVF Timer/Counter0 overflow
16 0x00F SPI_STC SPI serial transfer complete
17 0x010 ADC ADC conversion complete
18 0x011 EE_RDY EEPROM ready
19 0x012 ANA_COMP Analog comparator
20 0x013 TWI 2-wire serial interface

But that still didn't explain why the comparator interrupt was being blocked. Priority is lower, yes, but the interrupt flag should still be there.
So i found the key bit of information:

When an interrupt occurs, the global interrupt enable I-bit is cleared and all interrupts are disabled. The user software can
write logic one to the I-bit to enable nested interrupts. All enabled interrupts can then interrupt the current interrupt routine.
The I-bit is automatically set when a return from interrupt instruction – RETI – is executed.

So basically what happens is that each interrupt will globally disable all interrupts while it's running. 
But then what's the point of interrupt priority level? Or does interrupt priority not matter unless i manually re-enable interrupts?

Anyways, looks like i just need to write sei(); at the beginning of my timer interrupt. Would be nice if i could disable this interrupt disabling thingy, so i'll keep looking for that.

Edit: nope, this "The user software can write logic one to the I-bit to enable nested interrupts." means that there's not "enable nested interrupts" bit to set. 

[eugene]

My first reaction when I looked at the code in the opening post was that one shouldn't be doing that much inside an ISR. What you probably should do is just set a flag in the ISR, then move all the code to your main loop inside an if(flag) code block.

Honestly, I don't know enough about the timing requirements of sensorless BLDC, but that change will prevent the ANALOG_COMP ISR from blocking the other interrupts.

[Refrigerator]

My first reaction when I looked at the code in the opening post was that one shouldn't be doing that much inside an ISR. What you probably should do is just set a flag in the ISR, then move all the code to your main loop inside an if(flag) code block.

Honestly, I don't know enough about the timing requirements of sensorless BLDC, but that change will prevent the ANALOG_COMP ISR from blocking the other interrupts.

In the same code they change analog comparator inputs without disabling the IRQ first. 

[macboy]

...
But that still didn't explain why the comparator interrupt was being blocked. Priority is lower, yes, but the interrupt flag should still be there.
So i found the key bit of information:

When an interrupt occurs, the global interrupt enable I-bit is cleared and all interrupts are disabled. The user software can
write logic one to the I-bit to enable nested interrupts. All enabled interrupts can then interrupt the current interrupt routine.
The I-bit is automatically set when a return from interrupt instruction – RETI – is executed.

So basically what happens is that each interrupt will globally disable all interrupts while it's running. 
But then what's the point of interrupt priority level? Or does interrupt priority not matter unless i manually re-enable interrupts?
...

That is a normal way for interrupts to work on microcontrollers.

Interrupt priority ensures that when two interrupts are waiting to be serviced, then the higher priority one will be serviced first, so its ISR will be called, and when that returns, the next highest priority pending interrupt's ISR is called, and so on, until all pending interrupts are serviced. Then execution at 'task' level continues. You might wonder how two interrupts will fire at exactly the same time, but this would usually happen while inside another ISR, since any interrupt that fires won't be serviced immediately, but must wait for interrupts to be re-enabled. Another situation is when your task level code disables interrupts for a short period of time for some good reason, such as atomic access to a large-size variable that might be altered by an ISR, like a timestamp or event counter. Note that when you re-enable interrupts (nested interrupts), any new interrupt, even a lower priority one, can interrupt the current running ISR! You can work around that by manipulating the mask/enable bits for other interrupts, but that gets complicated and risky quite quickly.

Nested interrupts can be very useful, especially if the ISR absolutely must do more than a small amount of work before returning. This might reduce interrupt latency for other interrupts which might need to respond quickly to some real-time event. Your long-running ISR can do its real-time work immediately (capturing some status, like a port state, some timer value, some other register, etc.), then enable interrupts, and continue with the longer work. It is very important that the interrupt source is cleared, and might (likely) be important that the interrupt source for this ISR is disabled before globally re-enabling interrupts, or you will be in a lot of pain trying to debug. Most ISRs won't be written to be re-entrant. You need to have a good understanding of when and how often you expect your various interrupts to fire, when they need to be enabled, how long and when they can be disabled, etc., before you can even consider using nested interrupts.

[eugene]

My first reaction when I looked at the code in the opening post was that one shouldn't be doing that much inside an ISR. What you probably should do is just set a flag in the ISR, then move all the code to your main loop inside an if(flag) code block.

Honestly, I don't know enough about the timing requirements of sensorless BLDC, but that change will prevent the ANALOG_COMP ISR from blocking the other interrupts.

In the same code they change analog comparator inputs without disabling the IRQ first. 

You still don't need all of that code to be in the ISR. Inside the ISR, set a flag to true. There's no need to disable the interrupt. It can fire over and over again without the flag becoming more true. Once the code (in your main loop) is done, set the flag to false effectively enabling that interrupt.

Seriously, that's WAY TOO MUCH code to have in an ISR. There's a call to some function bldc_move()! There should never be a function call inside an ISR. Figure out a different way to do it. There's always a way.

[Refrigerator]

I'm rewriting the code for the interrupts and have come to this flag-based solution:

Code: [Select]

//**********************************************************************
/*
 */
// Timer 1 overflow ISR
ISR (TIMER1_OVF_vect) {
  sei();
  timer_trig = true;
}


//**********************************************************************
// Analog comparator ISR
ISR (ANALOG_COMP_vect) {
  ACSR &= ~(1 << ACIE); // disable comparator interrupt
  a_comp_trig = true;
} // end of AC ISR


//**********************************************************************
void loop() {
  // put your main code here, to run repeatedly:
  if (timer_trig){
    if (skip_counter == 626){
      skip_counter ++;
      PORTD &= ~(1 << PD0); // LED off
      ACSR |= 0x08;                    // Enable analog comparator interrupt
    }
    if (skip_counter < 626){
       skip_counter ++;
    }
    timer_trig = false;
   
  }
  if (a_comp_trig){
    skip_counter = 0;
    PORTD |= (1 << PD0); // LED on
    a_comp_trig = false;
  }
 
}
Seems to work fine, i wasn't able to spot any missed pulses and all pulses seem to be equal in length.
Previously when i had changed the code there seemed to still be missed pulses, yet i could not trigger on extended low pulses.
As it turned out my interrupts were working but my pulse timing would fail occasionally, producing a ~120ns long pulse.
These problems went away with the new code.
Next step will be to add speed measurement, and i'm thinking of a rolling average to even out any variances.

[Refrigerator]

Seems like my Attiny88 has gone completely inert and i don't have a spare one so that marks the end of this project i guess 

[thm_w]

Seems like my Attiny88 has gone completely inert and i don't have a spare one so that marks the end of this project i guess 

How long from China to Lithuania?
https://www.aliexpress.com/item/1005003652746229.html

[Refrigerator]

Seems like my Attiny88 has gone completely inert and i don't have a spare one so that marks the end of this project i guess 

How long from China to Lithuania?
https://www.aliexpress.com/item/1005003652746229.html

That's actually the exact same listing i bought mine from. 
I do have some Atmega328 clones (LGT8F328P) on the way. Atmega 328p is basically the same thing, with an extra timer i think.

[Siwastaja]

That is a normal way for interrupts to work on microcontrollers.

A clarification, though: it's the "normal" way for simple/cheap/old microcontrollers. With modern day 32-bit parts, you can pretty much expect/demand a truly nested i.e. pre-emptive interrupt controller, which allows interrupts with same priority to run one after each other, but higher priority interrupts interrupt already running lower priority ones. This makes design of code - and analysis of worst case timing - so much easier. You can even just do whatever needs to be done within the ISR without need to set flags, because more urgent interrupt sources can still interrupt that code.

People tend to say that core does not matter, only peripherals, but I tend to disagree because I have found the interrupt system in ARM Cortex-M series much more powerful than 8-bit AVR, for example, allowing one to write code with fewer "tricks", and more predictable performance.

[Refrigerator]

That is a normal way for interrupts to work on microcontrollers.

A clarification, though: it's the "normal" way for simple/cheap/old microcontrollers. With modern day 32-bit parts, you can pretty much expect/demand a truly nested i.e. pre-emptive interrupt controller, which allows interrupts with same priority to run one after each other, but higher priority interrupts interrupt already running lower priority ones. This makes design of code - and analysis of worst case timing - so much easier. You can even just do whatever needs to be done within the ISR without need to set flags, because more urgent interrupt sources can still interrupt that code.

People tend to say that core does not matter, only peripherals, but I tend to disagree because I have found the interrupt system in ARM Cortex-M series much more powerful than 8-bit AVR, for example, allowing one to write code with fewer "tricks", and more predictable performance.

Yeah i've been playing with STM and Atmel ARM processors and their NVIC makes things so easy.
I guess this brings me new appreciation for the NVIC module in those 
The flag approach works well enough so i think i might continue on with it once i receive my micros, wherever they may be at this point.
Meanwhile i might make myself a little single phase VFD to play around with. I have some ST32 blue pill clones sitting around.