Can't break out of a loop with a keypad

I’m using Arduino’s keypad library on a 3x4 keypad that selects which RGB LED pattern function to run. For the LED patterns, I’m using Adafruit’s NeoPixel library. This all works fine - when a pattern finishes, I push a button and that pattern runs. My next step is to set each pattern to loop infinitely and to break out of each loop with the press of any key which will then take me to another function - this is what’s not working. For testing, I haven’t set up the infinite loops yet, I’m just trying to break out of the longest LED function to give me time to try a number of keys on the the keypad first.
All the example breaks I’ve found use just one button, not a keypad of buttons. They also reference the specific pin the button is connected to, not a variable, which I suspect would be cleaner and more desirable. What am I missing?
The two commented if statements in ‘void rainbowCycle’ are variations I’ve tried. I’ve stripped the whole program down here to just using 8 on the keypad to initiate the LED pattern and 0 to ‘kill’ it to keep down the size.

#include <Keypad.h> // library for 12-key keypad
#include <Adafruit_NeoPixel.h> // library for driving WS2812B LEDs.
#define PIN 2 // data line out to LED string

// Keypad config
const byte rows = 4; //four rows
const byte cols = 3; //three columns
char keys[rows][cols] = {
  {'1','2','3'},
  {'4','5','6'},
  {'7','8','9'},
  {'#','0','*'}
};
byte rowPins[rows] = {6, 7, 8, 9}; //connect to the row pinouts of the keypad
byte colPins[cols] = {10, 11, 12}; //connect to the column pinouts of the keypad
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, rows, cols );

// WS2812B LED config
Adafruit_NeoPixel strip = Adafruit_NeoPixel(3, 2, NEO_GRB + NEO_KHZ800);

void setup(){
  strip.begin();
  strip.show(); // Initialize all pixels to 'off'
  keypad.addEventListener(keypadEvent); // Add an event listener for keypad
}

void loop(){
    char key = keypad.getKey();
    if (key) {
        keypadEvent(key);
    }
}

// Program selection
void keypadEvent(KeypadEvent key){
    switch (keypad.getState()){
    case PRESSED:
    if (key == '8') {
      rainbowCycle(2);
      }
    if (key == '0') {
      colorWipe(strip.Color(0, 0, 0), 0); // 'all LEDs off'
      }
    break;
    }
}

// Fill the dots one after the other with a color
void colorWipe(uint32_t c, uint8_t wait) {
  for(uint16_t i=0; i<strip.numPixels(); i++) {
      strip.setPixelColor(i, c);
      strip.show();
      delay(wait);
  }
}

void rainbowCycle(uint8_t wait) {
  uint16_t i, j;
  for(j=0; j<256*5; j++) { // 5 cycles of all colors on wheel
    for(i=0; i< strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
    }
    strip.show();
    delay(wait);
       // if(digitalRead(10 == LOW)) { break; }
       // if(keys == LOW) { break; }
  }
}

// Input a value 0 to 255 to get a color value.
// The colours are a transition r - g - b - back to r.
uint32_t Wheel(byte WheelPos) {
  if(WheelPos < 85) {
   return strip.Color(WheelPos * 3, 255 - WheelPos * 3, 0);
  } else if(WheelPos < 170) {
   WheelPos -= 85;
   return strip.Color(255 - WheelPos * 3, 0, WheelPos * 3);
  } else {
   WheelPos -= 170;
   return strip.Color(0, WheelPos * 3, 255 - WheelPos * 3);
  }
}
void loop(){
    char key = keypad.getKey();
    if (key) {
        keypadEvent(key);
    }
}

Why? The keypadEvent() function will be called when a key is pressed, because you registered an event handler.

       // if(digitalRead(10 == LOW)) { break; }

What pin is 10 == LOW?

How does reading pin 10, if that's what you were trying to do, relate to breaking out of the loop when a key on the keypad is pressed? Why are you reading from pin 10, if that is what you are trying to do, when pin 10 is used by the keypad?

Why? The keypadEvent() function will be called when a key is pressed, because you registered an event handler.

From the EventKeypad.ino example from Arduino, I guess that’s how I understood this needed to be to work - I was replacing their “Serial.println(key);” statement with the keypadEvent() function. I guess it should it be…

void loop(){
    char key = keypad.getKey();
 }

…correct? It seems to work that way. And I think I’m beginning to understand why. I thought the ‘if’ needed to be looped in order to listen to the keypad.

What pin is 10 == LOW?

How does reading pin 10, if that’s what you were trying to do, relate to breaking out of the loop when a key on the keypad is pressed? Why are you reading from pin 10, if that is what you are trying to do, when pin 10 is used by the keypad?

I thought I should be reading the pins that the keypad is connected to in order to check their state. I had tried three separate copies of that line for each of the pin numbers the columns were connected to…

if(digitalRead(10 == LOW)) { break; }
if(digitalRead(11 == LOW)) { break; }
if(digitalRead(12 == LOW)) { break; }

…as well as (separately) four lines for the four rows of the matrix…

if(digitalRead(6 == LOW)) { break; }
if(digitalRead(7 == LOW)) { break; }
if(digitalRead(8 == LOW)) { break; }
if(digitalRead(9 == LOW)) { break; }

I also tried testing for HIGH which only ever resulted in the function stopping after only the first set of colors were sent to the LEDs (vs. the several sets the function usually delivers). I did this as I figured that one of the lines in a column or row had to go HIGH or LOW when a button was pushed. I couldn’t find anything that said if the keypad library pulled rows or columns high or sank them to ground or what.

That’s why I also tried the line after that (separately) as it made more sense to look for a change in a variable rather than the pins which had already been defined. I’ve been searching through forums, examples and the documentation to try and figure out what the program is supposed to be ‘listening’ to in order to execute the break, but I can’t find anything or I’m just not getting my head wrapped around whatever it is I need to understand.

if(digitalRead(10 == LOW))

That's one of those pesky syntax errors that will still compile but not do what you think it should. If I'm not mistaken it will read pin 0 because 10 != LOW so the comparison is false which is == 0. I'm sure you want to compare the result of reading pin 10 with LOW. Which brings up something else, what's at pin 10? Give it a better name than 10. Assign 10 to a const variable name and use the name.

Assuming pin 10 is a button switch.

const byte BUTTON_PIN = 10; // now pin ten has a name
// read it and make a comparison
if (digitalRead(BUTTON_PIN) == LOW) {break;}

In C++ it helps to have a basic understanding of how functions accept parameters and return values.

Can a pin be assigned to two different names? The keypadEvent function, its setup (keys, rowPins, colPins, etc.) is already assigned pin 10 (actually, pins 6-12) as one input matrix. I would think that there should already be some variable that already contains the state of any two pins (or key). I'm just not sure what it is. I've tried a number of names already (keypad, key, keys, etc.) I was just trying just one of the pins (pin10) to keep things simple till I figured this out.

Can a pin be assigned to two different names?

Sure. But why? It's one physical pin.

The keypadEvent function, its setup (keys, rowPins, colPins, etc.) is already assigned pin 10 (actually, pins 6-12) as one input matrix. I would think that there should already be some variable that already contains the state of any two pins (or key).

No. The getKey() method cycles through the array of pins, setting some HIGH and some LOW, in an attempt to determine if any pair of pins is connected because a key is pressed. If a key is pressed, the function returns which key it was. Otherwise, it returns NO_KEY.

Separately reading one of the pins does not make sense.

OK, I feel like I'm not really getting any closer to a solution. I understand that you can assign more than one name to a pin, but that isn't practical (which I suspected from the beginning but attempted anyway in order to try and find some sort of solution). I understand that testing a pin (or series of) in a matrix of pins doesn't make sense when you have a name for that matrix that you can get a value from - and the only value I can see that's generated is a character being assigned to the name 'key'.

What I don't understand is how is the 'void keypadEvent' function gets called in the first place. There is no call for it in the 'void loop' function - there's just a char assignment from the keypad function to the variable key - but somehow it happens.

I also don't understand exactly what I should be testing for so that I can break out of a function and return to the 'void loop' so that another function can be called, according the the new value in 'key'.

Again, I am trying to test for a change on the keypad (when any key is pressed) so that, when that event happens, a different function (or even the same one as what's currently running; it doesn't matter) can be called. This test would have to take place inside of every LED pattern generating function (of which there are several) while that generator is looping. Once this works, then I can take all this to the next level and change all my LED pattern generating functions into infinite loops... probably with a 'while' statement which is where I'll put the final interrupt test (if I can ever figure it out).

What I don't understand is how is the 'void keypadEvent' function gets called in the first place.

Because you told the keypad class that you want it to be called when a key is pressed:

  keypad.addEventListener(keypadEvent); // Add an event listener for keypad

You can still call keypad.GetKey() any time you want to, and get the key that is pressed, if a key is being pressed.

I also don't understand exactly what I should be testing for so that I can break out of a function and return to the 'void loop' so that another function can be called, according the the new value in 'key'.

char key = keypad.GetKey();
if(key != NO_KEY)
   break;

Thanks PaulS, I didn't notice the "if (key != NO_KEY){" on the Matrix Keypad Library page. But, now, this almost works...

Pressing another key exits the current routine and starts the newly chosen routine, however, when its done with the new routine, it returns to the previous routine and finishes running that from where it left off. So, if you press several keys while each routine is still running, it will nest routine after routine inside a long chain, returning to each function at the point the break occurred.

In theory, this sounds OK if I'm setting all these functions to run indefinitely, as there would never actually be a return. However, this also means that my solid color routines which only need to send one set of code to the LEDs just once (including my 'all off' routine) would be running indefinitely, when they don't need to. It also means that, since all the previously run routines are still kept in memory, they will keep piling up as I press new keys until I run out of memory. Its just not very clean.

I also tried using return; instead of break; with the same results.

So, you've made some changes to your code, and it's not working as you want. Bummer.

As soon as my crystal ball gets back from the cleaners, I'll have a look.

When egos match IQs, the ignorant learn to fear questions and remain stupid, thus the world is the stupid mess it is. Forums serve the purpose of pointing the ignorant but willing in the right direction, not for bashing others because your suggestion didn't work. Don't be a contributor to world stupidity, PaulS – it means you've chosen to be an idiot (Wikipedia the word – fascinating etymology). Yes, I made some changes to my program – the changes you recommended, they didn't work.

When responding to questions, its best to point people in the right direction; don't give them the final answer, make them think. But another counterproductive behavior of egotists is to only point out problems, rarely solutions. Now, I've politely tried to get you focused on the actual problem repeatedly so I have no idea where your jerk behavior fits in.

Just point me in the right direction, I'm a genius too, I'll figure it out. If you're not interested in helping then why in the world are you here in the first place? Yes, I'm ignorant; everyone is, until they're taught. I'm new to the Arduino world and C but not programming, starting with Basic on a TRS-80 all the way back in 1982. Were you even born yet? So, if you don't mind, PaulS, please stay out of this conversation; you're just wasting my time.

For anyone else whose willing to help, it was my understanding that the break command breaks completely out of a function with no return, but that’s exactly what’s happening. The break gets executed, a new function gets called, but when that function is done, the program returns to the point of the break and finishes the function it broke from. Why does my break not make a clean break?

I’ve been experimenting with return, while, do…while, goto, if…else, all with the same result. Here’s the code as I have it now. Again, I’ve stripped it down to just two functions for clarity; one that runs a pattern of light across the LEDs and one that sets them all to 0s…

#include <Keypad.h> // library for 12-key keypad
#include <Adafruit_NeoPixel.h> // library for driving WS2812B LEDs.
#define PIN 2 // data line out to LED string

// Keypad config
const byte rows = 4; //four rows
const byte cols = 3; //three columns
char keys[rows][cols] = {
  {'1','2','3'},
  {'4','5','6'},
  {'7','8','9'},
  {'#','0','*'}
};
byte rowPins[rows] = {6, 7, 8, 9}; //connect to the row pinouts of the keypad
byte colPins[cols] = {10, 11, 12}; //connect to the column pinouts of the keypad
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, rows, cols );

// WS2812B RGB LED config
Adafruit_NeoPixel strip = Adafruit_NeoPixel(3, 2, NEO_GRB + NEO_KHZ800);

void setup(){
  strip.begin();
  strip.show(); // Initialize all pixels to 'off'
  keypad.addEventListener(keypadEvent); // Add an event listener for keypad
}

void loop(){
    char key = keypad.getKey();
}

// Program selection
void keypadEvent(KeypadEvent key){
    switch (keypad.getState()){
    case PRESSED:
    if (key == '8') {
      rainbowCycle(2);
      }
    if (key == '0') {
      colorWipe(strip.Color(0, 0, 0), 0); // 'all LEDs off'
      }
    }
}

// Fill the dots one after the other with a color
void colorWipe(uint32_t c, uint8_t wait) {
  for(uint16_t i=0; i<strip.numPixels(); i++) {
      strip.setPixelColor(i, c);
      strip.show();
      delay(wait);
      char key = keypad.getKey();
      if(key != NO_KEY)
        exit(0);
  }
}

void rainbowCycle(uint8_t wait) {
  uint16_t i, j;
  for(j=0; j<256*5; j++) { // 5 cycles of all colors on wheel
    for(i=0; i< strip.numPixels(); i++) {
      strip.setPixelColor(i, Wheel(((i * 256 / strip.numPixels()) + j) & 255));
    }
    strip.show();
    delay(wait);
    char key = keypad.getKey();
    if(key != NO_KEY)
      exit(0);
  }
}

// Input a value 0 to 255 to get a color value.
// The colours are a transition r - g - b - back to r.
uint32_t Wheel(byte WheelPos) {
  if(WheelPos < 85) {
   return strip.Color(WheelPos * 3, 255 - WheelPos * 3, 0);
  } else if(WheelPos < 170) {
   WheelPos -= 85;
   return strip.Color(255 - WheelPos * 3, 0, WheelPos * 3);
  } else {
   WheelPos -= 170;
   return strip.Color(0, WheelPos * 3, 255 - WheelPos * 3);
  }
}

it was my understanding that the break command breaks completely out of a function with no return

A break stops the current statement. If you have nested for and/or while loops, it only breaks out of the one that executed the break statement.

I don't know why my suggestion that you needed to post you code again irritated you so badly, but I won't be responding to you again.