msp430g2553 servo control

Hi Dogukan!

I did not look at your complete code, but what I see immediately: Why do you change the period of the PWM all the time? Normally you have a fixed period and change the duty-cycle. Is your servo one from a R/C model? Then your period is 20ms and the duty-cycle varies between 0.8ms and 2.2ms.

I once had a similar example here:

The only thing it would need is changing the duty-cycle (TA0CCR1).

Dennis

I editted the firts post. that is my final code. I have simplified the code in order to find error, but it didn't work either. as a matter of fact, this is my first project about microcontrollers, so i knew it would fail. Also i looked the link you share, i made a few changes.

edit: without button check it works.

TA1CCTL0 = CCIE; //interrupt enable
Why do you enable it?, you don't have a ISR for it and there is at the moment no need to know when CCR0 is hit.

P1REN |= (BIT3 + BIT4 + BIT5); // 3-4-5 pull up enable
Common mistake, it enable pull resistor, direction is determined with P1OUT

Don't use function call inside the ISR, the ISR is the function.
Don't put __delay_cycles  inside a ISR, in this case as it's the only ISR no harm done but learn how to wakeup main instead.
Or use the timer already running as way to software debounce.

Replace: __enable_interrupt(); for(;;){}
with this:

  while(1)
  {
    _BIS_SR(LPM0_bits + GIE);           // Enter LPM0 w/ interrupt
  }  

Not tested but should work.
Now you have a good base to add: if ccr1 < targetvalue ccr1 +=stepspeed stuff to make the elevator move slowly

#include <msp430.h>
#define smclk 1000000
#define servo_freq 50

char debounce = 25;                     // not zero as we need it as a init
int pwm_period = smclk /servo_freq;

int main(void)
{
  WDTCTL = WDTPW | WDTHOLD;             // Stop watchdog timer
  if (CALBC1_1MHZ != 0xff){             // don't use if erased
    BCSCTL1 = CALBC1_1MHZ;              // Set range
    DCOCTL = CALDCO_1MHZ;               // Set DCO step + modulation
  }

  P1DIR &= ~(BIT3 + BIT4 + BIT5);       // set 3-4-5 input (a PUC does that)
  P1REN |= (BIT3 + BIT4 + BIT5);        // 3-4-5 pull resistor enable
  P1OUT |= (BIT3 + BIT4 + BIT5);        // 3-4-5 pull-up direction
  P1IES |= (BIT3 + BIT4 + BIT5);        // high-to-low edge interrupt

  P2DIR |= BIT2;                        // P2.2 output;
  P2SEL |= BIT2;                        // set timer TA.1

  TA1CCR0 = pwm_period-1;               // Set Timer_A1 period to
  TA1CCTL1 = OUTMOD_7;                  // CCR1 reset/set
  TA1CCR1 = 1000;                       // CCR1 PWM duty cycle
  TA1CTL = TASSEL_2 + MC_1;             // smclock - upmdode
  TA1CCTL0 = CCIE;                      // interrupt enable

  while(1)
  {
    _BIS_SR(LPM0_bits + GIE);           // Enter LPM0 w/ interrupt
  }
}
//╞════════════════════════════ TA1 ISR ══════════════════════════════════════╡  
#pragma vector=TIMER1_A0_VECTOR         // Timer A1_0 interrupt service routine
__interrupt void Timer_A1 (void)
{  	   		
  if (debounce && !(--debounce)){       // good rule is not to pre-decrement
    P1IFG &= ~(BIT3 + BIT4 + BIT5);     // without checking if already zero
    P1IE |= (BIT3 + BIT4 + BIT5);       // interrupt enable
  }
}

//╞════════════════════════════ P1 ISR ═══════════════════════════════════════╡  
#pragma vector=PORT1_VECTOR
__interrupt void Port_1(void)
{
  P1IE &= ~(BIT3 + BIT4 + BIT5);         // interrupt disable as a debounce
  debounce = 25;                         // 25*20ms = 1/2sec lock-out period
  if (P1IFG & BIT3)       TA1CCR1 = 1000;
  else if (P1IFG & BIT4)  TA1CCR1 = 1500;
  else if (P1IFG & BIT5)  TA1CCR1 = 2000;
}

In your original code you don't need CCIE,
in my code CCIE and its ISR is needed as I actually take advantage that PWM is a timer so I can get a 1/2sec button lockout period.
It could be modified by not disable P1IE in the port-ISR and moving down the P1IFG &= ~ to that isr
and moving up P1IE enable to the main code and then timer ISR is no longer needed.

But if you want to add stuff so the elevator moves in steps from 1000 to 15000, it would be in the timer ISR you would do that.

i have written code for the speed. for example;

  if (P1IFG & BIT1)     
  	  {
	  while(TA1CCR1<=1500)
	  	  {
  		TA1CCR1= (TA1CCR1 + 1) ;
  		delay(1);
  	  	  }
  	  }

void delay(unsigned int ms)
{
 while (ms--)
    {
        __delay_cycles(1000); 
    }
}

it works when we want to go 1000 to 1500. but how about 1500 to 1000?. what should i do for that? i could write 

TA1CCR1= (TA1CCR1 - 1) ;

but it's the same

 +=  is increment

and -= is decrement.

TA1CCR1 += 1;

TA1CCR1 -= 1;

But bad programming style to use while() and delay() ,when a 20ms timer is already running.

You are hogging the system and once you start wanting to add something else that needs to "multitask" your code will fail completely.

volatile char cflr = 0;
volatile char nflr = 0;

void main(void)
{
..
..
..
}


#pragma vector=PORT1_VECTOR
__interrupt void Port_1(void)
{
  P1IE &= ~(BIT1 + BIT2 + BIT3 + BIT5 + BIT7);         // interrupt disable as a debounce
  debounce = 25;                         // 25*20ms = 1/2sec lock-out period

  if(P1IFG & BIT1)	//
  {
   nflr=1;   //next floor 1
   if((nflr-cflr)>0) // check if we go upstair?
   	 {
  	   while(TA1CCR1<=800){TA1CCR1 +=1; delay(30);}  //if we go up
   	 }
   else if((nflr-cflr)<0) //check if we go downstair?
   	 {
   	   while(TA1CCR1>=800){TA1CCR1 -=1; delay(30);} //if we go down
     }
   cflr = 1;   //current floor  1
  }

  else if(P1IFG & BIT2)
  {
   nflr=2;
   if((nflr-cflr)>0)
   	 {
  	   while(TA1CCR1<=1600){TA1CCR1 +=1; delay(30);}
   	 }
   else if((nflr-cflr)<0)
   	 {
   	   while(TA1CCR1>=1600){TA1CCR1 -=1; delay(30);}
    	 }
   cflr = 2;
  }


  else if(P1IFG & BIT3)
  {
   nflr=3;
   if((nflr-cflr)>0)
   	 {
  	   while(TA1CCR1<=2400){TA1CCR1 +=1; delay(30); }
   	 }
   else if((nflr-cflr)<0)
   	 {
   	   while(TA1CCR1>=2400){TA1CCR1 -=1; delay(30);}
    	 }
   cflr = 3;
  }

  if(P1IFG & BIT7)
  {
	  TA1CCR1 = TA1CCR1 + 800;    // increase TA1CCR1
	  if(TA1CCR1 >=2400)    TA1CCR1=2400;  // until 2400
  }
  else if(P1IFG & BIT5)
  {
	  TA1CCR1 = TA1CCR1 - 800;   //decrease TA1CCR1
	  if(TA1CCR1 <=800)    TA1CCR1=800;   //until 800
  }
}

void delay(unsigned int ms)
{
 while (ms--)
    {
        __delay_cycles(100);   //ms
    }
}

you misunderstood, i figure out the problem but it doesn't seem like good solution to me. is there any way to improve this

In my example where I use the pwm's CCR0 ISR to debounce the button.

you can keep debounce function or not, but it's in this ISR you add/sub the CCR1
As with this  iSR you know the CCR1 have been used at least once before you change it.
You don't need to use delay as this one is a nice 20ms time slot

With a single core msp with no preemptive multitasking (TI RTOS does emulate some of that though) 
You have to learn how to write code that can do many things at the same time using IRQs that send flags to main.c

#define aclk 12000
#define servo_freq 50
static const int pwm_period = aclk /servo_freq;

  BCSCTL1 &= ~XTS;
  BCSCTL3 |= LFXT1S_2;

 i guess it should be like this, but doesn't work.

Did you do TASSEL_1?

TA1CTL = TASSEL_1 + MC_1; // ACLK source