Speed Requirement for fast encoder and sensor !

Hello all,
In my current project, I use an encoder and an optical sensor to generate an output on an O/P pin.
Encoder Used : 1000 PPR Opto encoder
Sensor Used : 1Mhz Optical Sensor

I wrote code for this and here it is. It is fully interrupt based !

int time_to_fire = 500 ;
int product_state[25] ;
int product_pulses[25] ;
int product_num = 1 ;
int product_num_fire = 0 ;
//int numbers = 1 ;
int general_counter = 1 ;
int count_flag = 0 ;
int sense_flag = 0 ;

void setup()
{
  attachInterrupt(0,sense_flagfunc,RISING) ;
  attachInterrupt(1,count_flagfunc,RISING) ;
  pinMode(2,INPUT);
  digitalWrite(2,HIGH);
  pinMode(3,INPUT);
  digitalWrite(3,HIGH);
  pinMode(13,OUTPUT);
  pinMode(9,OUTPUT);
  flash();
  pinMode(7,OUTPUT);
  digitalWrite(7,HIGH);
  Serial.begin(9600);
  Serial.print("Loaded \n");
    
}

void loop()
{
  fire();
  count();
  sense();
}

void sense()
{
 if(sense_flag == 1)
  {
 //digitalWrite(13,HIGH);
 Serial.print("Sensed :");
 Serial.println(product_num);
 Serial.print("\n");
 product_state[product_num] = 1 ;
 product_num = product_num + 1 ;
 if(product_num > 24)
 {
   product_num = 1 ;
 }
}
sense_flag = 0 ;
}

void count()
{
  if(count_flag == 1)
  {
  
  for(general_counter = 1 ; general_counter < 26 ; general_counter = general_counter + 1)
  {
    if(product_state[general_counter] == 1)
    {
      product_pulses[general_counter] = product_pulses[general_counter] + 1 ;
      //flash();
      Serial.print("Pulse \n");
    }
  }
}
count_flag = 0 ;
}

void fire()
{
  for(product_num_fire = 1 ; product_num_fire < 26 ; product_num_fire = product_num_fire + 1)
  {
    if(product_pulses[product_num_fire] == 5)
    {
     //digitalWrite(13,HIGH);
     //delay(time_to_fire);
     //digitalWrite(13, LOW);
     flash();
     Serial.print("Fire \n");
     product_state[product_num_fire] = 0 ;
     product_pulses[product_num_fire] = 0 ;
     
    }
  }
}

void flash()
{
  digitalWrite(13,HIGH);
  delay(500);
  digitalWrite(13,LOW);
}
void flasher()
{
  digitalWrite(9,HIGH);
  delay(500);
  digitalWrite(9,LOW);
}

void sense_flagfunc()
{
  sense_flag = 1;
}

void count_flagfunc()
{
  count_flag = 1;
}

Forget the Serial Commands, I used them earlier to debug. Also forget the LED delays, not required.
This is used in an industrial setup, where the sensor and pin output are on a conveyor. The encoder is also fitted to the conveyor to measure distance. The aim here to sense a product under the opto sensor and give an output after a set distance determined by the encoder. Ex - Product gets sensed, product travels a set distance and when the product is under the output pin, pin goes HIGH and LOW for a set delay.

This program works, but the problem comes when the speed reaches it's maximum. At maximum speed, I get roughly 25000 pulses a second to the encoder interrupt and 27 pulses a second on the sensor interrupt. What starts to happen is; errors multiply, the set distance starts to offset by a big margin and at times the display freezes since there is no room to refresh it at all.

I was thinking probably to use a timer tick to poll the encoder and sensor and call a function. The call delay for the encoder comes to 40 microseconds and sensor call delay is roughly 27 to 30 milliseconds. Now, this is also a type of interrupt I believe.
I want your advice people, should I use a faster chip, something like the DUE or is it just pure 'in-efficient code' .

Somebody suggested I have a separate controller for the display and send values to the main board via serial, I don't want to do that, since it just increases components.

Also, as you can see in the code, as the products get sensed, they get cued in an array, i.e if the distance between the sensor and output is large, there are upto 25 products waiting in cue to receive an output.

SO, Please people, all ears for suggestions !! =( =(

BUG: array's in C go from 0.. n-1 unlike BASIC where they go from 1..n (several places)

change some datatypes, from int to byte (or uint8_t) use the smallest datatype possible
and make variables used in ISR's volatile (=> compiler won't optimize them away)

e.g
volatile byte count_flag = 0 ;
volatile byte sense_flag = 0 ;
byte general_counter=0;

a rewrite of count should be faster

void count()
{
  if (count_flag == 1)
  {
  for (general_counter =0; general_counter < 25 ; general_counter++)  //  <<<<<<< check the changes !!
  {
    if (product_state[general_counter] == 1)
    {
      product_pulses[general_counter] ++ ;  // just shoerthand
    }
  }
}
count_flag = 0 ;
}

attach the interrupt after setting the pinmode to input is better..

Okay, that's from the code perspective. In terms of the controller gulping 25000 pulses a second, don't you think a simlle 328p is unsuitable?, if so, can you suggest something more appropriate?

25000 pulses per second gives every 40usec a pulse. given that the call to the ISR is estimate 4uSec
implies you have 90% of the time to do other things.

Keep in mind the relationship between an encoder's PPR rating and how you process the encoder's A and B channels if using interrupts.

As shown in your sketch you are generating interrupts on both A and B channels on their RISING edges. So for a 1000 PPR encoder it will mean 2000 interrupts per revolution of the encoder. Your other options are:

Use both A and B interrupts using CHANGE will result in 4000 interrupts generated for each revolution of the encoder. And finally if you just use one encoder signal (A or B not both) to generate an interrupt using FALLING (or RISING) you will generate 1000 interrupts for this encoder.

It's a trade-off decision of encoder resolution capability Vs interrupt frequency per revolution. Often one doesn't require the maximum resolution possible (for example 4000 steps per revolution for a 1000 PPR) from a specific encoder and can lower the microcontrollers burden by running at one of the other modes explained above.

That make sense?

Your use of arrays contains numerous errors which could easily lead to erratic behaviour.

Array index values run from 0 to n-1 where n is the number of elements in the array.

Instead of scattering magic numbers throughout your code representing the size of your various arrays, define a constant for the number of elements in the array and use that.

const byte PRODUCT_COUNT = 25;
int product_state[PRODUCT_COUNT];

I am not clear what you are trying to achieve in count(), but the logic to shift the array contents does not cope correctly with end conditions and in any case should not be required. If you're trying to create a rotating data set you can achieve that by treating the array as a circular buffer, you don't need to shift the contents of the array.

Since you have a high interrupt frequency and relatively slow loop to process the resulting events, using a single flag to record the interrupt does not seem at all robust. In any case, the way you test and clear the flag inside sense() and count() leaves a huge timing window for interrupts to get lost. If you want to do processing for every interrupt, it would be better to have the interrupt handlers increment a volatile counter (it would only need to be a byte value) and in loop disable interrupts briefly while you test and decrement the counter - enable interrupts again before you go on to do whatever output is triggered by the interrupt. (On the other hand if you expect to get lots of interrupts and only have the first one trigger processing so that any others that occur during that processing get ignored, what you're doing is a good way to achieve that.)

Why does the count handler loop over an array at all.

Interrupt routines should do the minimum work possible. Looping over
26 elements of an array obviously isn't the minimum work possible, and this is slowing
down the handler a lot.

Normally counting a unidirectional encoder would be:

volatile long count ;

void handler ()
{
  count ++ ;
}