Jump to content
43oh

Little Clarity on Bit declarations


Recommended Posts

Hi Guys,

 

I'm hoping that this is a basic question, as I'm trying to learn in steps rather than bombarding myself with the volumes of guides and sheets. 

 

Right now, I'm trying to get a better understanding of the pin declarations.  It seems that there are a few ways to set pins, direct pins, etc. 

 

In this code I am simply trying to sense a voltage from my motion sensor (which works, I tested that), then, if a HIGH read, place a HIGH on a different pin (1.6, the launchpad LED). 

 

I would greatly appreciate a few words regarding the PIN vs POUT vs PDIR rather than the pieces of user guides that use different types of declarations and structures (like the hex version). Ideally, a little more clarity on the style that I am using here with the Boolean operators because I thought I was using them correctly, but as I step through the register view, Pin 1, Bit 6 never goes HIGH. My code is below and thank you very much for getting a new msp coder (with c and c++ cobwebs) into the swing of things!

 

If it helps, I'm using ccs5.3

 

Techgique

 

#include <msp430.h> 

/*
 * main.c
 */
// Pins for LED and button on port 1
    #define LED1 BIT6
    #define B1 BIT1


void main (void)
{
    WDTCTL = WDTPW | WDTHOLD; 		// Stop watchdog timer
    P1OUT &= ~LED1; 				// Initial LED1 off
    P1DIR = LED1; 					// Set pin with LED1 to output
    for (; {						// Loop forever
    	if ((P1IN & B1) == 1) {		// Is bit high?
    		P1OUT |= LED1; 			// Yes: Turn LED1 on
        } else {
        	((P1OUT & LED1) == 0); 	// No: Turn LED1 off
          }
    }
}

 

 

Link to post
Share on other sites

Just at first glance, you've not set "B1" up. You've simply, and only as far as i can tell #defined it as BIT1.

 

Secondly, how is B1 supposed to go high ? Is there a button on this pin ?

 

Anyway, its best to watch the msp430 launchpad videos to get a rough idea of whats going on.  Passed that, like the guy in the videos says, read the userguide, datasheet when need be, and open up the msp430g2553.h header file and start looking around. Assuming you are using a v1.5 launchpad with a g2553 MCU on it.

 

EDIT:

 

Just as a matter of style though. Why even bother defining #define B1 BIT1 ? It makes things even more cryptic, as its not descriptive as to what it actually does. IMHO it's just a waste of a #define.

 

Personally, I think it would be better to just use if(P1IN & BIT1), followed by making a clear comment about what this actually is supposed to do, and for what.

 

Also, what this does, is *if* later down the road when you have a huge program, and are trying to debug for an issue or whatever. It keeps you, or whomever else, from having to bounce from reading the code, to having to go back way up somewhere else looking for an actual #define meaning.

 

Anyway, your call.

Link to post
Share on other sites

Ah yes. You're on the right track.

In your "if" condition, you do "(P1IN & B1) == 1" which will always be false. Since B1 = BIT1 = 0x02, if you'd ever compare it with 1 it will be false; 2 will never contain 1. In a binary form: 2 equals b00000010 while 1 is b00000001. As you can see there are no bits commonly set high.

What you're rpobably trying to do is check whether this bit is set, you can either do "(P1IN & B1) == B1" or "(P1IN & B1)".

The first does "if I mask out all bits of P1IN except BIT1, will the result equal BIT1", the second is a bit more abstract and tells "if I mask out all bits of P1IN but BIT1, will the result be anything but zero". As there is only one bit not masked out, any non-zero result must be that the concerning bit is set high.

 

The second "error" in your code would be the "((P1OUT & LED1) == 0)" which would spell out as "if I mask out all bits of P1OUT but BIT6, would the result be zero". This is obviously not what you meant, you don't want to compare, but you want to assign.

During your setup you do it the right way: "P1OUT &= ~LED1" which is short for "P1OUT = P1OUT & ~LED1" which spells out to "assign to P1OUT the result of P1OUT with BIT6 masked out" or more correctly "assign to P1OUT the result of P1OUT with all bits ANDed with 1 except BIT6".

 

@yyrkoon: All pins default to output, by "P1DIR = LED1" all bits except LED1 implicitly switch to inputs. I think you mean you're missing a bit of setup like "P1OUT |= B1; P1REN = B1" which would set the pull-up resistor on this pin. This is only required if no external pull-up/down is in place. I already assumed B1 was short for button1.

 

If your button is shorting P1.1 to ground, you should check for "(P1IN & B1) == 0" instead of "(P1IN & B1) == B1", or your output will de the opposite of your requirement.

Link to post
Share on other sites

@yyrkoon: All pins default to output, by "P1DIR = LED1" all bits except LED1 implicitly switch to inputs. I think you mean you're missing a bit of setup like "P1OUT |= B1; P1REN = B1" which would set the pull-up resistor on this pin. This is only required if no external pull-up/down is in place. I already assumed B1 was short for button1.

 

If your button is shorting P1.1 to ground, you should check for "(P1IN & B1) == 0" instead of "(P1IN & B1) == B1", or your output will de the opposite of your requirement.

 

Well I have a few problems with this. 

 

First, how is an output supposed to handle a pin input ? Second, you're assuming this is in fact a button.  Also, as you already said you also need a pullup/down. Passed that, you need debouncing in hardware or code. *Assuming* this is in fact a button.

 

Lastly, and definitely not least. I believe in being explicit. Always assume if you do not explicitly set something of this nature, that it will not be what you want it to be. Forget about what the users-guide says in this instance. It is bad practice / habit. Nothing lost by physically setting pins up yourself, except perhaps a few minutes of your time.

Link to post
Share on other sites

I agree, you should at least consider every aspect of the GPIO pin for both B1 and LED1. If you're familiar enough you might group stuff together, it's good practice indeed to set all registers that will be of importance (so if PREN bit is low and PDIR bit is low, POUT bit is don't care).

The user guide states that POUT is allowed to be written when you're using the pin as an input, however if you have PREN enabled POUT will affect the internal resistor (will be pull up or pull down). So yeah, you're allowed to write to POUT if you're using it as PIN, although without PREN it won't have any effect.

 

Debouncing is required only if the code requires it. In this case bouncing of the button would cause bouncing of the LED, but since bouncing is so fast, you won't see it. In this case it doesn't really matter. In other cases (say you need to count button presses or something like that, or you want to toggle the LED with it) you must use debouncing (a capacitor in parallel with your button is not debouncing!) to get reliable read outs.

Link to post
Share on other sites

@roadrunner84

 

I hope that anyone that needs clarification gets a chance to read this. This was immensely helpful and I feel that I have a much better understanding of the specific areas I was struggling with. Admittedly, the interpretation of code I put together from reading sections of code and their accompanying definitions got a little sloppy, so I appreciate your combing through any vague references. The way you put it into perspective helped me to visualize the 8 bits sitting there with ones or zeros, depending upon what we are doing with the port.

 

@yyrkoon

 

You were both essentially right regarding the button, because it was a pieced over code from a button going low, but I was only using it as an unnecessary declaration. Even LED1 isn't really worthwhile aside from noting that it is the LED on the board, because ultimately I will be controlling something external from the dev board. I always appreciate good advice for future considerations like this mention of blanketing my bits with wording and having to run to the top of the source file to figure out what it meant. Ultimately, debouncing could become an issue, so it's good to bring up in regard to potential projects, but for this application, even 500ms of delay wouldn't matter any.

 

So a few questions for y'all:  (I have posted my cleaned up version that actually works and described operations of the code as I understand it.)

 

1. Are you guys referring to setting all P1REN in order to prevent floating pins?

2. Am I commenting on my lines of code correctly? If not, were did my wording go wrong?

 

Thank You guys for the replies, I am still purging Arduino's IF BITx.x SET BITx.x structure of coding while trying to revive my C knowledge (Mostly all the nifty shortcuts like the few you showed me). It's great to see a helpful community and I'm glad I got my boards when they were at the 5 dollar mark!

 

Steve

 

PS - As a side note, understanding the Hex helped too

 

i.e.   0x42 = 8 Bits

 

                      

   Hex        |          4          |           2           | 

   Binary    |        0100       |         0010         |

 

so BIT 6 and BIT 2 are HIGH

 

 

#include <msp430.h> 


                                                // Pins for LED and button on port 1
    #define LED1 BIT6


                                                //All pins defaulted to inputs (0000 0000)

void main (void)
{
    WDTCTL = WDTPW | WDTHOLD; 			// Stop watchdog timer

    P1OUT &= ~LED1; 				// Assign to P1OUT the result of P1OUT and all of it's bits AND'ed with 1, aside 
                                                // from LED1 (BIT 6) (1101 1111)
    
    P1REN = BIT1;				// Set pull-up resistor on BIT1
    
    P1DIR = LED1; 				// All pins become inputs aside from LED1 (BIT6) (iiOi iiii) Note that in binary for 
                                                // PxDIR this would read (0010 0000)

    for (; {					// Loop forever

    	if ((P1IN & BIT1)== BIT1) {	        // If all bits are masked (ignored) aside from BIT1, does the result match BIT1? In                   
                                                // other words, did voltage from the motion sensor cause a HIGH on BIT1? Remember we 
                                                // initially set 1101 1111. This doesn't change anything, rather it asks a question

    	P1OUT |= LED1; 				// If true, assign to P1OUT The value of LED1 Or'ed with P1OUT (1111 1111)
        
        } else {
        	(P1OUT = 0x00); 		// If false, use Hex to turn off all bits
          }
    }
}

 

 

  

Link to post
Share on other sites

Someone correct me if I am wrong( my binary math is a bit bad at times) but . . .

 

if ((P1IN & BIT1)== BIT1) essentially equates to if (P1IN & BIT1) right ? Minor point of contention. Anyhow. .  .

 

"1. Are you guys referring to setting all P1REN in order to prevent floating pins?

2. Am I commenting on my lines of code correctly? If not, were did my wording go wrong?"

 

#1 we were, at least I was refereeing to pulling the pin high, or low, so when the button is pressed, it is obvious to the MCU. Forgive me if I am wrong, I am not exactly an EE( hobbyist ) but it is my understanding that usually ( always? ) for buttons, you want to pull them high, so when the pin goes low, you fire off an interrupt. Assuming you have the interrupt setup properly. In this case, floating is bad, as you could get all kinds of undesired/undefined behavior. But what you mentioned in your question also applies. It is worth reading up on, and many people can explain this better than I.

 

#2 Well this is a bit subjective. We all can only comment well on code that we know well. . . as our understanding grows, our comments become closer to the "truth" However, when commenting your code, you're almost forced on some other level to understand what the code does. Which in turn helps the learning process . . . Did that answer your question ? lol

Link to post
Share on other sites

Someone correct me if I am wrong( my binary math is a bit bad at times) but . . .

if ((P1IN & BIT1)== BIT1) essentially equates to if (P1IN & BIT1) right ? Minor point of contention. Anyhow. .  .

 

Yes, if(PxIN & BITx) will do the job. 

Link to post
Share on other sites

Something I was noticing just now after revisiting your code.

 

(P1OUT = 0x00); <--- bad habit.  (P1OUT &= ~LED1); <--- would be better

 

The reason is simply this. If you're using any other peripherals on that port, you're going to possibly clear everything that may already be set when using "= 0x00". This may, or may not be your intention. In this given case . ..

 

= 0x00;       equates to clear everything.

|= LED1     equates to set specific bit high.

&= ~LED1  equates to clear this one specific bit. Or set low. 

^= LED1     Toggle this specific bit.

 

Does this help, or am I being too obvious ?     

Link to post
Share on other sites

Not being too obvious at all, because remember this may help many lurkers that are afraid to ask their questions. This is all much appreciated and that final note on the pins makes sense and I can see how that could throw a monkey wrench into the whole program by clearing everything.

 

Like I noted before too, those shortcuts are wonderfully helpful and assist in more efficient coding, so I will definitely update that little ditty.

 

With the button discussion, it would totally work different from an electrical perspective, so I tried to get all remants of buttoness out of the program in order to avoid confusion. The input pin receives input from a parallax PIR sensor, so in this instance I am not pondering the button aspect.

 

Thank you to all, this answered all of my questions...so far.

 

Side note: Would P1OUT = 0xBF also be a decent way to clear that specific bit?

Link to post
Share on other sites

Personal preference -> It may work, but it may not be clear at a glance. The whole idea behind my own personal preferences is to make things as clear as possible at a glance. Which I do admit with my own style gradually can change over time, as I think i find a better way of doing things.

 

Anyone who has spent time with an MSP430 should nearly instantly know what something like P1IN & BIT1 is/does. Coupled with short concise comments it will become obvious. Where something like P1OUT = 0xBF may take a few moments to sink in, also if you're not good with converting hex into binary in your head ( like me ), you're taking the comments at the authors word, that it is actually doing what is intended.

 

Indeed there are other possible implications. Such as does P1OUT = 0xBF take up less code space on the target versus setting each bit one declaration at a time for b10111111? I really do not think so but . . . It definitely takes more code in the editor( to set each bit individually ). Which would be clearer ? For this given situation, I think it would be perfectly acceptable to do this as an initialization step. But probably would be wise to make clear comments as to why, and for what you're doing this. As a general rule, and again personal preference here. I would try to avoid doing this with "runtime" code. But I could see at least one type of situation where it could be handy. Such as a situation where you need these bits high(second from the left low ), for a specific purpose/reason. No matter what their current state is.

 

Personally, I think at the hobbyist level, whatever works for *you* is probably good enough. Just keep in mind that later down the road, you may end up looking at the code, and wonder yourself wtf you were doing. At this point, it would be a good idea to adapt a good style that you can nearly instantly read always. Do keep in mind that when I say this, nothing can be perfect.

 

?When commenting though...

 

something = a + b; // something = a + b <---- obvious and not necessary.

 

However . . .

 

something = a + b; // Comments describing why we're adding two variables, and storing them in something <-- might be useful.

 

Again, this all has to do with personal style, context, and perhaps on some level;. A given developers experience with anything in relation for the given project.

Link to post
Share on other sites

 

 

 

#include <msp430.h> 


                                                // Pins for LED and button on port 1
    #define LED1 BIT6


                                                //All pins defaulted to inputs (0000 0000)

void main (void)
{
    WDTCTL = WDTPW | WDTHOLD; 			// Stop watchdog timer

    P1OUT &= ~LED1; 				// Assign to P1OUT the result of P1OUT and all of it's bits AND'ed with 1, aside 
                                                // from LED1 (BIT 6) (1101 1111)
    
    P1REN = BIT1;				// Set pull-up resistor on BIT1
    
    P1DIR = LED1; 				// All pins become inputs aside from LED1 (BIT6) (iiOi iiii) Note that in binary for 
                                                // PxDIR this would read (0010 0000)

    for (; {					// Loop forever

    	if ((P1IN & BIT1)== BIT1) {	        // If all bits are masked (ignored) aside from BIT1, does the result match BIT1? In                   
                                                // other words, did voltage from the motion sensor cause a HIGH on BIT1? Remember we 
                                                // initially set 1101 1111. This doesn't change anything, rather it asks a question

    	P1OUT |= LED1; 				// If true, assign to P1OUT The value of LED1 Or'ed with P1OUT (1111 1111)
        
        } else {
        	(P1OUT = 0x00); 		// If false, use Hex to turn off all bits
          }
    }
}

This is not working the way it is described in the comments. You P1REN = BIT1; which will set the resistor to pin P1.1. But since you never write P1OUT = BIT1; or P1OUT |= BIT1; P1.1 is actualle pull-down, not pull up (as the default value of P1OUT is 0, so all pins will be low).

I agree with yyrkoon that your commenta are a bit lengthy and reduntant. For example the line P1OUT &= ~LED1; could be commented with "turn off LED".

Your if-block looks twisted too, since you either set LED1 or clear all bits including LED1. This may be intentional, but you do create a dependeny between this statement and every other use of P1. In some defensive coding strategies it may be useful to overwrite every bit every time, but in this case it's looking weird and error prone.

Oh, and you put braces around P1OUT = 0x00, why did you do that?

Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...