Problem: Compute distance with msp430, interrupt issue

Huong Ha

Other Parts Discussed in Thread: MSP430G2553

Hello everyone. I'm trying to compute distance using msp430G2553 and SRF05. The connections pin are shown below:

SRF05 ======> MSP430

trigger =======> P1.7

echo =======> P2.1

I didn't know why the interruption on P2.1 happened a lot of times (one time for if(P2IN & BIT1 == BIT1) and many times for else after). And the distance value was wrong.

Here is my code (I use CCS 6.2.0). Sorry, I don't know how to embed .rar file. The uart_puts() and uart_put_num() just write a string and a number to terminal, respectively.

/* Description: compute distance and show on the terminal via UART
 * CONNECT: VCC --> 5V
 * 			TRIGGER --> P1.7
 * 			ECHO --> P2.1
 * 			GND --> GND
 * 			NC --> no connect
 */
// khong nhay vao ngat

#include <msp430g2553.h>
//#include <intrinsics.h>
#include "uart.h"
#define	trigger		BIT7;
#define	 echo		BIT1;

void timer_init(void);
void trig(void);

unsigned int t1=0, t2=0, distance=100;


void main(void){
	WDTCTL = WDTPW | WDTHOLD;
	BCSCTL1 = CALBC1_1MHZ;
	DCOCTL =  CALDCO_1MHZ;

	P1DIR |= trigger;		// trigger ouput
	P1OUT &= ~trigger;
	P2DIR &= ~echo
	P2SEL |= echo;			//
	uart_init();
	timer_init();
	_BIS_SR(GIE);			// global interrupt enable
//	uart_puts("------\r\n");
	while (1){
		trig();
//		uart_puts("-----1\r\n");
		__delay_cycles(30000);		//  30ms time out
//		uart_puts("----2\r\n");

	}

}


void timer_init(void){
	TA1CTL = TASSEL_2 + MC_2;		// SMCLK; continuos mode, 0 --> FFFF
	TA1CCTL1 = CM_3 + CAP + CCIS_0 + SCS+ CCIE;		// falling edge & raising edge, capture mode, capture/compare interrupt enable
	// P2.1 (echo) is connected to CCIA (capture/compare input )
	TA1CCTL1 &= ~ CCIFG;
}

void trig(void){
	P1OUT |= trigger;
	__delay_cycles(20);
	P1OUT &= ~trigger;
}



#pragma vector = TIMER1_A1_VECTOR
__interrupt void compute_distance(void){
	uart_puts("--6\r\n"); // Just test
	switch(_even_in_range(TA1IV,0x0e)){
	    case TA1IV_NONE:

	    	break; // no more interrupts pending, exit ISR here.
	    case TA1IV_TACCR1: {		// CCR1 interrupt
	    	if (P2IN & BIT1 == BIT1) {		//  detect rising edge
	    			t1 = TA1CCR1;

	    		}
	    		else {						// and falling edge
	    			t2 = TA1CCR1;

	    			if (t2 > t1){
	    				distance = (t2-t1)/58;
	    				uart_put_num(distance,0,0);
	    				uart_puts("\r\n");
	    			}

	    		}
	    	break;
	    }
	    case TA1IV_TACCR2:
	    	break; // CCR2 interrupt
	    case TA1IV_6:
	    	break;
	    case TA1IV_8:
	    	break;
	    case TA1IV_TAIFG:
	    	break; // timer overflow
	    default:
	    	break;
	  }

	TA1CCTL1 &= ~ CCIFG;
}

over 8 years ago

0 Bruce McKenney over 8 years ago

Guru 99235 points

1) This doesn't do what you think it does:
if (P2IN & BIT1 == BIT1)
It should rather be:
if ((P2IN & BIT1) == BIT1)

2) This also doesn't do what you think it does:
if (t2 > t1)
In Continuous mode the counter wraps every ~65 milliseconds, so there is a fairly good chance it will wrap between the two edges, and t2 < t1. Dealing with modulo-64K values can be messy, but if you compute the difference t2-t1, unsigned underflow will do the right thing and you'll get the expected result. Life gets easier if you only work with the deltas, not the actual timestamps.

I'm not sure what to recommend you replace this line with, because I'm not sure it should be there at all. My first suggestion would be to just remove it, and let the subtraction on the next line do what it does.

Unsolicited: I'll throw in the usual warning about not using the UART from an ISR, but maybe(?) you were planning to remove that stuff anyway.

0 Huong Ha over 8 years ago in reply to Bruce McKenney

Hi Bruce McKenney,

I try to change my code according to your suggestions but it still doesn't work well.

This is my code after changing:

switch(_even_in_range(TA1IV,0x0e)){
	    case TA1IV_NONE:
//	    	uart_puts("--7\r\n");
	    	break; // no more interrupts pending, exit ISR here.
	    case TA1IV_TACCR1: {		// CCR1 interrupt
	    	if ((P2IN & BIT1) == BIT1) {		//  neu co xung canh len tai chan echo
	    			t1 = TA1CCR1;
	    			uart_puts("--4\r\n");
	    		}
	    		else {						// neu co canh xuong tai chan echo
//	    			t2 = TA1CCR1;
	    			uart_puts("-5\r\n");
	    			if ((TA1CCR1 - t1) > 0){
	    				distance = ((TA1CCR1 - t1)/58);
	    				uart_put_num(distance,0,0);
	    				uart_puts("\r\n");
	    			}

	    		}
	    	break;
	    }
	    case TA1IV_TACCR2:
	    	break; // CCR2 interrupt
	    case TA1IV_6:
	    	break;
	    case TA1IV_8:
	    	break;
	    case TA1IV_TAIFG:
	    	break; // timer overflow
	    default:
	    	break;
	  }

This is the original code and it works well, so I don't know where are mistakes in my code.

/* Description: compute distance and show on the terminal via UART
 * CONNECT: VCC --> 5V
 * 			TRIGGER --> P1.7
 * 			ECHO --> P2.0
 * 			GND --> GND
 * 			NC --> no connect
 */


#include <msp430g2553.h>
#include "uart.h"
#define	trigger		BIT7;
#define	 echo		BIT0;

void timer_init(void);
void do_khoang_cach(void);

unsigned int t1=0, t2=0, delta=0, first_pulse=0;


void main(void){
	WDTCTL = WDTPW | WDTHOLD;
	BCSCTL1 = CALBC1_1MHZ;
	DCOCTL =  CALDCO_1MHZ;

	P1DIR |= trigger;		// trigger ouput
	P2SEL |= echo;			//connect P2.0 (echo) to CCI0A (capture/compare input )
	uart_init();
	timer_init();
	_BIS_SR(GIE);			//ngat toan cuc

	while (1){
		do_khoang_cach();
//		uart_puts("-----1\r\n");
		__delay_cycles(30000);		//  30ms time out

	}

}


void timer_init(void){
	TA1CTL = TASSEL_2 + MC_2 ;		// continuos mode, 0 --> FFFF
	TA1CCTL0 = CM_3 + CAP + CCIS_0 + SCS+ CCIE;		// falling edge & raising edge, capture mode, capture/compare interrupt enable
	TA1CCTL0 &= ~ CCIFG;
}

void do_khoang_cach(void){

	P1OUT |= trigger;
	__delay_cycles(20);
	P1OUT &= ~trigger;
}



#pragma vector = TIMER1_A0_VECTOR
__interrupt void timer0(void){

	if (P2IN & BIT0 ==1 ) {		//  neu co xung canh len tai chan echo (P2.0)
		t1 = TA1CCR0;
//		uart_puts("-----4\r\n");
	}
	else {						// neu co canh xuong tai chan echo (P2.0)
		t2 = TA1CCR0;
//		uart_puts("-----5\r\n");
		if (t2 > t1){
			delta = (t2-t1)/58;
			uart_put_num(delta,0,0);
			uart_puts("\r\n");
		}

		}

	TA1CCTL0 &= ~ CCIFG;
}

0 Huong Ha over 8 years ago in reply to Bruce McKenney

oh. I understood. Let see on Family User's guide: "Any access, read or write, of the TAIV register automatically resets the highest pending interrupt flag. If another interrupt flag is set, another interrupt is immediately generated after servicing the initial interrupt. For example, if the TACCR1 and TACCR2 CCIFG flags are set when the interrupt service routine accesses the TAIV register, TACCR1 CCIFG is reset automatically. After the RETI instruction of the interrupt service routine is executed, the TACCR2 CCIFG flag will generate another interrupt."
So I add lines:
TA1CCTL2 &= ~ CCIE;
TA1CCTL2 &= ~ CCIFG;

TA1CCTL0 &= ~ CCIE;
TA1CCTL0 &= ~ CCIFG;

0 Bruce McKenney over 8 years ago in reply to Huong Ha

Guru 99235 points

Your ISR code was truncated as you post it. Does it still end with this line?
TA1CCTL1 &= ~ CCIFG;

As your quote points out, this is done automatically by the hardware on reading TA1IV, so you shouldn't be doing that. If you clear anything there, it will be the CCIFG for the Next event.

This is particularly relevant with your addition of the line:
uart_puts("--4\r\n");
just after capturing t1. I'm guessing that uart_puts() doesn't complete until all the bytes have been written, and that the UART is running at 9600bps (since that's as fast as the G2 Launchpad USB UART can run). If so, that call will take 5 milliseconds to run.

In combination: by pausing for 5ms, then clearing TACCTL1:CCIFG, you may be clearing the CCIFG for the Next event (the falling edge), so you won't ever see it. 5ms looks to be about 86cm of distance, or about arm's length.

I recommend you remove the uart_puts("--4") (at least temporarily) and the CCIFG clearing (permanently).

What exactly "doesn't work well"?

0 Bruce McKenney over 8 years ago in reply to Huong Ha

Guru 99235 points

Regarding the other channels: The previous paragraph notes that "Disabled Timer_A interrupts do not affect the TAIV value", so their example assumes that CCR2:CCIE is also set. I suppose they could have said that explicitly, but it would have made the sentence more unwieldy.

So: Clearing those bits won't hurt you, but they won't help you either. The CCIE bits are 0 when your program starts, and (as noted) the CCIFGs don't matter much without the respective CCIEs set.

0 Huong Ha over 8 years ago in reply to Bruce McKenney

oh.. the line uart_puts("--4\r\n") just a log and "doesn't work well" means the distance values were wrong. But don't worry, when I add lines: TA1CCTL0 &= ~CCIE; TA1CCTL0 &= ~CCIFG; TA1CCTL2 &= ~CCIE; TA1CCTL2 &= ~CCIFG; the distance values were ture.

**Attention** This is a public forum

MSP low-power microcontrollers

MSP low-power microcontroller forum

Problem: Compute distance with msp430, interrupt issue