Writing ISR to read 4-bit output from 74c922 keypad encoder ic

This is my first attempt at writing a function as well as an ISR.
I have a 4x3 membrane keypad that looks exactly like this:

It has the standard "pin-1" arrow on the rightmost pin of the connector with the keypad right side up.
The 74c922 keypad encoder ic is breadboarded with the keypad and leds display the status of the
4 output pins (OUTA-pn17,OUTB-16,OUTC-15,& OUTD-14), as well as DATA_AV-pn-12.
With OE-NOT tied LOW to ground , the keypress data is latched at the output and displayed on the leds.
I am not certain that I have the four ROWS (Y1,Y2,Y3,Y4) and three columns (X1,X2,X3,) connected
correctly but that can wait till later. The 74c922 is a 16-key encoder, NOT 12 , and I THOUGHT I had
ordered a 4x4 keypad but in my haste actually ordered a 4x3. I have a 4x4 on order . I also ordered a
TCM5089 12-KEY keypad encoder IC and xtal as well. My problem right now is that my ISR is clearly
not working. I have attached two files, one that compiles and one that does not. It seems my
problem is that the ISR should return the value (val) from convering the BCD (I think) to an integer.
I know there must be functions and libraries already written to read a simple BCD encoded value
but they look very much like this:

so I adapted this "readSwitch" and changed the function name (that's plagarism isn't it ? yeah, so what?)..
I looked on the forum for how to write a function (here)

but I'm stuck on whether or not the ISR should have the name of the variable it is returning with
in the parenthesis:
ie:

      int readEncoder(total) <===(LIKE THIS)
{
 int total=0;
 if (digitalRead(OutA)==HIGH) { total+=1; }
 if (digitalRead(OutB)==HIGH) { total+=2; }
 if (digitalRead(OutC)==HIGH) { total+=4; }
 if (digitalRead(OutD)==HIGH) { total+=8; }
 return total;
}

The Tronixstuff code does NOT so I left it out.
In any case the program that compiles just returns "0" always, and the other program doesn't compile
so I think I am close but am missing something obvious that anyone with experience would know.
Can anyone help me ?
If I can get it to return some value then I can find address the other issue of why the BCD isn't correct.
The TRUTH TABLE for circuit now is the following;
KEY: D,C,B,A=>DECIMAL
1: L,L,L,L=> 0
2: L,L,L,H=> 1
3 : L,L,H,L=> 2
4: L,H,L,L=> 4
5: L,H,L,H=> 5
6: L,H,H,L=> 6
7: H,L,L,L=> 8
8: H,L,L,H=> 9
9: H,L,H,L=> 10
0: H,H,L,H=> 13
*: H,H,L,L=> 12
#: H,H,H,L=> 14

As you can see, the decimal values omit 3, & 7.
(0,1,2,4,5,6,8,9,10,13,12,14)
and "12" & "13" are swapped.
I have attached:

  1. a photo of the keypad with the adhesive cover paper removed.
  2. a screenshot of the IDE complile error. (I still can't figure out how anyone can copy the compiler output when
    the right-click /copy menu doesn't open when you right click.
  3. the program that compiles ok but always returns "0"
  4. the program with the compile error

keypad_4x3_COMPILES_ERROR_2.ino (938 Bytes)

keypad_4x3_COMPILES_OK.ino (1.13 KB)

ISRs take no arguments, the hardware doesn't pass anything. They talk to the rest of the system through
global volatile variables. Similarly they return no result (where would it go, your code didn't call it)

Can you tell me how to fix the ISR in the program that won't compile ?

As Mark said, no args and no return, you communicate using global variables. Something like this.

volatile int total=0;
volatile bool newKey = false;

void readEncoder() {
 total = 0;
 if (digitalRead(OutA)==HIGH) { total+=1; }
 if (digitalRead(OutB)==HIGH) { total+=2; }
 if (digitalRead(OutC)==HIGH) { total+=4; }
 if (digitalRead(OutD)==HIGH) { total+=8; }
 newKey = true;
} 

loop () {

   if (newKey) {
      Serial.print(total);
      newKey = false;
   }
}

BTW part of the process of designing embedded stuff is to try and optimise the hardware for the software, if you use appropriate pins on the processor you can often remove all that digitalRead() crap with a single

total = PIND & 0xF;

Rob

Thanks for the reply. I tried the ISR and I got a compiler error saying ISO c++ does not allow declaration
of a loop with no type, so I added "void" in front of it to get rid of the error. If that renders the code
inoperable then I don't know what to do. I added the pinMode statements for the INT pin-2 and a
some stuff for OE/NOT (could have just jumpered it to ground but I decided to add it anyway. I checked
it and it was low evern before I added it. The data is available but nothing prints to the screen when
I press a button. I attached a program that I have been working on that prints all the numbers
and corrects the incorrect BCD code and it works for all of them except 1, 2 & 3. The Terminal capture
program is attached as well. The Arduino IDE doesn't have capture to file so I use Clear Terminal.
You can see the values for the four lines. I weight them with code (if "D==HIGH, D=8;) and this works
to change the truth table in my first posting to one where all the keys are correct (0 to 11 | 10=* and 11=#)
but for some strange reason I can't get the total to print correctly for keys 1,2, & 3. I tried using boolean
flags but I guess I'm doing something wrong with the "true/false" stuff because they don't seem to work.
I use LEDs to test code by using digitalWrite crap to have the LED display the realtime flag status.
It seems to turn the led on regardless of the flag status so I don't know what's going on. I need to devise
a better test.I want add the flags so I can use boolean AND statements such that if D == 0 && C==0 &&
B==0 && A==0, then the total = 1, because the bit pattern when you press the 1 key is L,L,L,L. Strangely
this worked with all the rest but I'm not getting why it doesn't work with 1,2,& 3. If I can use a false flag
as a prerequisite to enter the conditional , once it finds a match for all four it will change the total to
the correct value , set the flag to true and then it can't enter any of the following conditionals. That
way it tests each one and only sets the flag once it finds a match and changes the total and bypasses
all the rest because the flag is set.
Anyway, thanks for your help and I'll keep plugging at it. I tried googling Interupt routines and couldn't
find anything helpful. It seems that there are alot of people who want to show off their code but
only know how to communicate with other people who already know the information, and they
call it instruction. I'll instruct you but you can only understand it if you already know it....or so it seems.
What's the point ? I try to understand it but it's still sort of Greek to me.

matrix_3x4_keypad_Robs_Code.ino (831 Bytes)

Matrix_3x4_keypad_Robs_code.txt (881 Bytes)

sketch_apr03a.ino (2 Bytes)

keypad_4x3_FUNCTIONAL_1_5.ino (2.87 KB)

so I added "void" in front of it to get rid of the error.

That's correct, it should have been there, my bad.

What happened to my "volatile" keywords on newKey and total. They are required.

What does digitalWrite(OE, LOW); do? Is that pin on the OE pin of the 922. Why not just hard wire pin 14 on the 922 low?

I had a look at keypad_4x3_FUNCTIONAL_1_5.ino but honestly it's got problems and it's not worth persevering with until we get reliable input. Then we can probably do all that in 2 lines of code.

One thing at a time.


Rob

Thanks for the reply.
I put your volatile keywords back in and there was still no output.
I can see the OE led lit (that was there for diagnostic purposes because I turn it off at the end of your ISR).
The DATA_AV is going HIGH on keypress (the INT is configured for rising edge).
I have a wire from INT pin-2 to DATA_AV pin-12 of 74c922.
Is it possible that there is something else I need to have to enable the INT ? (ie: 'SEI' or something ?)
The modified prog is attached.

keypad_4x3_Rob_test.ino (1.04 KB)

Rob_test.txt (1.1 KB)

SUCCESS !
I made some major changes and got all the keys reporting correctly.
I added your volatile variables but had to change the 'bool' to 'boolean' because it was not changing color
to red like a keyword (that's how I know I mispelled something).
I also corrected the wrong program name in the attachinterupt declaration (it was 'KEYPRESS' , changed to 'readEncoder').
More important changes:
Remember I said "I need to devise a better test ?'
What I did was first I added the setflag code to the main loop, one for each key , like the following for the '0' key:

   if (total==13 &&  setflag==false)  // 0
            {
              Serial.print("total= "); 
              Serial.print(total);  
             total=0;
              setflag=true;
              digitalWrite(Ledpin,LOW);
              delay(300);
              Serial.println("setflag SET! (key=0) !");
             
            }

What that does for me is by ANDing the sum of the BCD values, after they were weighted previously by similar a
similar routine that tests a digitalRead for ==HIGH and then depending on which bit it changes the value to
8,4,2, or 1 , effectively 'weighting' that bit .
The code I added ANDs the sum of the weighted values (which uniquely identifies which key based on the
TRUTH TABLE in my first post) and 'setflag==false', so if that result has not previously been tested it will
correct the BCD code and set the 'setflag to 'true', print the received weighted value first and then
the key pressed (0-9,+* & #) before changing the value.(explanation following)
. The 'total' is what it is (what comes from encoder)
and I didn't feel comfortable printing a total that has been changed so I moved the print 'total' code to
before the change so the printed total is correct (matches the code from the encoder) . Then I change
the value and print the key pressed so the printed information shows the weighted values , their total
and the key they correspond to. This way when you see the total = 13 but the key = '0' you know the
which key it 'actually' is and which key you want it to be. (is that confusing ?)
I also added print statements to the ISR saying 'ISR Complete !, 'setflag SET ! , but they never came out
in the serial monitor. I added code to turn on the setflag status led (ledpin) in the setflag routines and
the ISR with a delay so it is readible (because without it the code is completed and the led turned off
before you can even see it). The setflag status led turns on once and only once now (before it used to
toggle on & off multiple times indicating the setflag code wasn't working). It also does not turn on
for the three keys that return correct codes (4,5 & 6).
Also I discovered that my terminal capture program (Clear Terminal) was saving empty files so I had
to download and install a different one that captures ok but doesn't display it on the screen but at
least it saves it ok. I don't know why the ISR is not executing but if it did there would be serial output.
Is it possible that there is something else I need to add to enable interupts ?
Is there a way to read the internal registers and print it out ?
CONCLUSION:
The encoder circuit works now and reports the keypressed but the ISR doesn't work.
The title of this post is "Writing an ISR ..." so I guess my work is not finished.
Any ideas ?

keypad_prog_working_correctly_ISR_and_setflag_TEST_4_Terminal_Capture.txt (939 Bytes)

keypad_4x3_Program_Working_CORRECTLY_ISR_and_setflag_test_5.ino (5.74 KB)

keypad_Working_Correctly_ISR_and_keypad_TEST_5_source.txt (5.96 KB)

  1. a screenshot of the IDE complile error. (I still can't figure out how anyone can copy the compiler output when
    the right-click /copy menu doesn't open when you right click.

Assuming you are using Windows, by click-dragging to select the part you want to copy, clicking in the error window and pressing Ctrl-a. Then you click Ctrl-c

Sheesh! Kids.

THANK GOD !
FYI,
YES , WINDOWS 7.
cntrl-a didn't work. Also, once you highlight it , if you click on the highlighted text it removes the highlighting.
You have to just place the cursor arrow over the highlighted text and CNTRL-C, then open notepad and either
cntrl-V or right click /paste.
SHEESH, OLD PEOPLE ....
Thanks , by the way. You made my like much easier.

compiler output.txt (84.7 KB)

raschemmel:
THANK GOD !
FYI,
YES , WINDOWS 7.
cntrl-a didn't work. Also, once you highlight it , if you click on the highlighted text it removes the highlighting.
You have to just place the cursor arrow over the highlighted text and CNTRL-C, then open notepad and either
cntrl-V or right click /paste.

Actually, Ctrl-a does work. I suppose I could have written it more clearly...

EITHER
Click-drag to select the text you want
OR
Click in the window and press Ctrl-a
THEN
Press Ctrl-c

If you click in the window between the Ctrl-a and the Ctrl-c, it will definitely deselect your carefully chosen text.

SHEESH, OLD PEOPLE ....
Thanks , by the way. You made my like much easier.

Heh heh.. you're welcome.

Actually, Ctrl-a does work.

YOUR RIGHT. IT DOES WORK !
I thought about it then decided to go back and test it again.
I don't know what I did wrong the first time but it definately works.
What's really nice about it is that CONTROL-A highlights EVERYTHING so you don't have to drag the mouse.

SHEESH, OLD PEOPLE

I can say that because I'm probably older than you are...

Final conclusion: See following link for project details and files.
http://forum.arduino.cc/index.php?topic=203615.msg1499690#msg1499690

So did you ever prove one way or another that you entered the ISR, maybe you just have the wire on the wrong pin.

You can't Serial.print but you can toggle a pin or set a LED on a pin.

And we really have to look at that code, it's terrible :slight_smile:


Rob

raschemmel:

SHEESH, OLD PEOPLE

I can say that because I'm probably older than you are...

Somehow I doubt that. The first program I wrote was on an IBM 6400 Electronic Accounting Machine. That is, if you can count wiring a plugboard "writing". :slight_smile:

I'll be 70 in February.

Do you still have any punch cards ?

Rob,
I think I can say conclusively that the ISR never ran by vertue of the fact that the serial print statement
in the ISR was never output. As far as the wrong pin , the attachinterupt statement has a consta pin2ISR
declared for pin 2 which is where the wire was. As far as the code being terrible, that's probably true
since this is my first attempt at an ISR or any interupt code whatsoever, having only been doing this
since Oct 30, 2013, when I bought my first Arduino. I have no idea what you mean , any more than
someone who has only been in electronics would know their soldering is terrible. I can't see what you
see when you look at my code. All I know is that while I got the interface working, I never got an
ISR working. I know it's not rocket science but unless I can query the CPU registers I don't know how
I can determine why it didn't execute, unless you can tell me.

The ISR looks good to me (at least down to newKey = true;), there must be something we're not seeing and someone will come along and say "What's with the comma after X?" Did you check Ledpin to see if it changes state?

The main body of the code...when you see a 100 if-this-do-that-else-if-something-do-something-else... it's a pretty good sign that there's a better way.

Also the fact that you have almost identical code in the conditions, for example

 if (total==13 &&  setflag==false)  // 0
            {
              Serial.print("total= "); 
              Serial.print(total); 
              total=0;
              setflag=true;
              digitalWrite(Ledpin,LOW);
              delay(300);
              Serial.print(" "); 
              Serial.println("setflag SET! (key=0) !");
             
            }

Is repeated many times with the only real difference being the value that total is set to. And the "&& setflag==false" is repeated for every test, put all the tests inside a single block that tests for that. (Mind you setflag does nothing anyway as it's set to false just before the tests)

I haven't analysed the code much but you have 9 if blocks based on the value in total, I would say you could use either a lookup table or a case statement for example

byte  chars [] = "0123456789*#";

void loop () {

  total =  digitalRead(OutD) << 3;
  total !=  digitalRead(OutC) << 2;
  total !=  digitalRead(OutB) << 1;
  total !=  digitalRead(OutA);

  Serial.print("total= "); 
  Serial.print(total); 

  digitalWrite(Ledpin,LOW);
  delay(300);

  switch(total) {
  case 13:
    total=0;
    break;

  case 0:
  case 1:
  case 2:
    total++;
    break;

  case 8:
  case 9:
  case 10:
    total--;
    break;		

  case 12:
    total = 10;
    break;

  case 14:
    total = 11;
    break;
  }

  Serial.print(" setflag SET! (key="); 
  Serial.print(chars[total]); 
  Serial.println(")!"); 
}

This has not been compiled, probably has a few typos, and almost certainly doesn't do exactly what you have in mind; but you can see the difference, it's immediately clear what values do what.


Rob

Thanks Rob,
I'll work on it .

raschemmel:
Do you still have any punch cards ?

No, but I still have a few old circuit boards from machines we scrapped.