Servo driving problem

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

Hi I'm working with this program on atmega 328p trying to set a pwm for a servo. I'm doing the pwm with TIMER0 because I run out the others Timers and pins. Everything works fine if I don't add the cli() function and I can drive my servo. The problem is that I need to switch off the interrupts for a small time (in the code below to obtain the servo position) but adding the cli() the servo starts to turn endlessly. I can't understand why it does such a thing, where is the problem with cli() ?

 

#define MS 256-64

volatile int position, ovf;

int main(void)
{ 
 DDRB= 0xff;	
 
 ADMUX |= (1<<REFS0);
 ADCSRA |=(1<<ADEN)|(1<<ADPS1)|(1<<ADPS0);        //ADC0
 
 sei();
 TIMSK0 |= 0x01;        //enable overflow interrupt
 TCCR0B |= 0x01;        //prescaler=1
 TCNT0= MS;             //TCNT0 starting point

 
    while (1) 
    {
        cli();                 //without this works fine
        ADCSRA |= (1<<ADSC);
        while((ADCSRA & (1<<ADIF))==0);
        position= ADC*250L/1023+250;      //range 250-500
        ADCSRA |= (1<<ADIF);			 
        sei();
        
    }
}

ISR(TIMER0_OVF_vect)         //every 4us
{	
    TCNT0= MS;
    ovf++;
    
    if(ovf<position)         //1-2ms
        PORTB |= 0x02;
    
    if(ovf>position)
        PORTB &=~ 0x02;

    if(ovf>12500)	         //50ms
            ovf=0;
                
}

 

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

Don't CLI for the entire duration of the ADC reading. Nothing much else happens so it'll be in the CLI state almost all the time. If this is about atomic protection of the shared 'position'variable just wrap ATOMIC_BLOCK only around the write to it. Even precalculate the expression so ONLY the write is in the CLI block.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
#define MS 256-64

volatile uint16_t position;

int main(void)
{ 
 DDRB = 0xff;	
 
 ADMUX = (1<<REFS0);
 ADCSRA = (1<<ADEN)|(1<<ADPS1)|(1<<ADPS0);        //ADC0
 
 
 
 TIMSK0 |= 0x01;        //enable overflow interrupt
 TCNT0 = MS; //TCNT0 starting point 
 TCCR0B |= 0x01;        //prescaler=1
 
 sei();
 
    while (1) 
    {
        
        ADCSRA |= (1<<ADSC);
        while(ADCSRA & (1<<ADSC));
        cli();
        position= ADC*250L/1023+250;      //range 250-500		 
        sei();
        
    }
}

ISR(TIMER0_OVF_vect)         //every 4us
{
static uint16_t ovf = 0;
	
    TCNT0= MS;
    ovf++;
    
    if ( ovf < position )         //1-2ms
        {
        PORTB |= 0x02;
        }
    
    if ( ovf > position )
        {
        PORTB &=~ 0x02;
        }

    if ( ovf > 12500 )	         //50ms
        {
        ovf=0;
        }
                
}

I've applied a few little 'fixes'. 

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

Ok in this way it works.

Now I have to add other stuff in the code: I added one distance aquisition with hc-sr04 sensor and I swapped the adc position set with the reception of an nrf24l01 wireless antenna. I need to switch off the interrupt just for make the antenna work but even if I set the position value for myself the servo does what it wants. So here what is the problem?

volatile int position
volatile unsigned int t=0, distance;
volatile char mode;

int main()
{
	DDRB= 0xfe;
	DDRD |= 0xC0;

        set_wireless();
	set_TCNT0();
	set_inputcapture();
	sei();

    while(1)
	{
		cli();
		if(nrf24_dataReady())
		{
			nrf24_getData(data_array);
		}
		sei();

	//	position= data_array[1]+250;     //250 - 500

		position= 300;

		distance= find_distance();	

		if(distance<7)        //7cm
		PORTB |= 0x08;

		else
		PORTB &=~ 0x08;		

		switch(mode)
		{
			case 0:				

			break;

			case 1:

			break;

			case 2:

			break;

		}//switch

    }//while
}//main

If I remove the cli() function everything works fine

Last Edited: Sun. Mar 29, 2020 - 02:31 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I already told you why.

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

But now interrupts are not always off since there are other things in the code. How can I fix it?

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

When your code is multiplexing between things (called "tasks") like reading the NFR24, reading the proximity distance, and reading an ADC, then you need to have main() do all the things at specific times,  and do each task as few times as necessary for your application.    How quickly do you need to update the data from the NFR24, prox distance, and ADC?  10,000 times a second?  20 times a second? (for normal human interaction like push-switch or pot/encoder adjustments).

   Do what Arduino does.  Set a timer to trigger an interrupt 1000 times a second.  Increment a 32-bit variable at each interrupt (you have several weeks before an overflow).   Have another 32-bit variable for each task.  After doing each task, add the value for the next time that the task should be done to the task's 32-bit variable.   Your while(1) inside your main() should be constantly comparing the present 32-bit timer count to the 32-bit timer trigger value for each task.   When they are equal, then it's time to do the task again.

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

Simonetta wrote:
When your code is multiplexing between things (called "tasks") ... then you need (sic) to have main() do all the things at specific times

No, you don't need to - that is just one of many possible approaches.

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

For simple program, such as yours, if you have to keep shutting off interrupts you are probably doing something wrong.

Write your code assuming the interrupts are always running.  Do not shut them off.

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

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

The promblem is that the wireless comunication fails if I don't put them off. 

I tried to run the task of the nrf only every 400us (100 times the interrupt) and also other times and disable the interrupt with it but still the servo doesn't work. This doesn't make sense because interrupts are disabled only for a little time.

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

nico444 wrote:
The promblem is that the wireless comunication fails if I don't put them off. 
That would suggest that the interrupts you are using are consuming too much CPu time if it "upsets" the radio stack.

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

How often do you need to read or use the radio?  Put it on a timer & look at it maybe every 10ms. I usually use 40 times a second (25ms) to send button commands. 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

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

A 4us timer tick is rather onerous - it is probably sucking over 50% of the compute resource. Whilst your current method is simplistic, you might want to do some optimisation, like counting overflows then counting microseconds. ie 2ms might be 1 overflow (4us * 256) + 244 counts. Much less compute overhead.

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

If I use fast PWM mode on timer1 to create the pwm (I need two different pwm for two servos) is there a way to measure the signal of the hc-sr04 with an 8bit timer? I now measure it with the timer1 input capture, for this reason I'm using so small interrupt to create pwm. 

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

Timer1 can do compares and input capture concurrently. But don't use any of the pwm modes. The servo pwm is something like a 1 to 2ms pulse every 20ms. If you do one after the other you can easily get 10 output channels by using one compare. Whilst doing that you can also use input capture code to measure time and still have plenty of cpu time to burn.

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

Ja I'm going to do in that way, with output compares and input capture both on timer1. Can you explain me how to use output compare to generate a pwm? I've nevere done it before

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

Code here for servo signals done using output compare:

https://www.avrfreaks.net/comment/182391#comment-182391

/Lars

 

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

So I wrote a code with the output compare but there is still something to fix

 

//define F_CPU 16000000UL

int main(void)
{ 
 DDRB= 0xff;	

 sei();
 TCCR1A |= (1<<COM1A1)|(1<<COM1A0);    //set on compare match
 TCCR1B |= (1<<CS10)|(1<<CS10);        //prescaler= 64
 TIMSK1 |= (1<<OCIE1A);                //output compare interrupt enable

 
    while (1) 
    {
    }
}



ISR(TIMER1_COMPA_vect)
{
	static unsigned int position= 125;
	
	unsigned int currentTime= OCR1A;
	
	if ((TCCR1A & (1<<COM1A0))==0)          
	{
		OCR1A= currentTime+12500-position;
		TCCR1A |= (1<<COM1A0)|(1<<COM1A1);       //set on compare match
	}	
		
	
	if ((TCCR1A & (1<<COM1A0))!=0) 
	{
		OCR1A= currentTime+position;
		TCCR1A &=~ (1<<COM1A0);             //clear on compare match
	}	
	
	
}//isr

Isn't it the right way to do it?

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

Your ISR will always execute this part

        OCR1A= currentTime+position;
        TCCR1A &=~ (1<<COM1A0);             //clear on compare match

 

Replace

if ((TCCR1A & (1<<COM1A0))!=0) 

with an else.

/Lars

 

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

Right now everything works together. Thank you guys