Here is what I had. This starts with the LEDs turned on, then turns them off (permanently) with the button. What joy when they finally turned off

I think my biggest obstacle (besides minimal C skillz) was trying to visualize what was going on. I learned to use the debugger more, watching as registers changed.

Does anyone know where I can find a diagram with the S2 button on? I've looked all over the place.

@MrChips You mentioned making the code portable. I am interested, very. I don't know what that means, but I have a hunch. What do I do?

int main(void)
{
    WDTCTL = WDTPW | WDTHOLD;    // stop watchdog timer
    P2DIR = BIT1 + BIT4;    //Set LEDs on breadboard to toggle
    P1REN = BIT3;           //Enable pullup resistor for the S2 button
    P1OUT = BIT3;           //Resistor to pullup???


    while(1)
    {

        if((P1IN & BIT3))      //Checks for release - Executes when (P2IN & BIT3)
        {
            P2OUT |= BIT1 + BIT4;   //Toggle LEDs
        }
        else
        {
            P2OUT = !(BIT1 + BIT4);     //Shuts LEDs off
        }
    }
    return 0;
}

 

Which experimental/developmental board are you using?
Give full part number.

 

MSP430G2553

 

Which one is it? They are different.

MSP-EXP430G2
MSP-EXP430G2ET

 

The first one. MSP-EXP430G2. I always thought MSP430G2553 was the whole number.

 

MSP430G2553 is the part number of the chip.
The Lauchpad experimental board is MSP-EXP430G2. This has been replaced by MSP-EXP430G2ET.

The documentation and circuit schematic can be found on TI website.

On the MSP-EXP430G2
P1.0 is LED1
P1.6 is LED2
P1.3 is switch S2. This creates a short to GND when pressed. There is no external pull-up resistor. You can use the GPIO port pull-up as you have already done.

I will post links to the docs later.

 

Here is the User Guide for MSP-EXP430G2
http://www.ti.com/lit/ug/slau318g/slau318g.pdf

Here is the User Guide for MSP-EXP430G2ET
http://www.ti.com/lit/ug/slau772/slau772.pdf

RED LED on P1.0
GREEN LED on P1.6
S1 on P1.3 now has 47K external pull-up resistor.

 

Here is what I had.

            P2OUT = !(BIT1 + BIT4);     //Shuts LEDs off

A suggestion. This sets all the P2 pins (really output latches) to low. It's equivalent to "P2OUT=0;". This works fine in this program, since you're not doing anything else with P2, but it's a habit that will cause trouble eventually.

You can clear just P2.1 and P2.4 with

            P2OUT &= ~(BIT1 + BIT4);    //Shuts LEDs off

 

Yes, later on into a big project you will learn to pay attention to details.
For starters, do not use hardware specific identifiers.
Use #define statements such as:

#define LED_PORT P1OUT
#define RED_LED BIT0
#define GREEN_LED BIT6

Thus, in your code, do stuff like this:

LED_PORT |= RED_LED; // turn on red LED
LED_PORT &= ~GREEN_LED; // turn off green LED

Later I will should how to change the code so that it becomes clear, using structures and unions.
You will simply write:

RED_LED = ON;
GREEN_LED = OFF;

 

@mckenney Ok, makes sense. Is there a difference between !(BIT1 + BIT4) and ~(BIT1 + BIT4)? "~" is sometimes called flip-bit operator, but as far as I know, "!" does the same. EDIT: I found something on this. Apparently they are ! the same.

@MrChps Thanks for those sheets, I have completely forgotten they exist. You mention using #define over hardware identifiers, is it because of code readability?

And one thing more. I'm having a hard time understanding PxREN vs PxOUT. I thought PxREN was enough to enable the pull up resistor. I don't understand those lines.

// LED initial setup
P1DIR |= RED_LED;  // set RED_LED as output (1) pins and enable it
// Button setup
[B] P1DIR &= ~BUTTON;  // button is an input 
P1OUT |= BUTTON;  // pull-up resistor [/B]
P1REN |= BUTTON;  // resistor enabled

 

Perhaps "portable" may be the wrong word to use. "Maintainable" may be a better word.

1) Do not use specific hardware identifiers such as BIT3 and P2OUT in case you may want to change to a different pin or port.

Create user defined identifiers such as RED_LED, START_BUTTON.

2) When modifying single bits in a register, modify the target bit only and not other bits.

P2OUT = BIT1 + BIT4; will also unintentionally effect all other bits.
P2OUT |= BIT1 + BIT4; will set bit-1 and bit-4 and not affect the other bits.
P2OUT &= ~(BIT1 + BIT4); will clear bit-1 and bit-4 and not affect the other bits

! and ~ are not the same.

~ is a bit-wise inversion, i.e. it changes all zeros to ones, and ones to zeros.
! is a logical operator, i.e. it changes TRUE to FALSE, and FALSE to TRUE.

!(P1IN & BIT3) will always be TRUE regardless on the status of P1IN bit-3.

3) Do not use constant literals in your code such as delay(1234);
Declare a constant such as
#define ms_200 1234

delay(ms_200);

Put all you #define statements in one place, either at the top of your main.c code or in a header file such as constant.h

 

In my next post, I will show you how to write code such as:

if (BUTTON)
{
   RED_LED = ON;
}
else
{
   RED_LED = OFF;
}

This makes the code much more readable and easier to maintain in the future.

 

So we declare our own identifiers by using #define statements that are all placed in one convenient place, either at the up of your code or in a header file. In this way, you avoid having to search through thousands of lines of code to find where you used P1IN and BIT3.

#define ON  1
#define OFF 0

#define BUTTON  !P1IN_bit.P3
#define RED_LED P1OUT_bit.P0

 

@mckenney
And one thing more. I'm having a hard time understanding PxREN vs PxOUT. I thought PxREN was enough to enable the pull up resistor.

P1OUT |= BUTTON;  // pull-up resistor [/B]
P1REN |= BUTTON;  // resistor enabled

The internal resistor can pull either up or down. PxOUT says which direction. PxOUT is undefined after reset, so it's important to explicitly set it. This also applies to e.g. pins with LEDs: you should set PxOUT before PxDIR so you don't get an annoying blink (sometimes) at reset.

 

IAR toolset has bits already declared using structure and union. Code Composer Studio does not have this. Hence we have to declare them ourselves.

#pragma LOCATION(P1IN_bit,  0x0020);
#pragma LOCATION(P1OUT_bit, 0x0021);

struct BITS P1IN_bit;
struct BITS P1OUT_bit;

struct BITS
  {
    unsigned char P0   : 1;
    unsigned char P1   : 1;
    unsigned char P2   : 1;
    unsigned char P3   : 1;
    unsigned char P4   : 1;
    unsigned char P5   : 1;
    unsigned char P6   : 1;
    unsigned char P7   : 1;
  };

In IAR, all of the above is already done for you in the io430g2553.h header file.
In CCS, this is not done and the above LOCATION assignments make the code less than ideally portable.

 

If you're a fan of schematics, the data sheet (SLAS735J) figure on p. 44 shows the P1.3 wiring.

 

Here is my test code with improvements written for CCS.

#include <msp430.h>

#pragma LOCATION(P1IN_bit,  0x0020);
#pragma LOCATION(P1OUT_bit, 0x0021);

struct BITS P1IN_bit;
struct BITS P1OUT_bit;

struct BITS
  {
    unsigned char P0   : 1;
    unsigned char P1   : 1;
    unsigned char P2   : 1;
    unsigned char P3   : 1;
    unsigned char P4   : 1;
    unsigned char P5   : 1;
    unsigned char P6   : 1;
    unsigned char P7   : 1;
  };

#define ON  1
#define OFF 0

#define RED_LED   P1OUT_bit.P0
#define GREEN_LED P1OUT_bit.P6
#define BUTTON   !P1IN_bit.P3

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

  // For the purists, this part needs cleaning up
    P1DIR = BIT0 + BIT6;
    P1REN = BIT3;
    P1OUT = BIT3;
}

void main (void)
{
   init();

    while(1)
    {
        if (BUTTON)
        {
            RED_LED = OFF;
            GREEN_LED = ON;
        }
        else
        {
            RED_LED = ON;
            GREEN_LED = OFF;
        }
    }

}

 

Thanks for the help, guys. I am chewing heavily on this and will return with something tonight. After that, I'm will continue with the assignments.

 

@KeepItSimpleStupid At this point, all of it seems weird Thanks for your example.

He just uses names instead of numbers. Everyone thinks this is easier, but its not.

The idea is like this:
WatchdogRegisterName | Number

NameVar = Number;

so at the end we have

WatchdogRegistername | NameVar

 

Final cut.

There were still a couple of things I had not understood, and I also found out I was doing a very stupid error in the initialization.

I couldn't understand why "P1OUT = BIT0 | BIT6" was supposed to be inn the initialization part. Turned out it had no place there and I misread something. After that, things went smooth.

And now to implement what @MrChips said.

#include <msp430.h>               

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

    P1DIR = BIT0 | BIT6;    //Set P1.0 and P1.6 to output
    P1REN = BIT3;           //Enable internal pull-up resistor (this only enables it, it does not dictate direction)
    P1OUT = BIT3;           //Set enabled pull-up resistor to pull UP, forcing P1.3 to be high


    while(1)
    {

        if ((P1IN & BIT3) == 0)    //See if button is pressed - P1.3 is active-high, so pressed = low = light
        {
            P1OUT |= (BIT0 + BIT6);     //Remember to use |, otherwise it will delete BIT3 and remain high
        }
      else
        {
            P1OUT &= ~(BIT0 + BIT6);    //Take whatever is in P1OUT, keep it and flip BIT0 and BIT6
        }

    }
    return 0;
}