Leonardo Keyboard & SHIFT - Best Practice Question

Hi all,
First time here, just got my new Leonardo and I'm working on a keyboard project.
I'm interested in best practices for holding down SHIFT / CTRL / ALT + KEY.

Here's a demo:

void setup() {
  // make pin 2 & 4 inputs and turn on the 
  // pullup resistor so it goes high unless
  // connected to ground:
  pinMode(2, INPUT_PULLUP);
  pinMode(4, INPUT_PULLUP);
  Keyboard.begin();
}

void loop() {
  //if the button is pressed
  if(digitalRead(2)==LOW){
    Keyboard.write('a');
  }
  //if the button is pressed
  if(digitalRead(4)==LOW){
    Keyboard.press(KEY_LEFT_SHIFT);
  }
  if(digitalRead(4)==HIGH){
    Keyboard.release(KEY_LEFT_SHIFT);
  }
  //delay to slow things down a little on screen reading
  delay(50);
}

Quite simply:
pin2: Input 'a'.
pin4: Press LEFT SHIFT, so 'a' becomes 'A' when combined with pin2.

I'm concerned about the .press and .release.
Is what I have above the best possible code for that, it doesn't feel write, having both if statements for HIGH vs LOW on pin4.
It feels like having Keyboard.release repeat in that loop would be a waste of resources.

Any advice would be greatly appreciated.
Thank you! 8)

You probably want to look at the state change detection example and press and release the shift key only when there is a change in the state of the switch, not every time the switch state is checked.

Thanks PaulS, just had a look through, here's my first revision.
Works fine, probably something wrong with it. But here you go:

const int  buttonPin = 2;   
const int shiftPin = 4;
const int ledPin = 13;     
int buttonState = 0;        
int lastButtonState = 0;     

void setup() {
  pinMode(buttonPin, INPUT_PULLUP);
  pinMode(shiftPin, INPUT_PULLUP);
  pinMode(ledPin, OUTPUT);
  Keyboard.begin();
}


void loop() {
  if(digitalRead(buttonPin)==LOW){
    Keyboard.write('a');
  }
  
  buttonState = digitalRead(shiftPin);
  if (buttonState != lastButtonState) {
    if (buttonState == LOW) {
      digitalWrite(ledPin, HIGH);
      Keyboard.press(KEY_LEFT_SHIFT);
    } 
    else {
      digitalWrite(ledPin, LOW);
      Keyboard.release(KEY_LEFT_SHIFT);
    }
  }
  
  lastButtonState = buttonState;
  delay(50);
}

Any suggestions?
Thank you so much!

Any suggestions?

Yes. You have:

  buttonState = digitalRead(shiftPin);
  if (buttonState != lastButtonState) {
    if (buttonState == LOW) {

You have another variable called buttonPin. The state variables associated with the shiftPin variable should be called something like currShiftState and lastShiftState. It's confusing to see mixed names like you have.

  if(digitalRead(buttonPin)==LOW){
    Keyboard.write('a');
  }

Do you want to keep writing 'a' over and over, or do you want to write one a per switch press?

aha!
Good point about the naming there.

Yes, I would like to have it continuous.
But, what would you suggest for a single key press?

But, what would you suggest for a single key press?

The same as you are doing for the shift key. Test for a change in state, then press or release the key, depending on the current switch state (pressed or released).