Go Down

Topic: Unable to use mcp23008 for decoding keypress in interrupt (Read 271 times) previous topic - next topic

ilfuria

Hello,
I'm setting up a series of libraries for my own project, interfacing a matrix keyboard to an arduino nano (for the moment) using an mcp23008, and reading the pressed key as suggested in the AN1081 application note.

the problem is that I cannot use the component in an interrupt. Here is a sample code (I can expand it if it is necessary). Please do mind that the code works if outside of the interrupt

Code: [Select]

#include <Wire.h>
#include <mcp23008.h>
#include <i2ckeyboard.h>


const static char *keys[] = {"1", "2", "3", "4", "5", "6", "7", "8", "9", "*", "0", "#"};

// i2c address
i2ckeyboard keyboard(0x27);

void setup() {

// Just sets up the registrs
keyboard.setup();

//Serial if later needed
Serial.begin(9600);

// Sets up the INT0 interrupt pin port as input
  DDRD &= ~(1 << DDD2);
  PORTD &= ~(1 << PORTD2);

// Sets up port B1 as inactive output to flash LED when interrupt
  DDRB |= (1 << DDB1);
  PORTB &= ~(1 << PORTB1);

// Interrupt setup
  cli();

// Activate interrupt on falling edge of INT0
  EICRA|=(1<<ISC01);
  EICRA&=~(1<<ISC00);

// Enabling INT0 interrupt mask
  EIMSK |= (1 << INT0);

// Enabling interrupts
  sei();

}

// Interrupt
ISR(INT0_vect) {
 // Clears interrupt - TBD see if really needed
   cli();

   // Flashing led
   PORTB |= (1 << PORTB1);
   // Reads the key
   uint8_t c = keyboard.readKey();
   
  // Shuts down LED
  PORTB &= ~(1 << PORTB1);
 
  sei();

  //;

}

void loop() {
  // put your main code here, to run repeatedly:
  //Serial.println(100);
  delay(500);
}



Currently the code stops with the led on. The content of the setup function basically… sets up the registers of the component just as said in the aforementioned application note.

The readKey function has been now reduced to

Code: [Select]

uint8_t i2ckeyboard::readKey(void){
     uint8_t startCapture=readByte(0x08);
     return startCapture;
}


(yes I do have defined constants for registers)

And "readByte" is defined in an internal library as

Code: [Select]

        uint8_t data;

Wire.beginTransmission(_address);
Wire.write(adr);
Wire.endTransmission();

Wire.requestFrom((uint8_t)_address,(uint8_t)1);
while (Wire.available()) data=Wire.read();

return data;


and I have used it successfully in interrupts before (eg. on a attiny25v while interfacing to an mcp79410 RTC).

So my question is:
what am I doing wrong?
As remarked before the code works outside the interrupt, but I really would like to be able to read the values during the interrupt execution. Do you have enough info from the code I posted or do you need more?

Thanks

PaulS

Quote
what am I doing wrong?
Possibly more than one thing.

Code: [Select]
Wire.requestFrom((uint8_t)_address,(uint8_t)1);
while (Wire.available()) data=Wire.read();


Why are you using a while loop to read one byte? Using a while loop to spin until there is one byte to read would make sense.

The Wire library uses interrupts, which are disabled in an ISR. You should not be calling Wire functions in an ISR.

Code: [Select]
// Clears interrupt - TBD see if really needed
   cli();

It is not necessary to again disable interrupts in an ISR. They are already disabled.

Code: [Select]
  sei();
Re-enabling interrupts in an ISR is only for the very advanced, who know exactly what they are doing. Doing it as the last thing in the ISR, after which interrupts are enabled is pointless.

The art of getting good answers lies in asking good questions.

ilfuria

Possibly more than one thing.

Code: [Select]
Wire.requestFrom((uint8_t)_address,(uint8_t)1);
while (Wire.available()) data=Wire.read();


Why are you using a while loop to read one byte? Using a while loop to spin until there is one byte to read would make sense.
Hi there.

First of all, thanks for your reply! That code is taken from a library I found on the internet ~5 years ago, so I will gladly improve it with your suggestion!

Quote
The Wire library uses interrupts, which are disabled in an ISR. You should not be calling Wire functions in an ISR.
Well this sounds new and alarming: I have an RTC project where I just use the same code in an interrupt (not INT0 but a pin change interrupt - does it make a difference?) and has been working fine for the last 3 years, but if the freeze can happen, well can you please suggest a way in which I can improve the code? After all I need to communicate to the device while the interrupt function is executing…

Quote
Code: [Select]
// Clears interrupt - TBD see if really needed
   cli();

It is not necessary to again disable interrupts in an ISR. They are already disabled.
Nice, thanks!

Quote
Code: [Select]
  sei();
Re-enabling interrupts in an ISR is only for the very advanced, who know exactly what they are doing. Doing it as the last thing in the ISR, after which interrupts are enabled is pointless.


Gotcha, thanks again.

pylon

Quote
Well this sounds new and alarming: I have an RTC project where I just use the same code in an interrupt (not INT0 but a pin change interrupt - does it make a difference?) and has been working fine for the last 3 years, but if the freeze can happen, well can you please suggest a way in which I can improve the code? After all I need to communicate to the device while the interrupt function is executing…
Change your code to set a flag in the ISR and check for that flag in the standard loop to execute the code part that communicates to the device.

ilfuria

Change your code to set a flag in the ISR and check for that flag in the standard loop to execute the code part that communicates to the device.
Perhaps a stupid question: will that be quick enough to handle the keypress, ie to execute while the key is still pressed?

Thanks

pylon

Quote
Perhaps a stupid question: will that be quick enough to handle the keypress, ie to execute while the key is still pressed?
A key press is very slow in Arduino terms. So if you remove the delay() call from your loop (which is quite stupid anyway) it will be more than fast enough. You wouldn't even need the interrupt to check for a key press. Polling in the loop is more than fast enough.

ilfuria

Hi.
So I did everything that was suggested, ie, I only set a volatile bool variable in the interrupt (of course the variable is defined outside of the interrupt and initialized to false) and I check for it in the loop.

While this is ok, meaning that nothing freezes anymore, I still have a problem: reading the INTCAP register I correctly am able to find the row of the pressed key. Trying the columns though, which are done, as suggested in the manual:
  • Reading the IODIR register
  • Flipping its bits
  • Setting the flipped bits into INTCON, DEFVAL, GPINTEN, IODIR (in that order)


well, the GPIO pins read always yields 0xFF, so I'm not able to read the columns.

So I thought this could be a wiring issue, but I followed the manual exactly: row pins with 2.2k pullup,  column pins are just connected to the key matrix (since they're handled by the internal pullups.

So now I'm still stuck. Anyone kind enough to help me would probably need some information, and I'll provide anything asked.

Thanks.

pylon


ilfuria

Post your current code!
Hi, so here's my code:
Code: [Select]


#include <Wire.h>
#include <mcp23008.h>
#include <i2ckeyboard.h>


const static char *keys[] = {"1", "2", "3", "4", "5", "6", "7", "8", "9", "*", "0", "#"};
volatile bool t;

// i2c address
i2ckeyboard keyboard(0x27);

void setup() {

// Just sets up the registrs
keyboard.setup();

//Serial if later needed
Serial.begin(9600);

// Sets up the INT0 interrupt pin port as input
  DDRD &= ~(1 << DDD2);
  PORTD &= ~(1 << PORTD2);

// Sets up port B1 as inactive output to flash LED when interrupt
  DDRB |= (1 << DDB1);
  PORTB &= ~(1 << PORTB1);

// Interrupt setup
  cli();

// Activate interrupt on falling edge of INT0
  EICRA|=(1<<ISC01);
  EICRA&=~(1<<ISC00);

// Enabling INT0 interrupt mask
  EIMSK |= (1 << INT0);

// Enabling interrupts
  sei();

}

// Interrupt
ISR(INT0_vect) {
   t=true;
}

void loop() {
   if(t){
     t=false;
     PORTB |= (1 << PORTB1);
    Serial.println("OK");
      uint8_t c = keyboard.readKey();

    delay(500);
    PORTB &= ~(1 << PORTB1);

  Serial.println(c,HEX);
   }

}



The readKey function is

Code: [Select]

uint8_t startCapture;
uint8_t iodir;
uint8_t gpio;
uint8_t total;
uint8_t idx=(uint8_t)0;


startCapture=getCaptures();
Serial.println("captures");
Serial.println(startCapture,HEX);
iodir=getIOInputs();

Serial.println("startiodir");
Serial.println(iodir,HEX);
iodir=~iodir;
 
Serial.println("Switched");
Serial.println(iodir,HEX);

enableInterruptDefaultRegister(iodir);

setInterruptDefaultTriggers(iodir);

setInterruptPins(iodir);
setIOPins(iodir);

gpio=readPinValues();

        Serial.println("GPIO");
        Serial.println(gpio,HEX);

total=~(gpio|startCapture);

        iodir=~iodir;

        Serial.println("New");
        Serial.println(iodir,HEX);

enableInterruptDefaultRegister(iodir);
setInterruptDefaultTriggers(iodir);
setInterruptPins(iodir);
setIOPins(iodir);
// until it works, let's return something, anything will do
return iodir;


The methods for reading are basically doing a "readByte" with the appropriate register set. Eg the read of the gpio pins  (=readPinValues) is defined as

Code: [Select]

readByte(GPIO)


The registers shortcuts are defined as:
Code: [Select]

#define IODIR 0x0
#define IPOL 0x01
#define GPINTEN 0x02
#define DEFVAL 0x03
#define INTCON 0x04
#define IOCON 0x05
#define GPPU 0x06
#define INTF 0x07
#define INTCAP 0x08
#define GPIO 0x09
#define OLAT 0x0A


in the header file.


The logic analyzer shows that the i2c communication is going smoothly. The value read from register 0x09 is indeed 0xFF, regardless of the pressed key.

If you need anything else please ask.

Thank you.

pylon

Code: [Select]
#include <mcp23008.h>
#include <i2ckeyboard.h>


I'm missing these two files. Don't post excerpts, post the code as it is so that everyone on the forum is able to compile it. If one of them is an external library post a link to it.

Go Up