1
\$\begingroup\$

Currently I am working with two MSP430G2553 Microcontrollers for a school project and I have to use UART to have them communicate with each other. The transmit MSP430 is to send a character whenever a user inputs a password correctly to open an entryway. The door is being simulated by a servo motor on the receiving MSP430. My current code was functioning very well but the servo kept opening and closing; the only way to stop this was to disconnected or power off the MSP430. When I checked the 'UCA0RXBUF' register, we noticed the character would remain in the register and not clear. We attempted to push a zero to clear the register to no avail. My code has a series of .c files to make initialization of the UART easier to use. attached is my current code and maybe you might be able to point out a mistake, or syntax error, that I may have missed.

#include <msp430g2553.h>
#include <string.h>
#include "uartMsp.h"
#include "adcMsp.h"

void door(void);
void delay(void);
void buzzer(void);
void door2(void);
void door3(void);


volatile int i, x;
char received = 0;
unsigned int mode = 1;

void main(void){
//Setup
WDTCTL = WDTPW + WDTHOLD;// Stop Watchdog Timer

//Setup Timer A0.1 and Port 1.6
P1DIR |= BIT6;//Output for SERVO PWM
P2DIR |= BIT7;//Button to shut off buzzer
P1SEL |= BIT1 + BIT6;
P1SEL2 &= ~(BIT6);//??
TA0CTL = TASSEL_2;//Setup Timer
TA0CCTL1 = OUTMOD_6;
TA0CCR0 = 20000;

//Setup DCO
DCOCTL  = CALDCO_1MHZ;    //DCO at 1MHz
BCSCTL1 = CALBC1_1MHZ;    //DCO at 1MHz

//Setup ADC
ADC10CTL0    = ADC10ON + ADC10IE;             //Turn on ADC and Enable interrupts.
ADC10CTL1    = INCH_5;              //Pick an input to sample from (Any Port 1 Pin EXCEPT P1.0 or P1.6; those belong to the LEDs)
ADC10AE0     = BIT5;                //Set your pin to analog. Hint: BITx = P1.x

__delay_cycles(500000);

clock_init();                // Set clock frequency to 1 MHz
serial_init_inter();     // Initialize UART module
_enable_interrupts(); //Enable General Interrupts. Best to do this last.

while(1) {
    received = UCA0RXBUF;
    UCA0RXBUF = 0;
    if(received == 'o'){
        received = 0;
        door();

    }
    else if(received == 'b'){
        received = 0;
        buzzer();
    }
    else if(received == 'u'){
        received = 0;
        door2();

    }
    else if(received == 'e'){
        received = 0;
        door3();

    }
    ADC10CTL0 |= ENC + ADC10SC;
}//create an infinite loop that does nothing (or goes into low power mode)
}//End of main

// ADC10 Interrupt Service Routine
#pragma vector = ADC10_VECTOR
__interrupt void ADC10_ISR( void ) {
x = ADC10MEM;
if((x > 700) || (x < 300)){
    P1OUT |= BIT7;
    while((P1IN & BIT4) != BIT4){}
    delay();
    delay();
    P1OUT &= ~BIT7;
}
}//End of ADC10_ISR

//Door function
void door(void){
TA0CCR1 = 1500;
TA0CTL |= MC_1;//Setup Timer
delay();
TA0CCR1 = 750;
delay();
TA0CTL |= MC_0;//
}//End of door

void delay(void){
volatile unsigned j, k;
for(j=4;j>0;j--){
    for(k=20000;k>0;k--);
}
}

void buzzer(void){
P2OUT |= BIT5;
delay();
P2OUT &= ~BIT5;
}

void door2(void){
TA0CCR1 = 1500;
TA0CTL |= MC_1;//Setup Timer
}

void door3(void){
delay();
TA0CCR1 = 750;
delay();
TA0CTL |= MC_0;//

}
\$\endgroup\$
12
  • \$\begingroup\$ Always use more self-explanatory names for global variables. And this is no debugging or review service. See How to Ask and provide a [mcve]. \$\endgroup\$
    – Olaf
    Commented Apr 30, 2016 at 19:17
  • 1
    \$\begingroup\$ It looks as though you are reading the UART input data buffer without checking its rx status and ignoring that you have a serial interrupt routine. The data input register is not cleared by writing to it: reading it on a typical UART clears the rx data flag. You then don't read the data register again until the rx data flag tells you to. \$\endgroup\$
    – Weather Vane
    Commented Apr 30, 2016 at 19:21
  • \$\begingroup\$ And while I think of it, having a while loop waiting on status, and calling a delay function from within an ISR such as your __interrupt void ADC10_ISR are cardinal sins. Better find another way to structure the code. \$\endgroup\$
    – Weather Vane
    Commented Apr 30, 2016 at 19:32
  • 1
    \$\begingroup\$ @nicomp: Read the FAQs, see How to Ask. This is a Q&A site. \$\endgroup\$
    – Olaf
    Commented Apr 30, 2016 at 20:27
  • 1
    \$\begingroup\$ @Olaf, and he asked a Q that needs an A. QED. \$\endgroup\$
    – nicomp
    Commented May 1, 2016 at 0:28

1 Answer 1

1
\$\begingroup\$

The main issue is probably in the fact that you do not wait for a new character to be received before reading the UCA0RXBUF register. Unless you're using interrupt-based approach, you should busy loop like this:

while(!(UC0IFG & UCA0RXIFG)); /* wait for UCA0RXIFG to become set */
received = UCA0RXBUF;  /* read the received character */

You do not show the transmitting-side code here, but the idea there is the same - write to TXBUF register, and then wait until the byte is transmitted. Or use interrupts to get truly asynchronous operation over the UART (the "A" means asynchronous).

More broadly, I think you just have the wrong approach here. Why not use one of the synchronous buses? The MCU supports SPI and I2C. Normally when interoperating between two MCUs one of these protocols is used, putting one for the MCUs master mode and the other one in slave mode. Synchronous data exchange is generally more reliable and you don't have to worry about things like MCU speed calibration.

\$\endgroup\$
1
  • \$\begingroup\$ We haven't covered SPI and I2C in class yet but I will definitely look into and do some research. Also, I have figured out what I was doing wrong. I was not using the same configuration on the transmitting code. All I had to do was change the variable received = UCA0RXBUF; into c = UCA0RXBUF; because that is what was on the transmitting code. It fixed my issue and I was able to use Interrupt Service Routine to finalize my code. Thank you very much for your feedback and I will definitely look into SPI and I2C. \$\endgroup\$
    – MarineUTEP
    Commented May 7, 2016 at 7:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.