Pages: [1]   Go Down
Author Topic: Attaching interrupt kills main loop  (Read 2027 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 40
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

So I'm working on a device that takes in mouse and keyboard commands. With this I'm using the ps2.h, and PS2Mouse.h libraries. From the ps2.h library I erased all of the mouse information since the other gives me the functionality I want. The keyboard library also allows me to test for up and down keystrokes.

The problem is with the interrupt for the keyboard. If it is setup, then the main loop will not run at all, even if the actual function for the interrupt isn't running either. While interrupt is for the keyboard, the mouse should be polled in the main loop. So basically I am only getting the keyboard, and not the mouse. Any ideas?

Here is the code. the 'hmmms' are debug prints to see if the loop and function are running at the right times.

Thanks for helping!

Code:
#include <PS2Mouse.h>
#include <ps2.h>


#define MOUSE_DATA 12
#define MOUSE_CLOCK 13

PS2Mouse mouse(MOUSE_CLOCK, MOUSE_DATA, STREAM);

int ledPin = 9;    // LED connected to digital pin 9
int ledPin2 = 10;
int ledPin3 = 3;
int ledPin4 = 11;
int ltrigger = 5;
PS2 kbd(2, 7);

void setup()  {
    Serial.begin(9600);
 
  analogWrite(ledPin, 80);
  analogWrite(ledPin2, 80);
  analogWrite(ledPin3, 80);
  analogWrite(ledPin4, 80);
  mouse.initialize();
   
  char ack;
  kbd.write(0xff);  // send reset code
  ack = kbd.read();  // byte, kbd does self test
  ack = kbd.read();  // another ack when self test is done
 
  attachInterrupt(0, kbInterrupt, LOW);
}

void loop()  {

  Serial.println("hmmmmm");

  int data[2];
  mouse.report(data);

  if (data[0] == 9)
  {
    analogWrite(ltrigger, 60);
  }
  else
  {
    analogWrite(ltrigger, 250);
  }

  if (data[1] > 0 )
  {
    analogWrite(ledPin4, 160);

  }
  else if (data[1] < 0 )
  {
    analogWrite(ledPin4, 0);

  } 
  else
  {
    analogWrite(ledPin4, 80);
  }


  if (data[2] > 0 )
  {
    analogWrite(ledPin3, 160);

  }
  else if (data[2] < 0 )
  {
    analogWrite(ledPin3, 0);

  } 
  else
  {
    analogWrite(ledPin3, 80);
  }

  Serial.print(data[0]); // Status Byte
  Serial.print(":");
  Serial.print(data[1]); // X Movement Data
  Serial.print(",");
  Serial.print(data[2]); // Y Movement Data
  Serial.println();
  interrupts();
}

void kbInterrupt() {

  unsigned char code;
  /* read a keycode */
  code = kbd.read();
  /* send the data back up */

  delay(10);  /* twiddle */

  int coded = code;

  if (coded == 29){
    Serial.println("W-d");
    analogWrite(ledPin2, 160);
  }

  if (coded == 240){
    unsigned char code1;
    code1 = kbd.read();

    int coded1 = code1;
    if (coded1 == 29){
      analogWrite(ledPin2, 90);
      Serial.println("W-u");
    }
  }
  else{
    Serial.println("hmmm");
  }
}



Logged

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

Code:
  attachInterrupt(0, kbInterrupt, LOW);

You almost certainly don't want a LOW interrupt because that fires continuously. I would suggest FALLING.


Code:
  delay(10);  /* twiddle */

The delay() function does not work properly in an interrupt service routine.
Logged


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

http://gammon.com.au/interrupts
Logged


Offline Offline
Newbie
*
Karma: 0
Posts: 40
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Thanks a lot for the reply. Meant to get back to this last night, but the internet went down for some reason.

Nick, I tried both LOW and FALLING, but  I get the same results both ways, and the delay was an accident (Good catch! heh heh, but no change)

It is looking like even though I did put it inside of an interrupt that detects a keypress, the 'kbd.read();' function still takes over the program because it is waiting for something. It does get the right keys when pressed rather responsively, but it gets stuck regardless. I would use a different keyboard library that had an 'available' function, but the only one I found does not detect up and down strokes, only the key that was pressed. any ideas?

Here is the ps2.cpp with the read function in it if you can help.
Code:
/*
 * ps2.cpp - an interface library for ps2 devices.  Common devices are
 * mice, keyboard, barcode scanners etc.  See the examples for mouse and
 * keyboard interfacing.
 * limitations:
 *      we do not handle parity errors.
 *      The timing constants are hard coded from the spec. Data rate is
 *         not impressive.
 *      probably lots of room for optimization.
 *
 * PS2Mouse class by Jonathan Laloz - http://thepotterproject.com
 * Released into the public domain
 *
 */

#include "ps2.h"

/*
 * the clock and data pins can be wired directly to the clk and data pins
 * of the PS2 connector.  No external parts are needed.
 */
PS2::PS2(int clk, int data)
{
_ps2clk = clk;
_ps2data = data;
gohi(_ps2clk);
gohi(_ps2data);
}

/*
 * according to some code I saw, these functions will
 * correctly set the clock and data pins for
 * various conditions.  It's done this way so you don't need
 * pullup resistors.
 */
void
PS2::gohi(int pin)
{
pinMode(pin, INPUT);
digitalWrite(pin, HIGH);
}

void
PS2::golo(int pin)
{
pinMode(pin, OUTPUT);
digitalWrite(pin, LOW);
}

/* write a byte to the PS2 device */
void
PS2::write(unsigned char data)
{
unsigned char i;
unsigned char parity = 1;

gohi(_ps2data);
gohi(_ps2clk);
delayMicroseconds(300);
golo(_ps2clk);
delayMicroseconds(300);
golo(_ps2data);
delayMicroseconds(10);
gohi(_ps2clk); // start bit
/* wait for device to take control of clock */
while (digitalRead(_ps2clk) == HIGH)
; // this loop intentionally left blank
// clear to send data
for (i=0; i < 8; i++)
{
if (data & 0x01)
{
gohi(_ps2data);
} else {
golo(_ps2data);
}
// wait for clock
while (digitalRead(_ps2clk) == LOW)
;
while (digitalRead(_ps2clk) == HIGH)
;
parity = parity ^ (data & 0x01);
data = data >> 1;
}
// parity bit
if (parity)
{
gohi(_ps2data);
} else {
golo(_ps2data);
}
// clock cycle - like ack.
while (digitalRead(_ps2clk) == LOW)
;
while (digitalRead(_ps2clk) == HIGH)
;
// stop bit
gohi(_ps2data);
delayMicroseconds(50);
while (digitalRead(_ps2clk) == HIGH)
;
// mode switch
while ((digitalRead(_ps2clk) == LOW) || (digitalRead(_ps2data) == LOW))
;
// hold up incoming data
golo(_ps2clk);
}


/*
 * read a byte of data from the ps2 device.  Ignores parity.
 */
unsigned char
PS2::read(void)
{
unsigned char data = 0x00;
unsigned char i;
unsigned char bit = 0x01;

// start clock
gohi(_ps2clk);
gohi(_ps2data);
delayMicroseconds(50);
while (digitalRead(_ps2clk) == HIGH)
;
delayMicroseconds(1); // not sure why.
while (digitalRead(_ps2clk) == LOW)
; // eat start bit
for (i=0; i < 8; i++)
{
while (digitalRead(_ps2clk) == HIGH)
;
if (digitalRead(_ps2data) == HIGH)
{
data = data | bit;
}
while (digitalRead(_ps2clk) == LOW)
;
bit = bit << 1;
}
// eat parity bit, ignore it.
while (digitalRead(_ps2clk) == HIGH)
;
while (digitalRead(_ps2clk) == LOW)
;
// eat stop bit
while (digitalRead(_ps2clk) == HIGH)
;
while (digitalRead(_ps2clk) == LOW)
;
golo(_ps2clk); // hold incoming data

return data;
}
Logged

Austin, TX
Offline Offline
Full Member
***
Karma: 2
Posts: 182
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi,
   You should probably not call kbd.read inside an interrupt handler.  It can take hundreds of microseconds to return.  I'm not sure, but delayMicroseconds() might also have problems inside of an ISR.

In the read routine, code of the form:
Code:
while (digitalRead(_ps2clk) == HIGH)
;
can get "stuck" if the transition is missed.  This normally doesn't happen, but I don't know what effect the ISR has on it.
Doing serial operations inside an ISR is also discouraged.

Another thing to look at is if the mouse and keyboard routines are conflicting with each other. For example, if the mouse routine has the same timing constraints as the keyboard, then if mouse calls delay and then gets an interrupt, they are going to trip each other up.  In fact, I'd bet that's what is happening.  The call to mouse.data() is the one that's hanging up, because it gets derailed by the interrupt handler, and misses some of the incoming bits.  The PS2 routines are not very robust, in that they don't really detect that sort of problem.

If I were you, I'd shorten the ISR so that all it does is set a flag.  Then in the main loop, check the flag value and if it is set, do the keyboard stuff (resetting the flag when done).  Always keep interrupt handlers as short as possible.  That way the mouse and keyboard routines are never running at the same time, and there are no timing issues.  An ISR with only a couple of instructions in it would probably not throw off the timing enough to matter, if it was not called frequently.
Logged

Chris J. Kiick
Robot builder and all around geek.

Offline Offline
Newbie
*
Karma: 0
Posts: 40
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Thanks Cklick, I tried your way, but now the mouse only reports when a keystroke occurs. Starting to think its in the library.

here is the updated code.
Code:

#include <PS2Mouse.h>
#include <ps2.h>


#define MOUSE_DATA 12
#define MOUSE_CLOCK 13

PS2Mouse mouse(MOUSE_CLOCK, MOUSE_DATA, STREAM);

int ledPin = 9;   
int ledPin2 = 10;
int ledPin3 = 3;
int ledPin4 = 11;
int ltrigger = 5;
volatile boolean kbe = false;
PS2 kbd(2, 7);

void setup()  {
  Serial.begin(9600);

  analogWrite(ledPin, 80);
  analogWrite(ledPin2, 80);
  analogWrite(ledPin3, 80);
  analogWrite(ledPin4, 80);
  mouse.initialize();

  char ack;
  kbd.write(0xff);  // send reset code
  ack = kbd.read();  // byte, kbd does self test
  ack = kbd.read();  // another ack when self test is done

  attachInterrupt(0, kbInterrupt, FALLING);
}

void loop()  {

  int data[2];
  mouse.report(data);

  if (data[0] == 9)
  {
    analogWrite(ltrigger, 60);
  }
  else
  {
    analogWrite(ltrigger, 250);
  }

  if (data[1] > 0 )
  {
    analogWrite(ledPin4, 160);

  }
  else if (data[1] < 0 )
  {
    analogWrite(ledPin4, 0);

  } 
  else
  {
    analogWrite(ledPin4, 80);
  }


  if (data[2] > 0 )
  {
    analogWrite(ledPin3, 160);

  }
  else if (data[2] < 0 )
  {
    analogWrite(ledPin3, 0);

  } 
  else
  {
    analogWrite(ledPin3, 80);
  }

  Serial.print(data[0]); // Status Byte
  Serial.print(":");
  Serial.print(data[1]); // X Movement Data
  Serial.print(",");
  Serial.print(data[2]); // Y Movement Data
  Serial.println();
  interrupts();

  noInterrupts();
  if(kbe == true){

    unsigned char code;
    code = kbd.read();
    int coded = code;

    if (coded == 29){
      Serial.println("W-d");
      analogWrite(ledPin2, 160);
    }

    if (coded == 240){
      unsigned char code1;
      code1 = kbd.read();
      int coded1 = code1;

      if (coded1 == 29){
        analogWrite(ledPin2, 90);
        Serial.println("W-u");
      }
    }
    else{
      Serial.println(coded, DEC);
      Serial.println("BLAMMO!");
    }
    kbe = false;
  }
  interrupts();
}

void kbInterrupt() {
  kbe = true;
}



Here is the serial output, it doesn't coninue past this point unless I press a key
Code:
8:0,0
126
BLAMMO!
8:0,0
191
BLAMMO!
8:0,0
126
BLAMMO!
8:0,0
Logged

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

I wouldn't be turning interrupts on and off myself.

Code:
int data[2];
  mouse.report(data);

...

  if (data[2] > 0 )

Index 2 is not valid. You have two elements, 0 and 1.
Logged


Offline Offline
Newbie
*
Karma: 0
Posts: 40
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


Quote
Index 2 is not valid. You have two elements, 0 and 1.

index 2 still comes up with the right numbers, but for good measure I tried a couple of things out.

First
Code:
int data[3];
  mouse.report(data);

...

  if (data[2] > 0 )
Works just as before after setting this up, as I expected.

So, just to check something
Code:
int data[2];
  mouse.report(data);

...

Serial.print(data[3]);
This did not throw an error at compile. It wouldn't show the right number, but it didn't crash. So I'm guessing this is just a difference between explicit and implicit declarations? I've left it with the int data[3]; though just to make sure. Regardless this hasn't solved the issue, and I had ditched turning the interrupts off and on. Thanks for the help.
Logged

nr Bundaberg, Australia
Online Online
Tesla Member
***
Karma: 129
Posts: 8528
Scattered showers my arse -- Noah, 2348BC.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
This did not throw an error at compile.
It won't, C is happy for you to trash all the RAM if you want. But it ain't right.

If your other examples work I can only assume that mouse.report() actually loads more than two ints into "data" and you have been lucky in that there was nothing important at offset 2 and above. Can you point to the documentation for mouse.report()?


______
Rob
Logged

Rob Gray aka the GRAYnomad www.robgray.com

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

Code:
  noInterrupts();
 ...
      Serial.println("W-d");

It's not advisable to do Serial prints with interrupts off.
Logged


Offline Offline
Newbie
*
Karma: 0
Posts: 40
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Well, I thought I had it going. I'm going to have to mess with the keyboard library in order to get this going though. Thanks for the help guys, at least I got the interrupt stuff down.

Quote
Can you point to the documentation for mouse.report()?
Sorry I don't remember where I got the library from. C is a funny language.
Logged

nr Bundaberg, Australia
Online Online
Tesla Member
***
Karma: 129
Posts: 8528
Scattered showers my arse -- Noah, 2348BC.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
C is a funny language.
To some maybe smiley

______
Rob
Logged

Rob Gray aka the GRAYnomad www.robgray.com

Offline Offline
Edison Member
*
Karma: 26
Posts: 1339
You do some programming to solve a problem, and some to solve it in a particular language. (CC2)
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I've left it with the int data[3]; though just to make sure

 smiley-eek-blue

Leaving out-of-bound array indexes around is a recipe for disaster. Please _dont't_ do it.

It's also a good programming practice to use named constants for array sizes.
Logged

Austin, TX
Offline Offline
Full Member
***
Karma: 2
Posts: 182
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The reason you have to press a key to make progress in the main loop is that kbd.read is a blocking call: it will wait until there is data available from the keyboard.  The mouse works the same way, but there is always data available from the mouse.

In order for this to work the way you want, the keyboard read function must be modified to be non-blocking.  At the point where it is waiting for data from the keyboard, it can use a loop that breaks after a certain amount of time (or number of loops).  Then it should return an invalid value that the caller can detect.  There are several other ways to do it, but that one requires the smallest changes.

I don't know if you have the programming experience to do it yourself.  If you do get it working please post the code in the playground for others to use.

PS:
Do not turn interrupts off and on unless you have a clear case where not doing so causes a problem. Interrupts are on by default and turning them off causes problems with timing and other aspects of the arduino environment.
You also need to figure out the correct parameters to the mouse routines. Perhaps the header file has some comments that will help.
Logged

Chris J. Kiick
Robot builder and all around geek.

Offline Offline
Newbie
*
Karma: 0
Posts: 40
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote from: tuxduino
Leaving out-of-bound array indexes around is a recipe for disaster.

True, It's just a bit confusing once I changed it around. 2 is the libraries default though, so I'll just stick with that. I really have no excuse for my naming conventions ha ha, I'll work on it.

 
Quote from: ckiick
The reason you have to press a key to make progress in the main loop is that kbd.read is a blocking call: it will wait until there is data available from the keyboard.  The mouse works the same way, but there is always data available from the mouse.

In order for this to work the way you want, the keyboard read function must be modified to be non-blocking. 

That is where I'm at now. Seems the way to catch it should be through an interrupt, but the library isn't set up to handle it that way. I'm trying to get some more info on the actual patterns right now, I understand what the library does, and I think the trick is to have the clock on the interrupt, and listen for the clock to fall. Next pass the first bit of data, just wait for hi clock, then low clock. Once it's low take in data1, then wait for hi clock,and low clock again. Do this 7 more times for data2-8. The last thing to do is waste time on the last two clock cycles, report, reset and go back to listening for the interrupt again. Hopefully I'll figure it out, thanks guys!
Logged

Pages: [1]   Go Up
Jump to: