0

I am trying to turn on LED by pressing and releasing the button once and turn it off by pressing and releasing it again and so on. I wrote a bunch of code together but I'm not sure if it sounds right. I mean I don't know if it's my breadboard or loose connection but not sure if it works as expected. I know that the wiring is correct as simply turning it on while pressing the button and turning it off by releasing the button works. Any help is much appreciated. Thank you.

Here is the code:

//Reset and clock control - Advanced high-performance bus - Enabling GPIO Port C pin 6 and Port B pin 1
RCC -> AHBENR |= RCC_AHBENR_GPIOCEN;
RCC -> AHBENR |= RCC_AHBENR_GPIOBEN;

//Setup Control Registers for the LED output
//Mode register as Output
GPIOC -> MODER |= GPIO_MODER_MODER6_0;
GPIOC -> MODER &= ~(GPIO_MODER_MODER6_1);
//OtypeR - Push pull
GPIOC -> OTYPER &= ~(GPIO_OTYPER_OT_6);
//OspeedR - High
GPIOC -> OSPEEDR |= GPIO_OSPEEDER_OSPEEDR6;
//PUPDR
GPIOC -> PUPDR &= ~(GPIO_PUPDR_PUPDR6);

//Setup control registers for the push button input
//Mode register as input
GPIOB -> MODER &= ~(GPIO_MODER_MODER1);
//Pull up pull down register
GPIOB -> PUPDR &= ~(GPIO_PUPDR_PUPDR1); // Connected to ground externally (no need for internal pupdr
int counter = 0;

  while (1)  {
  //If the button is pressed (IDR - input data register)
  if((GPIOB -> IDR & (GPIO_IDR_1)) && counter == 0) //If button is pressed
  {
      GPIOC -> BSRR |= GPIO_BSRR_BS_6; //Turn ON the LED
      if(~(GPIOB->IDR &(GPIO_IDR_1))) // If the button is released
      {
          GPIOC -> BSRR |= GPIO_BSRR_BS_6; //LED stays ON
      }
  }
  counter = 1;

  if((GPIOB -> IDR & (GPIO_IDR_1)) && counter == 1) //If button is pressed
  {
      GPIOC -> BRR |= GPIO_BRR_BR_6; //Turn OFF the LED
      if(~(GPIOB -> IDR &(GPIO_IDR_1))) // If the button is released
      {
          GPIOC -> BRR |= GPIO_BRR_BR_6; //LED stays OFF
      }
  }
  counter = 0;
  }
zRage4
  • 39
  • 2
  • You should set the counter inside the inner if conditions. – Thomas Sablik Jan 20 '21 at 07:50
  • Why not base your decisions on `if (B interrupt) { if (counter) ... else ... counter = !counter }` and the same for `if (C interrupt) { /* same logic */ }` That way you are only responding when an interrupt is ready, even though you essentially duplicate the turn on/off logic inside each. Have you considered enabling an interrupt handler -- if they have them on the stm32? – David C. Rankin Jan 20 '21 at 07:52
  • I haven’t started interrupts on stm32 yet. Wouldn’t this code still work without the interrupt? – zRage4 Jan 20 '21 at 07:57
  • Sure, the same logic will work no matter how you trigger the conditions. You can read directly from a pin instead of using interrupts. – David C. Rankin Jan 20 '21 at 08:47
  • So, does this code make sense? Because I do see it turning on and off but I feel it skips it many times. For example, I’ll press and release once and it will turn on and then I’ll press and release again and it will still stay on and same thing the third time and then finally it will turn off. It’s completely random. Is this button denouncing, loose connection, or problem with the code? – zRage4 Jan 20 '21 at 08:52
  • I don't have a stm32, so the details escape me (I do have a MSP432, so the syntax is similar) Presuming you have correctly set the pin selection for input/output correctly, the pin direction, and resistor enable correctly then nothing stands out as being incorrect. Using any MCU is a deep dive into the tech-reference and data-sheet and learning the pin nomenclature and configurations (it is a lot at first), but then it will all work as advertised. st does a great job with documentation (sometimes too great with 1000s of pages of reference material `:)` – David C. Rankin Jan 20 '21 at 19:38
  • 1
    "Switch debouncing" is the keyword, please search it. – recep Jan 21 '21 at 04:54
  • dont use interrupts, not until you fully understand this, and dont interrupt on the gpio state change, period, never. To understand and see what is going on, first learn about the uart and be able to print numbers out. Then instrument some code that say counts the state changes of the gpio input tied to the button. Then every so often print that out on the uart (can use a debugger too sure). You will see that a single button press is not going to be exactly two state changes it could be a lot or a little, but just two is not expected. – old_timer Jan 24 '21 at 02:13
  • "dont interrupt on the gpio state change, period, never". Could you elaborate ? it is interesting. – Guillaume Petitjean Jan 25 '21 at 10:48

1 Answers1

1

You have several problems.

The first one might be linked to debouncing. When you press the button, the electrical signal is actually oscillating between two values and at the end, what your code will consider as "button is pressed a second time" might just be the bouncing of the switch.

Also, when your code detects a button press, it doesn't wait for the button to be released, it is only turning the LED on and immediatly checking whether the button has been released. It will not work in all cases (anyway your code is doing nothing new in if(~(GPIOB->IDR &(GPIO_IDR_1))) ...).

Then it will probably not work anyway because the way you coded it, you will lost many "press" events. Use interrupts instead.

Guillaume Petitjean
  • 2,408
  • 1
  • 21
  • 47
  • Interrupts on buttons are a huge problem. If you use interrupts you use them with a timer to periodically sample the button...Which polling will serve perfectly fine as well while learning or in some applications with a greater chance of success. If you cant solve it with polling you will fail with interrupts as well, plus the risk of figuring out interrupts. – old_timer Jan 24 '21 at 02:10
  • are you referring to the debouncing issue ? I agree it is tricky but are there other issues ? (in managing buttons with interrupts) – Guillaume Petitjean Jan 25 '21 at 10:46