Pages: [1]   Go Down
Author Topic: Keypad Library Bugs.  (Read 1733 times)
0 Members and 1 Guest are viewing this topic.
England
Offline Offline
Newbie
*
Karma: 0
Posts: 1
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


If you want to use the keypad library there are still a couple of outstanding issues.

Though Nick Gammons changes http://arduino.cc/forum/index.php?topic=58337.0 were partly implemented a slight bug crept in.

Spot the difference.

Suggested:
Code:

void Keypad::initializePins(){
  for (byte r=0; r<size.rows; r++){
    //configure row pin modes and states
    pinMode(rowPins[r],INPUT);
    digitalWrite(rowPins[r],HIGH);
  }
  for (byte c=0; c<size.columns; c++){
    //configure column pin modes and states
    pinMode(columnPins[c],OUTPUT);
    digitalWrite(columnPins[c],HIGH);
  } 
}



In Current Version:
Code:

void Keypad::initializePins(){
  for (byte r=0; r<size.rows; r++){
    //configure row pin modes and states
    pinMode(rowPins[r],INPUT);
    digitalWrite(rowPins[r],HIGH);
  }
  for (byte c=0; c<size.columns; c++){
    //configure column pin modes and states
    pinMode(columnPins[c],OUTPUT);
    digitalWrite(columnPins[c],LOW);
  } 
}


This means before every scan. All column outputs are taken low. Turning on all columns.

Then during every scan for each column it is taken low again (allready in that state) before scanning each row. Once each row has scanned it takes it high before doing the next column. Basically they start all on and are turned off one by one. So when it scans first column it will pick up keys from any column. On subsequent columns if basically picks up that column and all buttons to the right.

That means that any button press has the equivalence of pressing all buttons to its left as well. Fortunately this still works miraculously as the right most key is the last one to be scanned and therefore the only one remembered.

NO FOR MORE SERIOUS ISSUE:

It is claimed that the library does not need diodes. This would be true if the statement that all ouputs not being pulled low are converted to inputs to make them high impedance, but i see no evidence of this. Now if you pressed two buttons from the same row but on different colums you could short an output in its high state to one in its low state.

So here is how i suggest it can be cured.

initilizePins method should be:

Code:

void Keypad::initializePins(){
  for (byte r=0; r<size.rows; r++){
    //configure row pin modes and states
    pinMode(rowPins[r],INPUT);
    digitalWrite(rowPins[r],HIGH);
  }
  for (byte c=0; c<size.columns; c++){
    //configure column pin modes and states
    pinMode(columnPins[c],INPUT);
    digitalWrite(columnPins[c],HIGH);
  } 
}



scanKeys method should be:

Code:

boolean Keypad::scanKeys() {
static unsigned int allKeys=0;
byte curKey=0;
boolean anyKey;

// Assume that some other device is sharing the data pins used by the
// keypad. If that is the case then the pins will need to be re-intialized
// each time before they are used.
initializePins();

// I rewrote this method to provide a status change (anyKey OPEN/CLOSED) to the
// getKeyState() function which handles debouncing. Now we can scan the keypad
// without having to worry about huge debounce time delays.
for( int c=0; c<size.columns; c++) {
pinMode(columnPins[c],OUTPUT);
digitalWrite(columnPins[c], LOW);
for( int r=0; r<size.rows; r++) {
curKey = digitalRead(rowPins[r]);
allKeys += curKey;
if(curKey==0) currentKey = keymap[c+(r*size.columns)];

// All keys have been scanned. Set 'anyKey' value for use by getKeyState().
if( r==(size.rows-1) && c==(size.columns-1) ) {
if( allKeys==(size.rows*size.columns) )
anyKey = OPEN;
else
anyKey = CLOSED;
}
}
pinMode(columnPins[c],INPUT);
digitalWrite(columnPins[c], HIGH);
}
allKeys = 0;
return anyKey; // Status tells if keys are OPEN or CLOSED.
}



Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 485
Posts: 18806
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

This would be true if the statement that all ouputs not being pulled low are converted to inputs to make them high impedance, but i see no evidence of this. Now if you pressed two buttons from the same row but on different colums you could short an output in its high state to one in its low state.

I think you are right. The untested columns should be set high-impedance.
Logged


Chile
Offline Offline
Sr. Member
****
Karma: 0
Posts: 260
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Just a question: if I´m not interested in simultaneous keypresses, this change will help avoiding any accidental short, if more than one key is pressed in the same row?

Code:
void Keypad::initializePins(){
  for (byte r=0; r<size.rows; r++){
    //configure row pin modes and states
    pinMode(rowPins[r],INPUT);
    digitalWrite(rowPins[r],HIGH);
  }
  for (byte c=0; c<size.columns; c++){
    //configure column pin modes and states
    pinMode(columnPins[c],INPUT);
    digitalWrite(columnPins[c],HIGH);
  } 
}


All pins will be set as input with pull up reistors by default, is that Ok?
Logged

Anaheim CA.
Offline Offline
Faraday Member
**
Karma: 47
Posts: 2892
...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

4 180 ohm resistors in series with the row or column lines would allow sensing and at the same time limit the current to about 27 ma, well within the available current for any port and further not a real issue as the 'shorts' are of 'short' duration... IMO

Doc
Logged

--> WA7EMS <--
“The solution of every problem is another problem.” -Johann Wolfgang von Goethe
I do answer technical questions PM'd to me with whatever is in my clipboard

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 485
Posts: 18806
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

All pins will be set as input with pull up reistors by default, is that Ok?

They might be a bit weak. You might be better off making the changes in the above thread which will (should?) work in all cases, multiple or not.
Logged


Anaheim CA.
Offline Offline
Faraday Member
**
Karma: 47
Posts: 2892
...
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

A bit off course but it might be easier to detect multiple keys with an analog sensing arrangement and use one, possibly 2 if you used a pwm signal to excite the key matrix...

Doc
Logged

--> WA7EMS <--
“The solution of every problem is another problem.” -Johann Wolfgang von Goethe
I do answer technical questions PM'd to me with whatever is in my clipboard

Phillipsburg, NJ
Offline Offline
Full Member
***
Karma: 6
Posts: 184
Author: Matrix Keypad Library
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

it might be easier to detect multiple keys with an analog sensing arrangement and use one, possibly 2 if you used a pwm signal to excite the key matrix...
This sounds like an intriguing idea but I don't understand what you mean by "use one, possibly 2". Could you explain your idea a little more?

I know that there is already an analog driver for reducing the pin count but that doesn't sound like what you are talking about.
Logged

Munich, Germany
Offline Offline
Newbie
*
Karma: 0
Posts: 2
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Sorry, if this is a stupid question but I'm new to developing software for microcontroller and try to understand what's going on. Is there a special reason for using a two-step procedure to set the column pins as inputs with pull-up resistors enabled instead of using pinMode with option INPUT_PULLUP?
Code:
void Keypad::initializePins(){
  for (byte c=0; c<size.columns; c++){
    //configure column pin modes and states
    pinMode(columnPins[c],INPUT_PULLUP);
  } 
}
If there is a reason for the two-step procedure, can someone give me a hint what I might overlook?
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 614
Posts: 49376
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Is there a special reason for using a two-step procedure to set the column pins as inputs with pull-up resistors enabled instead of using pinMode with option INPUT_PULLUP?
You are asking why older code does not make use of newer features. It's simple. Those features didn't exist when the code was written.
Logged

Munich, Germany
Offline Offline
Newbie
*
Karma: 0
Posts: 2
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
It's simple. Those features didn't exist when the code was written.
Thank you for the quick answer -- I haven't been aware that the featur has been added later this year. Sorry!  smiley-red
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 614
Posts: 49376
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

That feature has been around for a short while, but new features can be slow to be adopted. I'm still not used to using the INPUT_PULLUP combination. New code that I write still uses two lines.
Logged

Pages: [1]   Go Up
Jump to: