Jump to content
Sign in to follow this  
teoah

Weird CCS problem with #define

Recommended Posts

Hi,

 

I am a newb (both in C and programmers) with an MSP430G2231 mounted on a LaunchPad. I successfully compiled the "Hello World" program:

 

#include "msp430g2231.h"

#define 	LEDRED   BIT0;
#define 	DELAY    (0xFFFF);

void main(void) {

WDTCTL = WDTPW + WDTHOLD;	// Stop watchdog timer
P1DIR |= LEDRED; 				// Set P1.0 to output direction
P1OUT &= ~LEDRED;				// Turn off LED

 	while (1) {

   	P1OUT ^= LEDRED; // Toggle LED

   	volatile unsigned int i = DELAY; // Delay
   	while (i != 0) i--;
}
}

Right now, I'm trying to also get the green LED to blink as well, so I have the following code:

 

#include "msp430g2231.h"

#define    LEDRED   BIT0;
#define    LEDGREEN BIT6;
#define    DELAY    (0xFFFF);

void main(void) {

  WDTCTL = WDTPW + WDTHOLD;   // Stop watchdog timer
  P1DIR |= LEDRED + LEDGREEN;             // Set P1.0 to output direction
  P1OUT &= ~(LEDRED + LEDGREEN);            // Turn off LED

  while (1) {

     P1OUT ^= LEDRED + LEDGREEN; // Toggle LED

     volatile unsigned int i = DELAY; // Delay
     while (i != 0) i--;
  }
}

 

But for some reason, it just won't compile! It tells me "expression has no effect on the three lines with LEDGREEN, and "expected an expression" at the ~(LEDRED + LEDGREEN) line.

 

The weird thing is that when I use BIT0 and BIT6 instead of LEDRED and LEDGREEN, everything compiles fine! Anyone knows what's going on here?

Share this post


Link to post
Share on other sites

A few errors, but no worries,I'll walk you through. Before I go ahead,welcome to the Forums!

 

#define LEDRED BIT0;

#define LEDGREEN BIT6;

#define DELAY (0xFFFF)

Do not use ";" for defines, so get rid of those. Your project will compile fine after you do that.

 

P1DIR |= LEDRED + LEDGREEN;

Always use parenthesis when you are evaluating more that one math expressions. Its proper technique and will save quite a few headaches later on. This is not an error, but good coding practice.

 

So the above line will become:

P1DIR |= (LEDRED + LEDGREEN); 

 

Next point:

P1DIR |= LEDRED + LEDGREEN;

You are trying to turn both LEDs on. What you are doing is bit manipulation. You actually "or"the two fields not "+"

So your line now becomes:

P1DIR |= (LEDRED | LEDGREEN); 

 

Do this to the rest of your code. Your corrected file is below:

#include "msp430g2231.h"

#define    LEDRED   BIT0
#define    LEDGREEN BIT6
#define    DELAY    (0xFFFF)

void main(void) {

  WDTCTL =  ( WDTPW | WDTHOLD   );   // Stop watchdog timer
  P1DIR |=  ( LEDRED | LEDGREEN );   // Set P1.0 to output direction
  P1OUT &= ~( LEDRED | LEDGREEN );   // Turn off LED

  while (1) {

     P1OUT ^= ( LEDRED + LEDGREEN ); // Toggle LED

     volatile unsigned int i = DELAY; // Delay
     while (i != 0) i--;
  }
}

 

If this works out for you, let us know. Any more questions,feel free to ask.

Share this post


Link to post
Share on other sites

Thanks for the warm welcome.

It works like a charm! :)

 

Being completely new to this world, I have a few general questions (let me know if I should post it in another subforum):

- Are all variables limited to 16 bits? For example, my DELAY variable has a maximum value of 0xFFFF. How would I go about sleeping for longer, say... 0xFFFFFFFF? (short of looping in a loop I guess).

- I kind of understand why we need to disable the watchdog timer, but what if I wanted to leave it on? How would I make sure that it won't reset my program unexpectedly, but still reset it if it crashes? (Not sure if this question even makes sense)

- What does volatile mean? It works just as well when I try without it.

- I understand the idea of registers and instructions, but the concept of input/output is still a bit fuzzy. Where can I read some more about it?

 

Thanks!

Share this post


Link to post
Share on other sites
Next point:
P1DIR |= LEDRED + LEDGREEN;

You are trying to turn both LEDs on. What you are doing is bit manipulation. You actually "or"the two fields not "+"

So your line now becomes:

P1DIR |= (LEDRED | LEDGREEN); 

bluehash (or any other of the smart folks out there):

 

In the case above, I definitely agree on your answers (I missed the semi-colon part :? ), but I have a question as a learning exercise for both teoah and myself:

 

Doesn't LEDRED + LEDGREEN (0b00000001 + 0b01000000 ~ 1 + 64) equal the same thing as LEDRED | LEDGREEN (i.e.: 0b01000001 ~ 65)? The result, naturally, would be ORed with P1DIR / P1OUT to get the desired result without affecting other pins. Probably not best coding practice, but I have used this ("+" vice "|") in the past.

 

(please see my sig for standard intelligence waiver :D )

 

-Doc

Share this post


Link to post
Share on other sites

From what I understand, using OR is not the same at all as using +.

For example:

 

1001 OR 1100 = 1101

(in decimals -> 9 OR 12 = 13)

but 13 in binary is 10101, not 1101.

So the end result is not the same.

Share this post


Link to post
Share on other sites

Doesn't LEDRED + LEDGREEN (0b00000001 + 0b01000000 ~ 1 + 64) equal the same thing as LEDRED | LEDGREEN (i.e.: 0b01000001 ~ 65)? The result, naturally, would be ORed with P1DIR / P1OUT to get the desired result without affecting other pins. Probably not best coding practice, but I have used this ("+" vice "|") in the past.

 

It works fine for when you know exactly what the two variables and the outcome will be, and when the two variables don't overlap.

 

So 1000 + 0001 = 1001, and 1000 OR 0001 = 1001 (No overlaping bits, addition and oring will work the same)

but

1000 + 1001 = 10001, while 1000 OR 1001 = 1001 (Overlaping bits, addition will roll over into next bit, while ORing will work as normal)

 

And then you have to factor in a variable, er, variable. If P1OUT is 00000001, and you OR it into 1001, it will be 00001001, but if you add it, it will be 00001010, which would leave bit 1 off when you want it on. What if P1OUT has been changed by an interrupt, and you don't know the exact value of it when the section of code that modifies it runs, by adding instead of ORing, you might change an important bit and the entire program hoses itself because somethings wrong. (My example, the LCD-I2C project had an issue where commands to the LCD would corrupt it. I had forgot that I cleared the buffer I used, which included the command/data bit, so the LCD would never work. You don't always know what you are manipulating, so what operation you use can make something work marvelously, or never work at all).

Share this post


Link to post
Share on other sites

actually when i 1st saw this BIT1 + BIT2 convention i thought it was very weird.

 

i understand it will work on the examples as long as the bits do not overlap. i.e. u don't do BIT1 + BIT1

BIT1 + BIT1 = BIT2 where as BIT1 | BIT1 = BIT1

 

i don't write like this (using add) but i guess this convention may work for beginners to understand things easier

 

i.e. if i want BIT1 "AND" BIT2 of this port to be on, why have i have to write BIT1 "OR" BIT2 (as in BIT1 | BIT2). placing it as BIT1 + BIT2 definitely allow beginners to relate things better. and it is definitely incorrect. i would not recommend it.

 

a lot of beginners w/ C experiences may not work w/ bitwise operator too much and the '+' operator may work for them in getting into MCU programming quick. but i think this is not right.

Share this post


Link to post
Share on other sites

I agree with everybody's answers.

 

I've ONLY used "+" with the defined "BIT0"-"BIT7", but I agree that it's a bad practice. :oops: I have been schooled, and will cease this heinous habit immediately! :lol:

Share this post


Link to post
Share on other sites

I actually prefer the + operator when using bit manipulations, assuming I know what's going on under the hood. But I'm from the school of thought that if something works and you understand it, then by all means use it. Just keep your code commented.

Share this post


Link to post
Share on other sites

@teoah

 

Are all variables limited to 16 bits?

 

no. registers are 16 bit so if a variable is 16 bit it's more efficient. u can define a long variable which is 32 bits and the compiler will generate assembly / machine codes to handle it. when doing math operations, registers are loaded w/ 16 bit word parts (high word, low word) so that the MCU can operate against the values. and thru the use of carry bits, extend the math operation to the other word. some MCUs may have special paired registers that are used to hold the low and high words and can perform math in fewer instructions, i am not sure if MSP430 does that (may be the high end devices do).

 

typically i would tried to use 8/16 bit variables and avoid 32 bit longs. it really shows up on code size.

 

consider we do a loop w/ long (32bit)

       volatile long i = 0x66667777;
       while (i--);

this is the assembler listing for the c code

   f834:   31 40 7c 02     mov #636,   r1  ;#0x027c
   f838:   b1 40 77 77     mov #30583, 0(r1)   ;#0x7777, 0x0000(r1)
   f83c:   00 00 
   f83e:   b1 40 66 66     mov #26214, 2(r1)   ;#0x6666, 0x0002(r1)
   f842:   02 00 
   f844:   2e 41           mov @r1,    r14
   f846:   1f 41 02 00     mov 2(r1),  r15 ;0x0002(r1)
   f84a:   0c 4e           mov r14,    r12
   f84c:   0d 4f           mov r15,    r13
   f84e:   3c 53           add #-1,    r12 ;r3 As==11
   f850:   3d 63           addc    #-1,    r13 ;r3 As==11
   f852:   81 4c 00 00     mov r12,    0(r1)   ;0x0000(r1)
   f856:   81 4d 02 00     mov r13,    2(r1)   ;0x0002(r1)
   f85a:   0e 93           tst r14
   f85c:   f3 23           jnz $-24        ;abs 0xf844
   f85e:   0f 93           tst r15
   f860:   f1 23           jnz $-28        ;abs 0xf844

you will notice that the variable are split as two words for all operations as the MCU is a 16bit cpu. comparisons need to be done twice. subtraction (the "add -1" is done on both words but one w/ carry flag "addc")

 

as a comparison, for a 16 bit integer.

       volatile int i = 0x6677;
       while (i--);

yields

   f834:   31 40 7e 02     mov #638,   r1  ;#0x027e
   f838:   b1 40 77 66     mov #26231, 0(r1)   ;#0x6677, 0x0000(r1)
   f83c:   00 00 
   f83e:   2f 41           mov @r1,    r15
   f840:   0e 4f           mov r15,    r14
   f842:   3e 53           add #-1,    r14 ;r3 As==11
   f844:   81 4e 00 00     mov r14,    0(r1)   ;0x0000(r1)
   f848:   0f 93           tst r15
   f84a:   f9 23           jnz $-12        ;abs 0xf83e

 

on code size f860-f84a= 21 bytes. so if i do not need a long i won't define one. same between int and char types.

 

this reply is getting long (and i got to go), may be i will try to reply the other questions later.

Share this post


Link to post
Share on other sites

Wow! tons of replies. Lots to learn. :)

Being completely new to this world, I have a few general questions (let me know if I should post it in another subforum):

- Are all variables limited to 16 bits? For example, my DELAY variable has a maximum value of 0xFFFF. How would I go about sleeping for longer, say... 0xFFFFFFFF? (short of looping in a loop I guess).

Like you said, put it in a loop. Or if your running off a hardware timer, slow it down.

 

- I kind of understand why we need to disable the watchdog timer, but what if I wanted to leave it on? How would I make sure that it won't reset my program unexpectedly, but still reset it if it crashes? (Not sure if this question even makes sense)

You kind of explained how a watchdog works. You set up the watchdog,so that your program tickles(or pets aka set a bit) before your watchdog timer expires and resets the chip. If your program runs correctly, it will always "tickle" the watchdog...but if it crashes, it will stop setting the watchdog bit..which will reset the chip.

 

- What does volatile mean? It works just as well when I try without it.

The Volatile keyword lets the compiler know that the variable is HANDS-OFF from optimization. Its normally used when declaring register mapped to a hardware register. For eg: If variable outPort writes to the P1OUT register, it must be declared as volatile.

volatile int16_t outPort;

For eg: If you write the following without the volatile keyword

outPort = 0xA5
for(i=0;i P1OUT = outPort ;

 

Sometimes the compiler is clever enough to know that P1OUT is set only once in the loop and will set P1OUT in the first iteration and not everytime. Hope this helps.

 

 

- I understand the idea of registers and instructions, but the concept of input/output is still a bit fuzzy. Where can I read some more about it?

Did not quite get you there. You mean writing to pins? There is also a direction register, if that's confusing you.

 

Buzz back if anything is unclear.

Share this post


Link to post
Share on other sites

some MCUs may have special paired registers that are used to hold the low and high words and can perform math in fewer instructions, i am not sure if MSP430 does that (may be the high end devices do).

 

It does on some. The USI peripheral for one. The USICTL 16bit register can be accessed by direct usage, or by USICTL0 and USICTL01, two 8 bit registers.

Share this post


Link to post
Share on other sites

Thanks for the reply bluehash!

 

You kind of explained how a watchdog works. You set up the watchdog,so that your program tickles(or pets aka set a bit) before your watchdog timer expires and resets the chip. If your program runs correctly, it will always "tickle" the watchdog...but if it crashes, it will stop setting the watchdog bit..which will reset the chip.

Yes, I understand that. My question is, how do I tickle the dog (using code) if I don't want to put him to sleep? Or is the dog tickled automatically?

 

Did not quite get you there. You mean writing to pins?

Yes! That's what I mean. What does it mean to set a pin as input or output?

Share this post


Link to post
Share on other sites
Yes, I understand that. My question is, how do I tickle the dog (using code) if I don't want to put him to sleep? Or is the dog tickled automatically?

By page 10-6 of the MSP430x2xxx family user's guide, you would do it like this in ASM:

   MOV    #WDTPW+WDTCNTCL,&WDTCTL

Which in C would look something like:

   WDTCTL = WDTPW + WDTCNTCL;

Assuming WDTCNTCL is defined in the appropriate device header. I haven't looked into it, but it should be. Check it out for yourself?

 

The problem with this is that you'd have to do this every few operations or the program would reset anyways. But maybe you can come up with a solution?

 

Good luck!

Share this post


Link to post
Share on other sites

Similar to what gatesphere said.

 

In

http://focus.ti.com/lit/ug/slau144e/slau144e.pdf, pg 380

WDTCNTCL Bit 3 Watchdog timer+ counter clear. Setting WDTCNTCL = 1 clears the count
value to 0000h. WDTCNTCL is automatically reset.
0 No action
1 WDTCNT = 0000h

 

You can reset the watchdog by:

WDTCTL  |= WDTCNTCL

Yes! That's what I mean. What does it mean to set a pin as input or output?

Well, You have to tell the controller how you wish to use the pin. If you want to connect it to an LED, you configure it as an output. If you connect a switch to the pin, you need to configure it as an input. Its not going to work if you connect an LED to the pin, and its configured as an input.

You configure a pin in one step at the start of the program. This allows the controller to change its internal gate structure to accept a signal(input) or apply a signal(output):

Set the direction ( input or output ) of the pin in the direction register. An example of an output pin:

P1DIR |= BIT0

 

Now then, in your main program you may read or write to the pin. An example of writing to the pin:

P1OUT |= BIT0;

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
Sign in to follow this  

×