help with an IR heart rate monitoring code!!

I am working on this project from the internet. An ir emitter and a detector pair is used as a sensor…which is touched to your skin…it shines light up your skin and then reflected light is dected by the detector…this signal is then fed into an lm386 which amplifies the signal which is fed into an analog pin of an arduino. My Arduino has the following code running on it…
\

int heart_high, heart_low;
int data[200], i, mapped;
void setup(){
  for(i=2; i<12; i++)//LEDs
  pinMode(i, OUTPUT);
Serial.begin(115200);//for the processing sketch
  
}//setup


void loop(){

      heart_high=0;//reset the high before we look for it,  everything is higher than 0
      heart_low = 1023;//reset the low before we look for it, everything is lower than 1023
      
      for(i=200; i>0; i--){//search for the low and high out of the last 200 samples
        data[i] = data[i-1];// move everything back one, this will leave data[0] empty, but we put the next read there after this
        
      if(data[i]>heart_high)//check to see if we have something higher than the current high
      heart_high=data[i];//if it is higher, write the new value in
      if(data[i]<heart_low)//same idea as the high check
      heart_low=data[i];
    }//for loop
    
    data[0] = analogRead(0);//now write the next analog value to data[0]
    
    mapped = map(data[0], 0, 1023, 0, 600);
    Serial.println(mapped);//send over the current value to the processing sketch, but scale it to match the screen height

  delay(5);//delay in here is important, we need enough samples to catch an entire waveform, so at least 1 sec of samples should be tored in data[0], 
  //so 5ms x 200 = 1000, we're good
  
  
  //This code is where we control the LEDs
  if((heart_high-heart_low)>150){//first, don't even go in here unless the span of the high and low is greater than 150.  This is important, so the LEDs don't go crazy 
  //if you're flat lined
  
  //I'll explain how one of these works
  //There is an 'if' check for each of the LEDs
  //Basically, how this works is we want the LEDs turn represent the entire span of a heart beat, which is why we get the lowest and highest value
  //this gives us the span, which can be used to take a percentage of, and that's exactly how all these statements work
  if(data[0] > (heart_high-.95*(heart_high-heart_low)))//this is true if the read is greater than 95% of the span
  digitalWrite(2, HIGH);
  else
  digitalWrite(2,LOW); 
  
  if(data[0] > (heart_high-.9*(heart_high-heart_low)))// greater than 90% of the span and so on
  digitalWrite(3, HIGH);
  else
  digitalWrite(3,LOW); 
 
    if(data[0] > (heart_high-.8*(heart_high-heart_low)))
  digitalWrite(4, HIGH);
  else
  digitalWrite(4,LOW); 
    if(data[0] > (heart_high-.7*(heart_high-heart_low)))
  digitalWrite(5, HIGH);
  else
  digitalWrite(5,LOW); 
    if(data[0] > (heart_high-.6*(heart_high-heart_low)))
  digitalWrite(6, HIGH);
  else
  digitalWrite(6,LOW); 
    if(data[0] > (heart_high-.5*(heart_high-heart_low)))
  digitalWrite(7, HIGH);
  else
  digitalWrite(7,LOW); 
    if(data[0] > (heart_high-.4*(heart_high-heart_low)))
  digitalWrite(8, HIGH);
  else
  digitalWrite(8,LOW); 
    if(data[0] > (heart_high-.3*(heart_high-heart_low)))
  digitalWrite(9, HIGH);
  else
  digitalWrite(9,LOW); 
     if(data[0] > (heart_high-.2*(heart_high-heart_low)))
  digitalWrite(10, HIGH);
  else
  digitalWrite(10,LOW); 
     if(data[0] > (heart_high-.1*(heart_high-heart_low)))
  digitalWrite(11, HIGH);
  else
  digitalWrite(11,LOW);  
  
  }//span check
  else
  for(i=2; i<12; i++)//turn all the LEDs off if we're flatlined
  digitalWrite(i,LOW);
  


    

}//loop

This code sends data via serial which then used by a processing app to visualize the waveform!!
But I don’t want to use a computer. what i want to do is calculate the bmp( beats per minute) and display is on a 16x2 LCD. (I have some experience with arduino LCD library). I want someone to modify this code…so that it could calculate the BMP…and may be give me some explanation so that I could tackle such problems myself in future. I am learning. :slight_smile:

I want someone (that would be you, OP) to use Tools + Auto Format on that abominable mess.

ok…i did auto formatting but it still looks same!! I don’t want you to do it for me…but i have no choice…ok just tell me how to do it…and i will try!!

int heart_high, heart_low;
int data[200], i, mapped;
void setup(){
  for(i=2; i<12; i++)//LEDs
    pinMode(i, OUTPUT);
  Serial.begin(115200);//for the processing sketch

}//setup


void loop(){

  heart_high=0;//reset the high before we look for it,  everything is higher than 0
  heart_low = 1023;//reset the low before we look for it, everything is lower than 1023

    for(i=200; i>0; i--){//search for the low and high out of the last 200 samples
    data[i] = data[i-1];// move everything back one, this will leave data[0] empty, but we put the next read there after this

    if(data[i]>heart_high)//check to see if we have something higher than the current high
        heart_high=data[i];//if it is higher, write the new value in
    if(data[i]<heart_low)//same idea as the high check
      heart_low=data[i];
  }//for loop

  data[0] = analogRead(0);//now write the next analog value to data[0]

  mapped = map(data[0], 0, 1023, 0, 600);
  Serial.println(mapped);//send over the current value to the processing sketch, but scale it to match the screen height

  delay(5);//delay in here is important, we need enough samples to catch an entire waveform, so at least 1 sec of samples should be tored in data[0], 
  //so 5ms x 200 = 1000, we're good


  //This code is where we control the LEDs
  if((heart_high-heart_low)>150){//first, don't even go in here unless the span of the high and low is greater than 150.  This is important, so the LEDs don't go crazy 
    //if you're flat lined

    //I'll explain how one of these works
    //There is an 'if' check for each of the LEDs
    //Basically, how this works is we want the LEDs turn represent the entire span of a heart beat, which is why we get the lowest and highest value
    //this gives us the span, which can be used to take a percentage of, and that's exactly how all these statements work
    if(data[0] > (heart_high-.95*(heart_high-heart_low)))//this is true if the read is greater than 95% of the span
      digitalWrite(2, HIGH);
    else
      digitalWrite(2,LOW); 

    if(data[0] > (heart_high-.9*(heart_high-heart_low)))// greater than 90% of the span and so on
      digitalWrite(3, HIGH);
    else
      digitalWrite(3,LOW); 

    if(data[0] > (heart_high-.8*(heart_high-heart_low)))
      digitalWrite(4, HIGH);
    else
      digitalWrite(4,LOW); 
    if(data[0] > (heart_high-.7*(heart_high-heart_low)))
      digitalWrite(5, HIGH);
    else
      digitalWrite(5,LOW); 
    if(data[0] > (heart_high-.6*(heart_high-heart_low)))
      digitalWrite(6, HIGH);
    else
      digitalWrite(6,LOW); 
    if(data[0] > (heart_high-.5*(heart_high-heart_low)))
      digitalWrite(7, HIGH);
    else
      digitalWrite(7,LOW); 
    if(data[0] > (heart_high-.4*(heart_high-heart_low)))
      digitalWrite(8, HIGH);
    else
      digitalWrite(8,LOW); 
    if(data[0] > (heart_high-.3*(heart_high-heart_low)))
      digitalWrite(9, HIGH);
    else
      digitalWrite(9,LOW); 
    if(data[0] > (heart_high-.2*(heart_high-heart_low)))
      digitalWrite(10, HIGH);
    else
      digitalWrite(10,LOW); 
    if(data[0] > (heart_high-.1*(heart_high-heart_low)))
      digitalWrite(11, HIGH);
    else
      digitalWrite(11,LOW);  

  }//span check
  else
    for(i=2; i<12; i++)//turn all the LEDs off if we're flatlined
      digitalWrite(i,LOW);





}//loop

I personally always use braces around statements If and else and for statements etc even if there is only one statement in the if else or for …

I also make sure that I have clear and consistent indentation, it makes the code easier to read and helps you spot mistakes
e.g.

if(data[0] > (heart_high-.9*(heart_high-heart_low)))// greater than 90% of the span and so on
{
      digitalWrite(3, HIGH);
}
else
{
      digitalWrite(3,LOW);
}

leave 1 blank line between each if / else

e.g.

if(data[0] > (heart_high-.3*(heart_high-heart_low)))
{
      digitalWrite(9, HIGH);
}
else
{
      digitalWrite(9,LOW); 
}

if(data[0] > (heart_high-.3*(heart_high-heart_low)))
{
      digitalWrite(9, HIGH);
}
else
{
      digitalWrite(9,LOW); 
}

use braces after for loops as well

  else
{
    for(i=2; i<12; i++)//turn all the LEDs off if we're flatlined
    {
         digitalWrite(i,LOW);
    }
}
    for(i=200; i>0; i--){//search for the low and high out of the last 200 samples

How the hell is one to reasonably be expected to findthatcurlybracehidinginthere?

Put EVERY { on it's own line with space after it if you feel COMPELLED to add a useless comment.

There is just NO excuse for making i a global variable. I won't even read code that is structured that way.

I’m not personally a fan of putting comments onto the end of a line unless its after a semi colonn

i.e.
Bad

for(i=2; i<12; i++)//turn all the LEDs off if we're flatlined

Better

//turn all the LEDs off if we're flatlined
for(i=2; i<12; i++)

I hope you guys will read this one!! This is not my code…found it on the internet. Guys, I really need your help :frowning:

int heart_high, heart_low;
int data[200], i, mapped;
void setup(){
  for(i=2; i<12; i++)              //LEDs
    pinMode(i, OUTPUT);
  Serial.begin(115200);      //for the processing sketch

}          //setup


void loop(){

  heart_high=0;                     //reset the high before we look for it,  everything is higher than 0
  heart_low = 1023;               //reset the low before we look for it, everything is lower than 1023

  for(i=200; i>0; i--){              //search for the low and high out of the last 200 samples
             data[i] = data[i-1];    // move everything back one, this will leave data[0] empty, but we put the next read there after this

        if(data[i]>heart_high)      //check to see if we have something higher than the current high
             
             heart_high=data[i];    //if it is higher, write the new value in
        
        if(data[i]<heart_low)       //same idea as the high check
                    
             heart_low=data[i];
  }//for loop

  data[0] = analogRead(0);    //now write the next analog value to data[0]

  mapped = map(data[0], 0, 1023, 0, 600);
  
  Serial.println(mapped);     //send over the current value to the processing sketch, but scale it to match the screen height

  delay(5);                   //delay in here is important, we need enough samples to catch an entire waveform, so at least 1 sec of samples should be tored in data[0], 
                              //so 5ms x 200 = 1000, we're good


  //This code is where we control the LEDs
  
  if((heart_high-heart_low)>150){       //first, don't even go in here unless the span of the high and low is greater than 150.  This is important, so the LEDs don't go crazy 
    //if you're flat lined

    //I'll explain how one of these works
    //There is an 'if' check for each of the LEDs
    //Basically, how this works is we want the LEDs turn represent the entire span of a heart beat, which is why we get the lowest and highest value
    //this gives us the span, which can be used to take a percentage of, and that's exactly how all these statements work
    
    if(data[0] > (heart_high-.95*(heart_high-heart_low)))    //this is true if the read is greater than 95% of the span
      digitalWrite(2, HIGH);
    else
      digitalWrite(2,LOW); 

    if(data[0] > (heart_high-.9*(heart_high-heart_low)))     // greater than 90% of the span and so on
      digitalWrite(3, HIGH);
    else
      digitalWrite(3,LOW); 

    if(data[0] > (heart_high-.8*(heart_high-heart_low)))
      digitalWrite(4, HIGH);
    else
      digitalWrite(4,LOW); 
    if(data[0] > (heart_high-.7*(heart_high-heart_low)))
      digitalWrite(5, HIGH);
    else
      digitalWrite(5,LOW); 
    if(data[0] > (heart_high-.6*(heart_high-heart_low)))
      digitalWrite(6, HIGH);
    else
      digitalWrite(6,LOW); 
    if(data[0] > (heart_high-.5*(heart_high-heart_low)))
      digitalWrite(7, HIGH);
    else
      digitalWrite(7,LOW); 
    if(data[0] > (heart_high-.4*(heart_high-heart_low)))
      digitalWrite(8, HIGH);
    else
      digitalWrite(8,LOW); 
    if(data[0] > (heart_high-.3*(heart_high-heart_low)))
      digitalWrite(9, HIGH);
    else
      digitalWrite(9,LOW); 
    if(data[0] > (heart_high-.2*(heart_high-heart_low)))
      digitalWrite(10, HIGH);
    else
      digitalWrite(10,LOW); 
    if(data[0] > (heart_high-.1*(heart_high-heart_low)))
      digitalWrite(11, HIGH);
    else
      digitalWrite(11,LOW);  

  }//span check
  else
    for(i=2; i<12; i++)//turn all the LEDs off if we're flatlined
      digitalWrite(i,LOW);





}//loop

I hope you guys will read this one!!

Not me. That's the worst looking crap you've posted so far. You've ignored every bit of the advice you've been given.

Well...I have figured it out myself :D. I kept looking at the code for hours...reloading this page in a hope you guys will answer. You see that pin 11...that turns on to drive a led...when the beat is at its peak. As we are storing 1 sec of data in data[0]...So puting a counter "C" gives me how many times pin 11 turns on in a sec ...if we multiply C with 60 we get beats per minute. Which can be than lcd.print() to an LCD but now the problem is the void loop...!! To keep the output on the lcd so that the user can see it...i need to add a delay but if i add an delay in void loop...the data won't be real time anymore. So, it there any way to keep the data on the screen with out using a delay?? :S If i don't use a delay..the output will be so fast users won't see it!!

So, it there any way to keep the data on the screen with out using a delay?

Of course. Feel free to modify the code you didn't post to do so.

Do NOT post code without taking into account all the things you've been advised to do.

Mr_Omnos: Well...I have figured it out myself :D. I kept looking at the code for hours...reloading this page in a hope you guys will answer. You see that pin 11...that turns on to drive a led...when the beat is at its peak. As we are storing 1 sec of data in data[0]...So puting a counter "C" gives me how many times pin 11 turns on in a sec ...if we multiply C with 60 we get beats per minute. Which can be than lcd.print() to an LCD but now the problem is the void loop...!! To keep the output on the lcd so that the user can see it...i need to add a delay but if i add an delay in void loop...the data won't be real time anymore. So, it there any way to keep the data on the screen with out using a delay?? :S If i don't use a delay..the output will be so fast users won't see it!!

The code I see is hard to read as you've been told, but I have some other suggestions that might help.


1) This line (and many others) mixes integer and floating point, which slows execution about 100x.

    if(data[0] > (heart_high-.95*(heart_high-heart_low)))    //this is true if the read is greater than 95% of the span

Assuming that heart_high might reach 1023, this form does the math using 32-bit integers:

    if ( data[ 0 ] > ((long) heart_high - ( 95L * (long) heart_high-heart_low / 100L ))    // if greater than 95% of the span

That will run faster and give as or more accurate results. It works in longs because 95 x 1023 > 32768, the biggest 16 bit int you can have.


2) why do you need 10 leds and why turn them on and off?


3) I didn't get to the part where you use the actual waveform as opposed to timing highs and lows. Do you?


4) You really don't need delay(). You need to learn to use millis() to time when to run timed code.

Here's a link to a blog that will really help: http://www.gammon.com.au/forum/?id=11411

I go a bit farther in separating code for tasks, but that is a very good starting point and full explanation.

I think you guys need to learn a little bed side manners or the newbie's will never come back.

You might want to take a poll on that or come visit the forum Bar. Or, for a real shock, go join AVRFREAKS to see why this forum gets rated high. Of course that doesn't mean we cater to everyone, especially not time-wasters.

2) why do you need 10 leds and why turn them on and off?


3) I didn't get to the part where you use the actual waveform as opposed to timing highs and lows. Do you?

[/quote] Hi!! Thanks a lot :) those 10 leds swing with the waveform...something like in a audio mixture or amp!! But i am not using them...this is not my code...i just figured out that they can be used to count the beats.

Timing highs and lows...hmm...i know nothing about it!! :S How can i implement it in this project??

GoForSmoke:


  1. You really don’t need delay(). You need to learn to use millis() to time when to run timed code.

Here’s a link to a blog that will really help:
http://www.gammon.com.au/forum/?id=11411

I go a bit farther in separating code for tasks, but that is a very good starting point and full explanation.

[/quote]

this thing is changed my life!! THANKS A LOT!!

Now you're maybe ready to write event-driven code and state machines. Several different "programs" can run as concurrent tasks and coordinate through values in variables.

One thing about flags though. If you have several that work together then make them bits in one variable. You can operate on the lot using bitwise logic and reduce long if() logic by half or less.

This is the Arduino Reference, bitwise logic is under Structure, the functions are under Functions, array is under Variables. These are code equivalents to ABC's and spelling words. Knowing of these and learning them through use really helps when coming up with algorithms..