ATmega328p - Software PMW disturbed by PCINT interrupt

Go To Last Post
14 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello,

 

This is my first post on this forum, maybe you will help me on that! Sorry for my english...

 

I did a software PMW using the 16bits timer1 to control 4 brushless motors, which works very well. Here is "a part" of the code I use:

 

volatile unsigned int servo[4] = {700, 700, 700, 700}; //Initial speed in microseconds
volatile int8_t channel = 1; //1, 2, 3 or 4 : the motor id to control

int main(void){
    
    TCCR1B |= 1<<CS11; //Prescaler of 8 because 8MHz clock source
    TIMSK1 |= (1<<OCIE1A); //Interrupt on OCR1A
    DDRD |= 1<<DDD1 | 1<<DDD2 | 1<<DDD3 | 1<<DDD4; //motor's ESC as output
	PORTD = 1<<channel; //Set first servo pin high
	
	sei(); //Enable global interrupts
	
	while(1){
	    servo[0] = 1200; //Time that PMW signal need to be high (between 700 and 2300us)
		servo[1] = 1200;
		servo[2] = 1200;
		servo[3] = 1200;
	}
	
}

//PMW ISR
ISR(TIMER1_COMPA_vect)
{
    uint16_t timerValue = TCNT1;
	if(channel < 0){ //Every motors was pulsed, waiting for the next period
		if(timerValue >= usToTicks(20000)){
			TCNT1 = 0;
			channel = 1;
			PORTD |= 1<<channel;
			OCR1A = servo[0];
		}
		else{
			OCR1A = usToTicks(20000);
		}
	}
	else{
		if(channel < 4){
			OCR1A = timerValue + servo[channel];
			PORTD &= ~(1<<channel); //Clear actual motor pin
			PORTD |= 1<<(channel + 1); //Set the next one
			channel++;
		}
		else{
			PORTD &= ~(1<<channel); //Clear the last motor pin
			OCR1A = usToTicks(20000);
			channel = -1; //Wait for the next period
		}
	}
}

I also receive from a RC command a 50Hz PMW signal on my PCINT3 port. I calculated the time of the PMW signal with this code, which works well too :

 

int main(void){
    
    PCICR |= 1<<PCIE0; //Enable interrupt of PCINT7:0
	PCMSK0 |= 1<<PCINT3; //Enable PCINT3
	
}

//PCINT interrupt
ISR(PCINT0_vect, ISR_NOBLOCK){
	uint16_t timerValue = TCNT1;
	
	uint8_t changedbits = PINB ^ portbhistory;
	portbhistory = PINB;
	
	//If interrupt is on PCINT3
	if( (changedbits & (1 << PB3)) )
	{

		//If PORTB3 is high, get the timerValue
		if(PINB & 1<<PORTB3){
			previousThrottle = timerValue;
		}
		//If PORTB3 is low, calculate the time it was high
		else{
			if(timerValue > previousThrottle){
				throttle = ticksToUs(timerValue - previousThrottle);
			}
			else{
				throttle = ticksToUs((usToTicks(20000) - previousThrottle) + timerValue);
			}
		}
	}
}

Now here is my problem.

If I only use the PMW part of code, I can run my 4 motors very well, without any issue.

If I only use PCINT part of code, I can read the exact time my RC Command generates a high PMW signal.

 

BUT : if I use both part of code to read the RC signal and command my motor depending on the RC signal, then my motors act weirdly. 80% of time they run well, but sometime (like once every second) the motor accelerated to maximal speed, or completely stop for a few millisecond.

 

I conclude that I certainly miss a PMW interrupt and so the timing is not respected anymore... but I am not sure and dont really know how to solve it. Maybe you can see something in my code that is wrong that I don't see...

 

Thanks!

Last Edited: Fri. Feb 20, 2015 - 12:50 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You have not set the COMnx bits in TCCR1A.  Nor have you selected a PWM mode.

 

If you are not using a real PWM mode,  the hardware does not update OCR1A at a 'safe' time.

And you generally never change TCNT1.   i.e. you use a fixed PWM period and alter the duty cycle with OCR1A and OCR1B.

 

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am using a "software" PMW, not hardware. That is why COMnx bits are keep at 0. That way I can generate PMW on any PORT's bit I want...

I have to change TCNT1 to control my period in that case...

 

Again, my PMW works very well if I remove the PCINT interrupt, so I don't think the problem is from the logic...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

BUT : if I use both part of code to read the RC signal and command my motor depending on the RC signal, then my motors act weirdly. 80% of time they run well, but sometime (like once every second) the motor accelerated to maximal speed, or completely stop for a few millisecond.

 

I conclude that I certainly miss a PMW interrupt and so the timing is not respected anymore... but I am not sure and dont really know how to solve it. Maybe you can see something in my code that is wrong that I don't see...

Well, certainly, you are doing a lot of stuff in the PCINT ISR.  That will throw off your soft PWM.

 

If the timer1 interrupt is most important, then do minimal work in the PCINT ISR, like set a flag for the mainline to do the calculations.

 

			if(timerValue > previousThrottle){
				throttle = ticksToUs(timerValue - previousThrottle);
			}
			else{
				throttle = ticksToUs((usToTicks(20000) - previousThrottle) + timerValue);
			}

I don't know if you have given us the whole picture...calculation of "throttle" value, no definition, and not used?  Are all accesses atomic throughout?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for the answer.

 

Something that I did not mention is that my PCINT ISR definition is : ISR(PCINT0_vect, ISR_NOBLOCK)

I use ISR_NOBLOCK so even if my PCINT routine is too long, I should go into the timer1 interrupt everytime I should, am I?

 

Here is the entire code, it could help you a bit more :

 

#include <avr/io.h>
#include <avr/interrupt.h>

//Convert microseconds to ticks
uint32_t usToTicks(uint32_t us);

//Convert ticks to microseconds
uint32_t ticksToUs(uint32_t ticks);

//Re-maps a number from one range to another
long map(long x, long in_min, long in_max, long out_min, long out_max);

//Constants
const float clockSourceMhz = 8.0f;
const uint8_t prescaler = 8; //Timer1 prescaler

//PMW Constants PMW in microseconds
const uint16_t rcMinUs = 1400;
const uint16_t rcMaxUs = 2000;
const uint16_t motorMinUs = 700;
const uint16_t motorMaxUs = 2300;

//Time in milliseconds since the program has begun
volatile uint32_t timeFromStartMs = 0;

//Initial motor speed (PMW high signal in microseconds)
volatile unsigned int servo[4] = {2300, 2300, 2300, 2300};
//Controlled motor ID number : 1, 2, 3 or 4
volatile int8_t channel = 1;

//Used to calculate the RC PMW lenght
volatile uint16_t previousThrottle = 0;
volatile uint8_t portbhistory = 0;

//RC commands
volatile int16_t throttleUs = 0; //Altitude control

int main(void){

	PCICR |= 1<<PCIE0; //Enable interrupt of PCINT7:0
	PCMSK0 |= 1<<PCINT3;
	
	TCCR1B |= 1<<CS11; //Prescaler of 8 because 8MHz clock source
	TIMSK1 |= (1<<OCIE1A); //Interrupt on OCR1A
	OCR1A = servo[0]; //Set the first interrupt to occur when the first pulse was ended
	
	DDRD |= 1<<DDD1 | 1<<DDD2 | 1<<DDD3 | 1<<DDD4; //Motors as output
	PORTD = 1<<channel; //Set first servo pin high
	
	sei(); //Enable global interrupts
	
	while(1){
		
		//ESC calibration
		if((timeFromStartMs > 2300) && (timeFromStartMs < 7000)){
			servo[0] = 700;
			servo[1] = 700;
			servo[2] = 700;
			servo[3] = 700;
		}
		
		//Motor controlled by RC command
		if(timeFromStartMs > 7000){
			int16_t computedThrottle = map(throttleUs, 1400, 2000, 700, 1400);
			servo[0] = computedThrottle;
			servo[1] = computedThrottle;
			servo[2] = computedThrottle;
			servo[3] = computedThrottle;
		}
	}

	return 0;
}

//PMW Building ISR
ISR(TIMER1_COMPA_vect)
{
	uint16_t timerValue = TCNT1;
	if(channel < 0){ //Every motors was pulsed, waiting for the next period
		if(timerValue >= usToTicks(20000)){
			TCNT1 = 0;
			channel = 1;
			PORTD |= 1<<channel;
			OCR1A = servo[0];
			timeFromStartMs += 20;
		}
		else{
			OCR1A = usToTicks(20000);
		}
	}
	else{
		if(channel < 4){
			OCR1A = timerValue + servo[channel];
			PORTD &= ~(1<<channel); //Clear actual motor pin
			PORTD |= 1<<(channel + 1); //Set the next one
			channel++;
		}
		else{
			PORTD &= ~(1<<channel); //Clear the last motor pin
			OCR1A = usToTicks(20000);
			channel = -1; //Wait for the next period
		}
	}
}


ISR(PCINT0_vect, ISR_NOBLOCK){
    
    uint16_t timerValue = TCNT1;
    	
	uint8_t changedbits;

	changedbits = PINB ^ portbhistory;
	portbhistory = PINB;
	
	//PCINT3 changed
	if( (changedbits & (1 << PB3)) )
	{
		//Is now high
		if(PINB & 1<<PORTB3){
			previousThrottle = timerValue;
		}
		//Is now low
		else{
			if(timerValue > previousThrottle){
				throttleUs = ticksToUs(timerValue - previousThrottle);
			}
			else{
				throttleUs = ticksToUs((usToTicks(20000) - previousThrottle) + timerValue);
			}
			
			//Constrain value between motorMaxUs and motorMinUs
			if(throttleUs > motorMaxUs){
				throttleUs = motorMaxUs;
			}
			else if(throttleUs < motorMinUs){
				throttleUs = motorMinUs;
			}
		}
	}
}


//Functions describe before

uint32_t usToTicks(uint32_t us){
	return (clockSourceMhz * us) / (float)prescaler;
}

uint32_t ticksToUs(uint32_t ticks){
	return ((ticks * (float)prescaler) / clockSourceMhz);
}


long map(long x, long in_min, long in_max, long out_min, long out_max)
{
	long result = (x - in_min) * (out_max - out_min) / (in_max - in_min) + out_min;
	
	if(result < out_min){
		result = out_min;
	}
	else if(result > out_max){
		result = out_max;
	}
  
	return result; 
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
	int16_t computedThrottle = map(throttleUs, 1400, 2000, 700, 1400);

This access of the value of throttleUs is not atomic, and as it is done repeatedly -- every few microseconds -- it could well be your cause.

 

Or at least part of it... servo[] accesses in main() need to be atomic as well.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok, so I am not familiar with that "atomic" thing... I understood that it means I clear the interrupt while the instruction is realised right? Two questions about it:

 

1) What is the better way of doing it?

 

2) If i clear the interrupt, then I could miss a timer1 interrupt and so my software PMW will still act weirdly, isn't it?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

2) If i clear the interrupt, then I could miss a timer1 interrupt and so my software PMW will still act weirdly, isn't it?

No, you don't "miss" the interrupt.  You delay servicing it.  For about a microsecond.

1) What is the better way of doing it?

Well, I have a number of comments/suggestions.  If your code is pretty close to "working", then perhaps continue and just do atomic accesses properly.

 

That said:

-- Tell what is on PB3 for the pin change.  If a button, buttons bounce and you might get false hits.  Most of us here wouldn't use an interrupt at all to service buttons.  Now the ISR goes away.

-- IMO/IME I'd try to abstract all the microseconds-timer counts stuff.  Figure it out at programming time based on the AVR's clock speed and then use timer counts.

-- I'd let timer1 free-run, and use a leapfrog approach to set the next compare value.

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Tutorials forum articles of interest:

http://legacy.avrfreaks.net/inde...

http://legacy.avrfreaks.net/inde...

 

Look for "atomic", and as you are using GCC "ATOMIC_BLOCK".

http://www.nongnu.org/avr-libc/u...

http://www.atmel.com/webdoc/AVRL...

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I use ISR_NOBLOCK so even if my PCINT routine is too long, I should go into the timer1 interrupt everytime I should, am I?

A recipe for disaster. If you ever use NOBLOCK you are not only allowing other interrupts to break into your current handler but you are also allowing interrupts of the same type to pre-empt the current handling. What happens if you get 15 PCINTs in the time it takes the first one to handle the original event? Have you even written the handler to be re-entrant anyway?

 

The real solution here is to fix this:

if my PCINT routine is too long

PS forgot to say that is you are using NOBLOCK then at least turn off the interrupt source you are currently handling. It's better to miss the odd interrupt than to get into some recursion nightmare!

Last Edited: Fri. Feb 20, 2015 - 04:25 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for all your comments. I partially find the problem.

 

Actually this is not from the atomicity missing, but from my map() function. If you look at the code I posted last time, I did this:

 

int16_t computedThrottle = map(throttleUs, 1400, 2000, 700, 1400);
servo[0] = computedThrottle;
servo[1] = computedThrottle;
servo[2] = computedThrottle;
servo[3] = computedThrottle;

If a directy code the map function like this:

 

servo[0] = ((float)throttleUs - 1400) * (1400 - 700) / (2000 - 1400) + 700;
servo[1] = ((float)throttleUs - 1400) * (1400 - 700) / (2000 - 1400) + 700;
servo[2] = ((float)throttleUs - 1400) * (1400 - 700) / (2000 - 1400) + 700;
servo[3] = ((float)throttleUs - 1400) * (1400 - 700) / (2000 - 1400) + 700;

Then it works... Notice that I need a float cast.

 

I still don't really know why, maybe somebody can help me. It is certainly about types...

 

Now I have a new thing that is weird. The code above works, but this one is not working:

 

ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
{	
	int16_t computedThrottle = ((float)throttleUs - 1400) * (1400 - 700) / (2000 - 1400) + 700;
}

servo[0] = computedThrottle;
servo[1] = computedThrottle;
servo[2] = computedThrottle;
servo[3] = computedThrottle;

If I use this code, my motors are going at maximum speed whatever is the throttleUs value...

Here I really don't understand why... Can somebody help me on that?

 

Thanks.

Last Edited: Sun. Feb 22, 2015 - 10:42 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Get a pencil and paper and work out the numbers using integer arithmetic. This might explain the problem. The float cast is causing the compiler to use floating point rather than integer.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If other avrfreaks are not used to the PulseModulationWidth acronym, perhaps at least the thread title could be edited to mention PWM, which will allow more successful searches.

 

Imagecraft compiler user

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

bobby4078 wrote:
Now I have a new thing that is weird. The code above works, but this one is not working:

 

ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
{	
	int16_t computedThrottle = ((float)throttleUs - 1400) * (1400 - 700) / (2000 - 1400) + 700;
}

servo[0] = computedThrottle;
servo[1] = computedThrottle;
servo[2] = computedThrottle;
servo[3] = computedThrottle;

If I use this code, my motors are going at maximum speed whatever is the throttleUs value...

Here I really don't understand why... Can somebody help me on that?

You seem to have two separate computedThrottle's.

The one inside the braces only exists inside the braces.

Didn't you get any warnings?

 

Also, I'd recommend against doing floating point inside an atomic block.

Usually only put fetches and stores inside atomic blocks.

Floating point can take a while.

Iluvatar is the better part of Valar.