Time critical code crippled by LCD output

Hi,

After receiving a lot of help from forum members I completed my interrupt code to measure flow speeds through a turbine. - not mentioning names since I do not want to miss someone.
The device has not gone through callibration yet, so all I have is raw time outputs. One requirement though was reliable direction detection.
This was achieved and was working very well.

I started adding the rest of the code, in particular the output to LCD.
The problem is that the output to the LCD, even for 3chars of text, takes about 60Ms with the u8Glib library and about the same with the PCD8544 library.

This causes the direction detection to go completely haywire. Normally I get a ratio (between half revolution time and trigger time between sensors) of 3 or 5 and with the LCD output it jumps to 20!

The problem seems to be that I am saving the half revolution and trigger difference at the start of each loop, and when the loop is running too long, the next value cannot be saved and keeps on incrementing.

Thus I have two options, one is to abandon the saving of the values, which previously hampered the code since the value was read before B has triggered, resulting in negative values. The check for (DirT>0) might solve this - and I will see if it works now - but that was added after the code was stable, so I have not fretted with it since.
With and without the storage, I am probably missing quite a couple of readouts.

Second option:
I think this is possible: each time the interrupt has a value to store, it does so and puts it into an expandable array and increments a count.
When the loop eventually calls the array it does so along with the count, and then calculates the average based upon the count.
While the loop is busy it flags itself as busy, which then tells the ISR to keep on updating the array.
This approach might present a problem, ie. averages can skew your data, expecially when it gets caught during a reverse of direction.

Both options present problems.

Any help and suggestions would be much appreciated - I am at a bit of a loss as to what I should do.

My code is below: - note, the LCD output is uncommented. And I know serial slows it down as well, but necessary for debugging.

//Libraries
#include "U8glib.h"

//LCD
U8GLIB_PCD8544 u8g(13, 11, 10, 9, 12);                    //SCK, MOSI, CS, A0, Reset

//variables
volatile long DirT;	//time between A Falling and B Rising
volatile long HalfrevT;	//time of one half revolution
volatile unsigned long Atrigger;	//time when A is triggered (Falling)
volatile boolean flag = false;	//flag to keep B from being less than A
boolean Dirflag;	//true = clockwise

//storage variables
volatile long Time_store; //time storage
long DirT_store;  //time difference storage
long HalfrevT_store; //half-revolution time storage

//calculated values
long Volume;
long VolumeKG; //Volume/KG
long Frate;  //Flowrate
double Weight = 60;  //initial weight

//Inputs/Outputs (rubbish pin assignments)
//LEDs - All on PortC
int T_Red = 18; //A4
int T_Orange = 17;
int Green = 16;
int B_Red = 15;
int B_Orange = 14; //A0

//Buzzer
int Buzzer = 19; //A5 - only on Arduino Nano

//Buttons
int Left = 4;
int OK = 5;
int Right = 6;

//Function declaration
boolean Direction();	//Calculates the ratio between DirT and halfrev
long calc_Volume();	//Calculates the volume of air passing through the turbine and outputs data to LCD
long calc_FRate();	//Calculates the rate of flow through the turbine
void LEDs_Buzzer();	//Determines which LED to light and if the Buzzer should sound

//Interrupts
void A_ISR()	//Interrupt service routine for A(Falling)
{
  Time_store = micros();	//Time is stored
  if (!flag)
  {
    HalfrevT = Time_store - Atrigger;	//halfrevolution is current time - previous trigger time
    Atrigger = Time_store;	//sets trigger to the stored time
  }
}

void B_ISR()	//Interrupt service routine for B(rising)
{
  if (!flag)
  {
    DirT = micros() - Atrigger;	//DirT is current time - the time A was triggered
    flag = true;	//sets the flag so that A is always less than B
  }
}

void setup()
{
  Serial.begin(115200);
  Serial.println("Interrupt, direction and speed test:");
  Atrigger = micros();
  attachInterrupt(0, A_ISR, FALLING);    //A is pulled low by Pin 2
  attachInterrupt(1, B_ISR, RISING);     //B rises on Pin 3
  
  u8g.setFont(u8g_font_unifont);
  
}

void loop()
{
  if (flag)
  {
    if (DirT>0)
    {
      //store variables
      DirT_store = DirT;
      HalfrevT_store = HalfrevT;
      flag = false;
      //calculations
      Serial.print(DirT);
      Serial.print(" | ");
      Serial.print(HalfrevT);
      Serial.print(" | RPM: ");
      Serial.print(1/(HalfrevT*0.000002)*60);
      Serial.print(" | ");
      Serial.print(HalfrevT/DirT);
      Serial.print(" | ");
      Direction();
      calc_Volume();
      calc_FRate();
      LEDs_Buzzer();
      
//u8g.firstPage(); //LCD printouts are VERY slow!!! 60Ms/60000uS
//  do
//  {
//draw();
//  }
//  while( u8g.nextPage() );

    }
  }
  
}

//Functions (rubbish calculations except for direction)
boolean Direction()
{
  if (HalfrevT/DirT<4)
  {
     Serial.print("In");
    Dirflag = true;
  }
  else
  {
    Serial.print("Out");
    Dirflag = false;
  }
}

void draw(void)
{
if (Dirflag)
    {
    u8g.drawStr( 0, 36, "In");
    }
  else
  {
    u8g.drawStr( 0, 36, "Out");
  }
}

long calc_Volume()
{
  Volume = 1.035*HalfrevT;
  VolumeKG = Volume/Weight;
  if (Dirflag)
    {
//      Serial.print("Vol_IN: ");
//      Serial.print(Volume);
//      Serial.print("L | Vol/KG_IN: ");
//      Serial.print(VolumeKG);
//      Serial.print("L/KG | ");
u8g.drawStr( 0, 10, "Vin");
    }
  else
    {
//      Serial.print("Vol_OUT: ");
//      Serial.print(Volume);
//      Serial.print("L | Vol/KG_OUT: ");
//      Serial.print(VolumeKG);
//      Serial.print("L/KG | ");
u8g.drawStr( 0, 10, "Vout");
    }
  return Volume;
}

long calc_FRate()
{
  Frate = 1.02*1586*pow(HalfrevT,-1.013);
  if (Dirflag)
    {
//      Serial.print("Frate_IN: ");
//      Serial.println(Frate);
    }
  else
    {
//      Serial.print("Frate_OUT: ");
//      Serial.println(Frate);
    }
  return Frate;
}

void LEDs_Buzzer()
{
  //Turn of all LEDs and buzzer
  digitalWrite(T_Red, LOW);
  digitalWrite(T_Orange, LOW);
  digitalWrite(Green, LOW);
  digitalWrite(B_Orange, LOW);
  digitalWrite(B_Red, LOW);
  digitalWrite(Buzzer, LOW);
	
  if (Frate<5)
    {
      digitalWrite(T_Red, HIGH);
    }
  else if (Frate<10)
    {
      digitalWrite(T_Orange, HIGH);
    }
  else if (Frate<15)
    {
      digitalWrite(Green, HIGH);
    }
  else if (Frate<20)
    {
      digitalWrite(B_Orange, HIGH);
    }
  else
    {
      digitalWrite(B_Red, HIGH);
      digitalWrite(Buzzer, HIGH);
    }
}

I’m reluctant to put this here since it might take over the posts, but a third and crazy(ish) idea would be to add a secondar AVR to output to the LCD while the Arduino does the calculations etc. Since I do not have too much time I do not think this is very feasible, but an interesting idea nonetheless. Is it possible to be implemented quickly? I have zero experince with programming AVRs. ie. outside the Arduino ecosystem.
Just to reiterate, this is just a crazy idea - ignore if crazy.

Instead of doing 8 Serial prints you could build a string of what you want to print and display it with just one Serial.print. Just my 2c from a quick look.

      Serial.print(DirT);
      Serial.print(" | ");
      Serial.print(HalfrevT);
      Serial.print(" | RPM: ");
      Serial.print(1/(HalfrevT*0.000002)*60);
      Serial.print(" | ");
      Serial.print(HalfrevT/DirT);
      Serial.print(" | ");

Having the below constants defined instead of being variables could also improve speed.

//Inputs/Outputs (rubbish pin assignments)
//LEDs - All on PortC
int T_Red = 18; //A4
int T_Orange = 17;
int Green = 16;
int B_Red = 15;
int B_Orange = 14; //A0

//Buzzer
int Buzzer = 19; //A5 - only on Arduino Nano

//Buttons
int Left = 4;
int OK = 5;
int Right = 6

Why don't you add a counter to your main loop and only display data on the LCD every N iteration of the loop, there is no point on writing to the LCD everycycle, if you update it a couple of times a second or when the data changes, (which ever is longer), that should help considerably.

Something like:

#define DISPLAY_RATE   25 // This is the number of loop iterations

int intLCDdisplayCtr = 0;

void loop() {

  if ( (intLCDdisplay++ % DISPLAY_RATE) == 0 ) {
// Your display code here
  }
}

hellonearthis: Instead of doing 8 Serial prints you could build a string of what you want to print and display it with just one Serial.print. Just my 2c from a quick look.

Serial printouts are not the problem - they are only for debugging and take a fraction of the time LCD printouts take. They are seperate since I do not always need all of them, and un/comment them as needed. With or without them the LCD code completely buggers the reliability.

SPlatten: Why don't you add a counter to your main loop and only display data on the LCD every N iteration of the loop, there is no point on writing to the LCD everycycle, if you update it a couple of times a second or when the data changes, (which ever is longer), that should help considerably.

Something like:

#define DISPLAY_RATE   25 // This is the number of loop iterations

int intLCDdisplayCtr = 0;

void loop() {

  if ( (intLCDdisplay++ % DISPLAY_RATE) == 0 ) { // Your display code here   } }

Thanks, I'll try this after lunch and let you know how it goes.

SPlatten:
Why don’t you add a counter to your main loop and only display data on the LCD every N iteration of the loop, there is no point on writing to the LCD every cycle, if you update it a couple of times a second or when the data changes, (which ever is longer), that should help considerably.

HAH!!! Thank you!!! It works! Will have to test the speeds with a scope and some graphing, but for now it seems to work great!

Thanks for the help!

Glad I could help.

You could add an ATmega on SPI bus to your design to deal with the LCD and leave the first one to do the time critical work. Even only printing once in a while you still get breaks in monitoring motion.

Another method you could implement which could be more effecient would be to create a class to manage the LCD, in the class it would have two character maps representing the display. Your class would have a method to write a string to a specifc X,Y location. Internally the two maps would be compared and any differences would be written to the LCD display, you could also throttle the writing to the display with a timer.

I think there's an error because you store the volatile variables into the _store ones (which is correct), but then you use the volatile ones throughout the code.

That sort of defeats the purpose of the _store variables in the first place, IMHO.

HTH