1
\$\begingroup\$

I am trying to implement a small piece of code, and can basically be simplified to the following.

I have three different LEDs (say P0.0, P0.1 & P0.2). I have one push button that when pressed should first switch on the LED at P0.0. When pressed again should switch on P0.1 and so on. The LED is to remain on until the next push button is detected.

In the following test code, I am referring to each LED as a function. My idea was to increment a counter and depending on the count, light up a particular LED.

The output of this code is not what I expect. There is no transition between functions. Any suggestions on how I could restructure this code would be helpful.

I do not with to use interrupts for this code. Also, I have not included debouncing as I am targeting to be able to reproduce this code from scratch in 15 minutes.

else if ( SW1 != (0x00000002) )  //when pressed     
    {
    delay();  //for debouncing
    count++;
    }   

    if (count == 1) {
        IO0SET = (1<<0);
        pattern1();
}
 else   if (count == 2) {
        IO0SET = (1<<6);
        pattern2();
}
     else   if (count == 3) {
        IO0SET = (1<<7);
        pattern3();
}

Is there a way in which I can break out immediately of the current function, instead of having to wait for it to finish?

|improve this question
\$\endgroup\$
  • 1
    \$\begingroup\$ You're reading the switch state once, at the start of your main while(1) loop, but you seem to have lines of code lower down that are waiting for it to change. Your SW1 variable won't change until you read it from IO0PIN again. \$\endgroup\$ – Finbarr Jun 7 '17 at 14:03
  • \$\begingroup\$ In addition to Finbarr's point, you also don't wait for the switch to be released before the while, so your while condition will always be false because the code will reach there before the switch is released. \$\endgroup\$ – brhans Jun 7 '17 at 14:05
  • \$\begingroup\$ Thanks for both of your comments. I will try to implement some changes accordingly. \$\endgroup\$ – John Smith Jun 7 '17 at 14:10
2
\$\begingroup\$

It's not going to work without some basic debouncing which if you don't want to use interrupts means a basic wait function unless you debounce in hardware.

The intended functionality of your code seems to be to call the set LED functions constantly while the button is pressed, going by your description there is no need to do this, you can call them once and then wait for the button to be released. They will remain lit until a different LED setting function is called.

Finally if you are ANDing the IO register with 0x02 then the result is either 2 or 0. Rather than checking for a specific value you can use the c convention that 0 is false and all other values are true, this makes the code a lot cleaner looking. Moving that into a function makes it even nicer to read.

int SW1Pressed (void) {
  return !(IO0PIN & 0x00000002) //switch connected on P0.1. Low when pressed.
}

....

count = 0;
while (1){
  if (SW1Pressed()) {                         //when pressed
    wait_ms(100);                             // wait 100 ms
    if (SW1Pressed()) {                       // still pressed.
      if (count==0)                           // turn on correct led
        function1();
      else
        function2();

      if (++count == 2)                       // update count
        count = 0;

       while(SW1Pressed())                  // do nothing until the button is released
         ;
    }    
  }
}
|improve this answer
\$\endgroup\$
  • \$\begingroup\$ Hi Andrew, thanks for your insight on this topic. I updated my code in the original question. The code is working as expected. On each switch press I go to the next function. However, the function needs to finish. So if I have some sort of LED pattern which may take 30 seconds, I need to wait before it changes. Any idea how I can switch immediately? Maybe a switch-case statement? \$\endgroup\$ – John Smith Jun 7 '17 at 15:25
  • \$\begingroup\$ @JohnSmith Make your display patterns work in the background. You can do this by driving a state machine with a timer, for example. The alternative is to "salt" your pattern routines with key-monitoring code and I consider that to be "very bad" in the long run. It's a hack. It might solve your immediate problem "within some idea of reason", but it's not a good way to go because it all just turns into a nightmare rats' nest as you develop more code. Never go down that rabbit hole. \$\endgroup\$ – jonk Jun 7 '17 at 15:42
  • \$\begingroup\$ @jonk, I am not 100% sure I understand you correctly. However I will look these methods up and try to implement them. \$\endgroup\$ – John Smith Jun 7 '17 at 16:01
  • 2
    \$\begingroup\$ @JohnSmith If you are not familiar with the ideas, you should become familiar. They a vital tool. Break down your patterns into the smallest time period between each (or as the largest common factor [look up GCD].) Then implement a timer at that rate. So if a pattern used 15ms, 12ms and 30ms, your GCD would be T=3ms. So you'd set your timer for that. Then your timing is 5T, 4T, and 10T. Easy to set counters for integers. When counter of 5 expires you advance to the next state and set up its counter to 4, etc. But it's easy to terminate the pattern now (just stop the timer perhaps?) \$\endgroup\$ – jonk Jun 7 '17 at 16:11
  • 1
    \$\begingroup\$ @JohnSmith Your new code needs to wait until the switch is released. Not an issue if the LED pattern takes 30 seconds but it will be once you fix that. As Jonk indicated the best way to exit the LED pattern is to have a timer interrupt at a fixed rate that updates the LEDs. Your button code can then either start/stop that timer or set a variable that the timer checks to see which pattern it should be showing. \$\endgroup\$ – Andrew Jun 7 '17 at 16:19
1
\$\begingroup\$

I think the basic problem is that your code does not debounce the switch. When pressing a mechanical switch, it will initially rapidly chatter open and closed. This will cause your code to step through the various functions rapidly.

Your code has unnecessary condition checks that could be cleaned up as well. Basically, you want to wait for the button to be pressed, check if it still pressed say 75-100 mS later and if so, wait for the button to be released and then increment your function counter and go.

|improve this answer
\$\endgroup\$
  • \$\begingroup\$ Hi, proceeded to do what you said. I will update my code. Working well, except that when I press the switch, the function still has to be done before going onto the next function. Is there any way that I can break out immediately upon switch press? \$\endgroup\$ – John Smith Jun 7 '17 at 15:24
  • \$\begingroup\$ The logic to control the various LEDs could be something like If Count ==1 then do this. If Count ==2 then do this. If Count ==3 then do this. At the bottom of these condition statements, simply go back to the monitoring of the push button. \$\endgroup\$ – Glenn W9IQ Jun 7 '17 at 15:32
  • \$\begingroup\$ is the new edited code what you are talking about? I arranged the coding and everything is working well, except that I cannot seem to switch between functions immediately. This mean that I have to wait out the current function to finish before the switch press is registered. \$\endgroup\$ – John Smith Jun 7 '17 at 15:34
  • \$\begingroup\$ Normally putting the switch on an interrupt input would be the way to handle this. An alternative would be to use an internal timer interrupt to check the switch. A less attractive alternative is to call the switch check function periodically from within your longer functions (especially during any internal wait loops). \$\endgroup\$ – Glenn W9IQ Jun 7 '17 at 16:06

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.