Program execution freezes when using Interrupt, Please help

I am preparing arduino based speedometer/odometer and my arduino freezes after some iterations of successful execution of this code. I have copy-pasted the Arduino RPM code and tried to display it on Nokia 5110 lcd using the <LCD5110_Graph.h> which is for graphical display on NokiaLCD. I’m using hall sensor and magnet which will interrupt pin 2 (interrupt 0). I have also used the RTC date/time library. Can somebody locate the problem and give a workaround solution to keep the program running consistently without freezing?

//      SCK  - Pin 3
// DIN/MOSI - Pin 4
//      DC   - Pin 5
//      RST  - Pin 6
//      CS   - Pin 7
//
#include "Wire.h"
#include "LCD5110_Graph.h"
#include "RTClib.h"

RTC_DS1307 RTC;
LCD5110 lcd(3,4,5,6,7);

extern uint8_t SmallFont[];
extern uint8_t TinyFont[];
extern uint8_t BigNumbers[];
extern uint8_t MediumNumbers[];

int half_revolutions = 0;

int rpm = 0;

unsigned long lastmillis = 0;

void setup()
{
   lcd.InitLCD();
   Wire.begin();
   RTC.begin();     // Instantiate the RTC
   attachInterrupt(0, rpm_fan, FALLING);
}

void loop()
{
   if (millis() - lastmillis == 1000)
   { //Uptade every one second, this will be equal to reading frecuency (Hz).

      detachInterrupt(0);//Disable interrupt when calculating

      // Get the current time
      DateTime now = RTC.now();   

      rpm = half_revolutions * 60; 

      //     Below code is for displaying on the Nokia5110 LCD:
      lcd.setFont(SmallFont);
      lcd.clrScr();
      lcd.print(String(now.hour(), DEC), 0, 0);
      lcd.print(":",12, 0);
      lcd.print(String(now.minute(), DEC), 18, 0); 
      lcd.print(":",30, 0);
      lcd.print(String(now.second(), DEC), 36, 0); 
      lcd.print(String(now.day(), DEC), 0, 8); 
      lcd.print("/",12, 8);
      lcd.print(String(now.month(), DEC),18, 8); 
      lcd.print("/",30, 8);
      lcd.print(String(now.year(), DEC), 36, 8);
      lcd.print("Total:", LEFT, 16);
      lcd.print(String(half_revolutions,DEC), RIGHT, 16);
      lcd.print("Speed:", LEFT, 24);
      lcd.print("Trip:", LEFT, 32);
      lcd.update();
      lcd.setFont(BigNumbers);
      lcd.printNumI(rpm, RIGHT, 24, 2,'0');
      lcd.update();

      half_revolutions = 0; // Restart the RPM counter
      lastmillis = millis(); // Uptade lasmillis
      attachInterrupt(0, rpm_fan, FALLING); //enable interrupt

   }
}

void rpm_fan()
{
   half_revolutions++;
}

Start debugging - ditch the LCD stuff, the RTC and the Strings, and just use the serial monitor to print really simple stuff.

I'd prefer to see "half_revolutions" qualified as "volatile".

if (millis() - lastmillis == 1000)

is much safer written as if (millis() - lastmillis >= 1000)

greatidea: Can somebody locate the problem and give a workaround solution to keep the program running consistently without freezing?

Your code is a mess. And wrong.

The prototype of the lcd.print function is:

void print(char *st, int x, int y);

Looked up here: https://github.com/MisaZhu/Arduino/blob/master/libraries/LCD5110_Graph/LCD5110_Graph.h

So the first parameter has to be a "char pointer" and NOT a "String object"!

That is (like ALL libraries) a library that works with C-strings (nullterminated strings) and NOT with "String object" like you are doing.

You send a "String object" as a first parameter, like:

      lcd.print(String(now.minute(), DEC), 18, 0);

Best practice in Arduino programming: AVOID "String" objects! Always! And in any Arduino program! Using "String" objects is BAD PROGRAMMING PRACTICE!

If you want to maintain bad programming practice (even programmers of Arduino example programs do that, too), you should at least use your "String" objects in a more correct way.

Instead of bullshit like:

      lcd.print(String(now.minute(), DEC), 18, 0);

I'd better try something like (not tested):

      lcd.print(String(now.minute()).c_str(), 18, 0);

The 'c_str()' function could retrieve the char pointer from a String object, that's what you need.

Or just use C-strings as usual:

  char str[7]; // declare some char buffer (will be used multiple times for integer conversions)
... and then when integers are needed to be converted into a string (untested, but should be OK)
  lcd.print(itoa(now.minute(),str,10), 18, 0);

That's what I saw on first sight on your code.

With interrupt handling I just found, that you do not declare your ' half_revolutions' variable as 'volatile'.

All variables, that are used in both, normal code and interrupt code, have to be declared 'volatile'.

See volatile reference.

AWOL: Start debugging - ditch the LCD stuff, the RTC and the Strings, and just use the serial monitor to print really simple stuff.

I'd prefer to see "half_revolutions" qualified as "volatile".

if (millis() - lastmillis == 1000)

is much safer written as if (millis() - lastmillis >= 1000)

Hi AWOL,

Thanks a lot for your suggestions. I finally got this working and I tested it rigorously and the system didn't freeze at all. I think only missing the volatile keyword was the problem.

Thanks a mil.

jurs: Your code is a mess. And wrong.

The prototype of the lcd.print function is:

void print(char *st, int x, int y);

Looked up here: https://github.com/MisaZhu/Arduino/blob/master/libraries/LCD5110_Graph/LCD5110_Graph.h

So the first parameter has to be a "char pointer" and NOT a "String object"!

That is (like ALL libraries) a library that works with C-strings (nullterminated strings) and NOT with "String object" like you are doing.

You send a "String object" as a first parameter, like:

      lcd.print(String(now.minute(), DEC), 18, 0);

Best practice in Arduino programming: AVOID "String" objects! Always! And in any Arduino program! Using "String" objects is BAD PROGRAMMING PRACTICE!

If you want to maintain bad programming practice (even programmers of Arduino example programs do that, too), you should at least use your "String" objects in a more correct way.

Instead of bullshit like:

      lcd.print(String(now.minute(), DEC), 18, 0);

I'd better try something like (not tested):

      lcd.print(String(now.minute()).c_str(), 18, 0);

The 'c_str()' function could retrieve the char pointer from a String object, that's what you need.

Or just use C-strings as usual:

  char str[7]; // declare some char buffer (will be used multiple times for integer conversions)
... and then when integers are needed to be converted into a string (untested, but should be OK)
  lcd.print(itoa(now.minute(),str,10), 18, 0);

That's what I saw on first sight on your code.

With interrupt handling I just found, that you do not declare your ' half_revolutions' variable as 'volatile'.

All variables, that are used in both, normal code and interrupt code, have to be declared 'volatile'.

See volatile reference.

Hi Jurs,

thanks for writing back, but the problem was just with the volatile keyword. The rest pertaining to the LCD was working great. I haven't tried your C-strings though