0
\$\begingroup\$

I am new to embedded C and been struggling with it for a while now. The desired output for this project is: When SW1 is not pressed, the blue LED must be on. When SW1 is pressed the blue LED should turn on and off every 100ms. I wrote this code but it doesn't seem to be working properly. I tried it on the simulator and the LED toggles but the delay is more than 100ms, it's like a second. And on the real board, I get random results, sometimes it doesn't turn off and sometimes it changes color to purple. Why isn't this code behaving as it should? Why am I getting random results?

enter image description here Code: pastebin.com/ShE9rDCG

// BranchingFunctionsDelays.c Lab 6
// Runs on LM4F120/TM4C123
// Use simple programming structures in C to
// toggle an LED while a button is pressed and
// turn the LED on when the button is released.  
// This lab will use the hardware already built into the LaunchPad.
// Daniel Valvano, Jonathan Valvano
// January 15, 2016

// built-in connection: PF0 connected to negative logic momentary switch, SW2
// built-in connection: PF1 connected to red LED
// built-in connection: PF2 connected to blue LED
// built-in connection: PF3 connected to green LED
// built-in connection: PF4 connected to negative logic momentary switch, SW1

#include "TExaS.h"

#define GPIO_PORTF_DATA_R       (*((volatile unsigned long *)0x400253FC))
#define GPIO_PORTF_DIR_R        (*((volatile unsigned long *)0x40025400))
#define GPIO_PORTF_AFSEL_R      (*((volatile unsigned long *)0x40025420))
#define GPIO_PORTF_PUR_R        (*((volatile unsigned long *)0x40025510))
#define GPIO_PORTF_DEN_R        (*((volatile unsigned long *)0x4002551C))
#define GPIO_PORTF_AMSEL_R      (*((volatile unsigned long *)0x40025528))
#define GPIO_PORTF_PCTL_R       (*((volatile unsigned long *)0x4002552C))
#define SYSCTL_RCGC2_R          (*((volatile unsigned long *)0x400FE108))
#define SYSCTL_RCGC2_GPIOF      0x00000020  // port F Clock Gating Control

// basic functions defined at end of startup.s
void DisableInterrupts(void); // Disable interrupts
void EnableInterrupts(void);  // Enable interrupts
void portF_init(void);
void delay100ms(unsigned long time);


int main(void)
{
        unsigned long volatile delay;
      // activate grader and set system clock to 80 MHz
        TExaS_Init(SW_PIN_PF4, LED_PIN_PF2);  
        portF_init();
        EnableInterrupts();      
        // set PF2
        GPIO_PORTF_DATA_R |= 0x04;
        while(1)
        {
                delay100ms(1);
              // if switch PF4 is pressed and LED is ON (00000101)
            if( GPIO_PORTF_DATA_R == 0x05)
                {
                      // turn LED OFF (clear bit)
                      GPIO_PORTF_DATA_R &= ~0x04;
                }      
                // if switch PF4 is pressed and LED is OFF (00000001)
                else if (GPIO_PORTF_DATA_R == 0x01)
                {
                        // set PF2 - turn LED ON
                      GPIO_PORTF_DATA_R |= 0x04;
                }
                else
                {
                        // set PF2
                        GPIO_PORTF_DATA_R |= 0x04; 
                }
        }
}

void portF_init(void)
{
        volatile unsigned long delay;
        SYSCTL_RCGC2_R |= 0x00000020;     // 1) F clock
        delay = SYSCTL_RCGC2_R;           // delay  
        GPIO_PORTF_AMSEL_R = 0x00;        // 3) disable analog function
        GPIO_PORTF_PCTL_R = 0x00000000;   // 4) GPIO clear bit PCTL  
        GPIO_PORTF_DIR_R = 0x04;          // 5) PF4 input, PF2 output  
        GPIO_PORTF_AFSEL_R = 0x00;        // 6) no alternate function
        GPIO_PORTF_PUR_R = 0x01;          // disble pull-up resistor
        GPIO_PORTF_DEN_R = 0x14;          // 7) enable digital pins PF4 & PF2        
}

void delay100ms(unsigned long time)
{
        unsigned long i;
        while(time > 0)
        {
                i = 1333333;  // this number means 100ms
                while(i > 0)
                {
                        i = i - 1;
                }
                time = time - 1; // decrements every 100 ms
      }
}
\$\endgroup\$
9
  • \$\begingroup\$ We don't know anything about your course work, so you're going to need to include a lot more detail. Include a schematic, we do know know what things are connected to what. When you say "the LED should turn on and off" which color do you mean? \$\endgroup\$
    – pgvoorhees
    Oct 17 '16 at 19:00
  • \$\begingroup\$ @MahmoudKhaled What is your question? Please be specific and post a question, this is a Q&A forum. See electronics.stackexchange.com/help/how-to-ask \$\endgroup\$
    – Voltage Spike
    Oct 17 '16 at 19:12
  • \$\begingroup\$ @pgvoorhees on top of the program it says what is connected to what, it's all built-in on the launch bad. And I mean the blue color \$\endgroup\$ Oct 17 '16 at 19:20
  • \$\begingroup\$ @laptop2d I am sorry but I tried to include all the information, my question is why is this code isn't behaving as it should? Why do I get random results? \$\endgroup\$ Oct 17 '16 at 19:23
  • \$\begingroup\$ I think the best way to debug complex systems is to test them piece by piece and see which parts are working and which ones are not. \$\endgroup\$
    – Voltage Spike
    Oct 17 '16 at 20:01
8
\$\begingroup\$
  • The delay function is broken.

    First of all, you should avoid writing delays with busy-loops. Such delays are unstable, unreliable and non-portable. They create a tight coupling to the system clock frequency. Also in case of sleep modes, they make the CPU consume current needlessly. Always use on-chip hardware timers instead.

    Now if you insist on writing such a delay loop, you must declare the actual loop iterator volatile. Otherwise the compiler might remove the whole loop, since it contains no side effects. It isn't enough to make the variable in the caller volatile, since the function is using a local copy of the passed variable. The variable inside the function must be volatile.

  • Another issue here is the lack of switch de-bouncing. You can only get away without it if you have a hardware RC filter on the switch.

  • Also, does the switch have an external pull resistor or does it assume that you enable one internally? You need to share the schematics.

\$\endgroup\$
7
  • \$\begingroup\$ I couldn't have ranted about delays and switch bounce better myself! \$\endgroup\$
    – vini_i
    Oct 19 '16 at 13:39
  • 1
    \$\begingroup\$ In fairness, if the OP is a beginner, a busy loop is perfectly valid for delays and it's a good teaching tool on how to optimize instruction counts for a particular system. I'm going to assume based on the fact that they are trying to blink an LED that they have not yet learned about timer, or timer interrupts. \$\endgroup\$ Oct 19 '16 at 14:08
  • \$\begingroup\$ @BrendanSimpson I disagree, busy-loops are 100% bad practice and a very common cause for completely broken embedded programs around the world. The sooner one learns how to use on-chip hardware timers, the better. It is not rocket science to set up a hardware timer. It is probably far easier than disassembling your program to count instruction cycles (...for this particular compilation). \$\endgroup\$
    – Lundin
    Oct 19 '16 at 14:12
  • 2
    \$\begingroup\$ From experience teaching embedded systems, it's not that simple. If you have people literally struggling to turn on and off LEDs (not this OP, but many students), you cant possibly explain to them something as "massively complex" as a timer. It's not like the OP or my students are writing a "real" program. You have to start somewhere, and a blinking LED using a busy-delay is a good place to start. \$\endgroup\$ Oct 19 '16 at 14:16
  • \$\begingroup\$ @BrendanSimpson What's stopping you from writing the driver yourself and hand it out to them? They don't need to worry about the implementation. It's a huge problem for the SW industry that people pick up bad habits when they study and then later on use those in production code. \$\endgroup\$
    – Lundin
    Oct 19 '16 at 14:25
1
\$\begingroup\$

Double check which switch you're intending to use and which bit that corresponds to. The comments near the top suggest that SW2 is associated with bit 0 and SW1 is associated with bit 4. In portF_init() you set GPIO_PORTF_DEN_R = 0x14;, which suggests that you intend to use SW1. But then in the while loop in main() you test if( GPIO_PORTF_DATA_R == 0x05), which reads SW2.

Regarding the delay issue, I wouldn't expect the simulator to exhibit the same timing as the real hardware system. It does not surprise me that the simulator runs slower than the hardware.

\$\endgroup\$
1
  • \$\begingroup\$ Oh, I see now that the questioner expects PF4 to be clear when SW1 is pressed. It's not clear why PF0 is expected to be set though. \$\endgroup\$
    – kkrambo
    Oct 20 '16 at 18:56
0
\$\begingroup\$

I believe you need to enable the internal pull-up resistor for PF4 (SW1). But I suspect the following line does not do so.

GPIO_PORTF_PUR_R = 0x01;          // disble pull-up resistor

Should it be this instead?

GPIO_PORTF_PUR_R = 0x10;          // enable PF4 pull-up resistor

Also, the following conditional relies on status of every bit of Port F when you are really only interested in two of the eight bits. (BTW, it's not clear why you expect PF0 to be set when you didn't configure/enable PF0.)

      // if switch PF4 is pressed and LED is ON (00000101)
    if( GPIO_PORTF_DATA_R == 0x05)

You should rewrite this so it's not dependent on the value of the bits you don't care about. Use the bitwise-AND operator to isolate the bits of interest. And I think it would be nice to use helper functions that make it obvious what your intention is. Maybe like this:

#define BLUE_LED_BIT (0x04)
#define SW1_BIT (0x10)

bool IsSwitch1Pressed()
{
    // Bit is clear when switch is pressed.
    return ((GPIO_PORTF_DATA_R & SW1_BIT) == 0);
}

bool IsBlueLedOn()
{
    // Bit is set when LED is on.
    return ((GPIO_PORTF_DATA_R & BLUE_LED_BIT) != 0);
}

int main(void)
{
    ...

    while(1)
    {
        delay100ms(1);

        if( IsSwitch1Pressed() && IsBlueLedOn() )
        {
            // Turn LED off (clear bit)
            GPIO_PORTF_DATA_R &= ~BLUE_LED_BIT ;
        }      
        else
        {
            // Turn LED on
            GPIO_PORTF_DATA_R |= BLUE_LED_BIT ; 
        }
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ One thing I still don't get it, though. About bit 0, when I debugged the program on the simulator and checked the value of GPIO_DATA I always found bit 0 to be set (1) even though I didn't configure or enable it. Why is this happening? \$\endgroup\$ Oct 21 '16 at 10:18
  • \$\begingroup\$ I don't know why PF0 is 1. Ask your professor. One suspicious thing is that PF0 is a "special consideration" GPIO pin as discussed in the datasheet on page 650 and 659. \$\endgroup\$
    – kkrambo
    Oct 21 '16 at 11:24
0
\$\begingroup\$

You are changing incorrect port register bits.

  • Port F register should be 0x10 (0001.0000) and not 0x05 (0000.0101) or 0x01 (0000.0001), if you want to change PF4.
  • To change PF2 port register value will be 0x02 (0000.0010), register value in your code is 0x04 (0000.0100).
  • Adding both together will make register value to be 0x12 (0001.0010). This is for the case when SW1 (PF4) is pressed and PF2 (Blue LED) HIGH.
  • Similarly, register will be 0x10 (0001.0000) for the case when SW1 (PF4) is pressed and PF2 (Blue LED) LOW.

Check datasheet page 660 and page 662 for more information about how these register bits are changed for the above problem.

For delay issue: The delay is around 76 ms and is incorrect. You need to use a precise timer for the same. Probably, ask your professor about why the function is written like that. Depending upon the IDE you are using this delay calculation might vary. Same will be observable when you program the board with the same code.

Your code might be behaving inappropriately as incorrect bits are being manipulated. It is also suggested that you separate reading input and writing output, if you are a beginner. Implement both separately and then try to club together.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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