Struggling with "if... else if... else" to change the function of a keypress

Using IDE 1.8.1 and an Arduino Pro Micro. Start button on pin 4 and macro key on 16.

Scenario is a video game controller with a start button and a macro key.
Normally they each have their individual functions; start is a start button and the macro key sends a pin code with the keyboard library, to alleviate the need of having an external numpad plugged into the game.

But
I want to change the function of the macro key by holding down the start button. This is where I'm having trouble. I've tried multiple iterations, lots of googling on the use of the if and else statements, etc. etc, so now I am coming to the gurus.

With this current code, the start and macro keys still work separately, but I am not getting any numpad output when I press start + macro key. (I tried changing from "if... else if... else" to two "if...else" statements to try to get it to work. Hopefully someone can show me how I should do it properly lol.

Edit: got rid of the unimportant stuff in the code to make this post shorter.

#include "Keyboard.h"
iivxReport_t report;

#define REPORT_DELAY 2000

// Options, change only these values
int old_something;
int old_something2;
uint8_t buttonCount = 7;
uint8_t macropin = 16;


uint8_t buttonPins[] = { 4, 5, 6, 7, 8, 9, 10};




void setup() {
  
  delay(1000);
  // Setup I/O for pins
  for (int i = 0; i < buttonCount; i++) {
    pinMode(buttonPins[i], INPUT_PULLUP);
    pinMode(ledPins[i], OUTPUT);
  }
    pinMode(macropin, INPUT_PULLUP);
  //  pinMode(sysPin,INPUT_PULLUP);
  //setup interrupts
}

void loop() {
  // Read buttons
  for (int i = 0; i < buttonCount; i++) {
    if (digitalRead(buttonPins[i]) != HIGH) {
      report.buttons |= (uint16_t)1 << i;
    } else {
      report.buttons &= ~((uint16_t)1 << i);
    }
  }

int something = digitalRead(macropin);
int something2 = digitalRead(4);
 if (digitalRead(4) == HIGH) { //if start button is not pressed
  //macro key
  if (something != old_something) { //if we think macro was pressed (I use this so the button doesn't spam output)
   if (digitalRead(macropin) == LOW){ // if macro pin is pressed
   Keyboard.begin();
   Keyboard.press(223);//numpad +
   delay(50);
   Keyboard.release(223);
   delay(50);
   Keyboard.press(225); //numpad 1
   delay(50);
   Keyboard.release(225);
   delay(50);
   Keyboard.press(226); //numpad 2
   delay(50);
   Keyboard.release(226);
   delay(50);
   Keyboard.press(227); //numpad 3
   delay(50);
   Keyboard.release(227);
   delay(50);
   Keyboard.press(228); //numpad 4
   delay(50);
   Keyboard.release(228);
   Keyboard.end();// end keyboard
   
   } else {
  Keyboard.end();
   }
  }
 }

if (digitalRead(4) == LOW) { //if start was pressed, then you can do this
 if (something2 != old_something2) { //if we think start was pressed (I use this so the button doesn't spam output)
    if (something != old_something) { //if we think macro was pressed (I use this so the button doesn't spam output)
    if (digitalRead(macropin == LOW)) { //if macro was pressed
      Keyboard.begin();
      Keyboard.press(230);
      delay(50);
      Keyboard.release(230);
      Keyboard.end();
       } else {
        Keyboard.end();
       } 
       }
    }
   }
 
 old_something = something; // save the current state as the last state, for next time through the loop
 old_something2 = something2;


  // Send report and delay
  iivx.setState(&report);
  delayMicroseconds(REPORT_DELAY);
}

First off, naming a variable 'something' or 'something2' etc. is terrible. Variable names are meant to help humans understand the code. Try something more descriptive like 'startButtonState' or 'macroButtonState'

Have a look at the StateChangeDetection example (File->examples->02Digital->StateChangeDetection) because you want to react to when the buttons change state, you don't really care about their current state.

You should be able to adapt that to your code.

Will do, thanks!

blh64:
First off, naming a variable 'something' or 'something2' etc. is terrible. Variable names are meant to help humans understand the code. Try something more descriptive like 'startButtonState' or 'macroButtonState'

Have a look at the StateChangeDetection example (File->examples->02Digital->StateChangeDetection) because you want to react to when the buttons change state, you don't really care about their current state.

You should be able to adapt that to your code.

Okay I've re-written the code with the suggestions. Not sure if I should be using two separate if statements or have them both in an if-else if-else function. But let's focus on the first problem... I can't even get the start button + macro key output to work. Here's the code for that, minus "void setup()"

#include "Keyboard.h"

int old_macroButtonState;
int old_startButtonState;
uint8_t buttonCount = 7;
uint8_t macropin = 16;
uint8_t buttonPins[] = { 4, 5, 6, 7, 8, 9, 10};

void loop() {

int macroButtonState = digitalRead(macropin);
int startButtonState = digitalRead(4);
 if (startButtonState != old_startButtonState) {
  if (digitalRead(4) == LOW) { //if start button is pressed
    if (macroButtonState != old_macroButtonState) { //compare the button states (from StateChangeDetection Example)
      if (digitalRead(macropin) == LOW){ // if macro pin is pressed
      Keyboard.begin();
      Keyboard.press(230);
      delay(50);
      Keyboard.release(230);
      Keyboard.end();
       } else {
        Keyboard.end();
       } 
    }
 
  }
 }
 
}

This is probably closer to what you want to do, but one BIG issue is old_startButtonState is never initialized NOR updated.

#include "Keyboard.h"

int old_macroButtonState;
int old_startButtonState;
uint8_t buttonCount = 7;
uint8_t macropin = 16;
uint8_t buttonPins[] = { 4, 5, 6, 7, 8, 9, 10};

void loop()
{
  int macroButtonState = digitalRead(macropin);
  int startButtonState = digitalRead(4);

  if (startButtonState != old_startButtonState) //compare the button states (from StateChangeDetection Example)
  {
    if (digitalRead(macropin) == LOW) // if macro pin is pressed
    {
      Keyboard.begin();
      Keyboard.press(230);
      delay(50);
      Keyboard.release(230);
      Keyboard.end();
    } 
    else
    {
        Keyboard.end();
    }
  }
}

Sorry, I forgot to include that I DO have these two lines at the end of the code to update the values:

Should I update the top two lines to be like: int old_macroButtonState = 0; ??

old_macroButtonState = macroButtonState; // save the current state as the last state, for next time through the loop
old_startButtonState = startButtonState; // save the current state as the last state, for next time through the loop

I tried your snippet of code, but it only sends the keypress if I let go of the start button while still holding the macro key. Weirdness

cam_prevail:
I tried your snippet of code, but it only sends the keypress if I let go of the start button while still holding the macro key. Weirdness

I didn't edit your code for logic, I simply got rid of the two redundant if statements.

OH WAIT. I see now - I misread the contents of the second set of if statements, they aren't redundant, sorry

I'm puzzled here lol.

OKAY, I tried a few more combinations and I noticed I was getting reverse results. So I reversed the whole operation and it worked as intended, smh.

if (macroButtonState != old_macroButtonState) {
  if (digitalRead(macropin) == LOW) { //if macro button is pressed
   // if (macroButtonState != old_macroButtonState) { //compare the button states (from StateChangeDetection Example)
      if (digitalRead(4) == LOW){ // if start pin is pressed
      Keyboard.begin();
      Keyboard.press(230);
      delay(50);
      Keyboard.release(230);
      Keyboard.end();
       } else {
        Keyboard.end();
       } 
    }