How to unify all RPS reading in one function?

Question

Want to unify rps reading in one function. However, it shows incorrect values. when it is reading for one it works fine. Following is the code. Appreciate for your help.

float rps[4] = {0.00, 0.00, 0.00, 0.00};
long int PRINT_PERIOD = 1000;
long int lastTimePrinted = 0;

void setup() {
    Serial.begin(9600);
    DDRD &= ~_BV(7); // pinMode(7, INPUT);     // rps_1
    DDRB &= ~_BV(0); // pinMode(8, INPUT);     // rps_2
    DDRC &= ~_BV(1); // pinMode(A1, INPUT);    // rps_3
    DDRD &= ~_BV(2); // pinMode(2, INPUT);     // rps_4
}

float rps_out(int k) {
    static int lastReedState;
    static unsigned long lastTransition;
    static float rps_result;
    int reedState;
    unsigned long now;
    unsigned long revolutionTime;

    if (k == 1)    reedState = (PIND & _BV (7)) == 0; // digitalRead(7);
    else if (k==2) reedState = (PINB & _BV (0)) == 0; // digitalRead(8);
    else if (k==3) reedState = (PINC & _BV (1)) == 0; // digitalRead(A1);
    else if (k==4) reedState = (PIND & _BV (2)) == 0; // digitalRead(2);

    // HIGH LOW measuring the low period
    // LOW HIGH measuring the high period
    // On a rising transition of the reed switch:
    if (reedState == HIGH && lastReedState == LOW) {

        // Compute time since last valid transition.
        now = micros();
        revolutionTime = now - lastTransition;    // measuring the High period
         rps_result = 1.00 / (12.00*revolutionTime) *1000000.00;  // 12 is rotary encoders resolution 
        // Remember this transition.
        lastTransition = now;
    }

    // Remember last state.
    lastReedState = reedState;

    return rps_result;
}

void loop() {
    rps[0] = rps_out(1);
    rps[1] = rps_out(2);
    rps[2] = rps_out(3);
    rps[3] = rps_out(4);

    if (millis() - lastTimePrinted >= PRINT_PERIOD) {
        Serial.println(rps[0]);
        Serial.print(',');
        Serial.println(rps[1]);
        Serial.print(',');
        Serial.print(rps[2]);
        Serial.print(',');
        Serial.println(rps[3]);
        lastTimePrinted = millis();
    }
}

worked code except for rps[3] at the beginning. it shows rps[3] = 672.04

float rps[4];
long int PRINT_PERIOD = 100;
long int lastTimePrinted = 0;
int lastReedState[4];
unsigned long lastTransition[4];
int reedState[4];
unsigned long now[4]; 
float rps_result[4];   
unsigned long revolutionTime[4];

void setup(){

 Serial.begin(9600);

 DDRD &= ~_BV (7); // pinMode (7, INPUT);       // rps_1
 DDRB &= ~_BV (0); // pinMode (8, INPUT);       // rps_2
 DDRC &= ~_BV (1); // pinMode (A1, INPUT);      //  rps_3
 DDRD &= ~_BV (2); // pinMode (2, INPUT);       //  rps_4


}


float rps_out(int k){

    //static int lastReedState;
    //static unsigned long lastTransition;
    //static float rps_result; 
    //float rps_result;
    //int reedState;
    //unsigned long now;    
    //unsigned long revolutionTime;

   if ( k == 0 )     reedState[0] = (PIND & _BV (7)) == 0; // digitalRead (7);
   else if ( k==1 )  reedState[1] = (PINB & _BV (0)) == 0; // digitalRead (8);
   else if ( k==2 )  reedState[2] = (PINC & _BV (1)) == 0; // digitalRead (A1);
   else if ( k==3 )  reedState[3] = (PIND & _BV (2)) == 0; // digitalRead (2);

    // On a rising transition of the reed switch:
    if (reedState[k] == HIGH && lastReedState[k] == LOW) {                   // HIGH  LOW   measuring the low period    // LOW HIGH measuring the high period 
        // Compute time since last valid transition.
        //unsigned long now = micros();
        //unsigned long revolutionTime = now - lastTransition;       // measuring the High period
         now[k] = micros();
         revolutionTime[k] = now[k] - lastTransition[k];       // measuring the High period
        // Compute the rps

        if(revolutionTime[k] == 0 ) rps_result[k] = 0.00;     // avoid deviding by zero


        else rps_result[k] = 1.00 / (12.00*revolutionTime[k]) *1000000.00;  // 12 is rotary encoders resolution 

        // Remember this transition.
        lastTransition[k] = now[k];   

    }

    // Remember last state.
    lastReedState[k] = reedState[k];


    return rps_result[k]; 

}


void loop() {

   rps[0] = rps_out(0);
   rps[1] = rps_out(1);
   rps[2] = rps_out(2);
   rps[3] = rps_out(3);

    if (millis() - lastTimePrinted >= PRINT_PERIOD) {   

     Serial.print(rps[0]);
     Serial.print(',');
     Serial.print(rps[1]); 
     Serial.print(',');
     Serial.print(rps[2]);
     Serial.print(',');
     Serial.println(rps[3]);

      lastTimePrinted = millis();

     }

}

Answer 1

Your problem is that you have one set of (static) state information that you are trying to use for all your inputs. That's never going to work. You need separate state information for each and every input.

There's numerous ways of doing that:

1. The traditional C library "re-entrant" style. This involves passing pointers to any state variables into your function. The caller is responsible for creating and managing these variables and passing the pointers when the function is called. This is messy but pretty much completely portable across systems since it doesn't rely on any C++ facilities.
2. Similar to option 1 but grouping your different state variables into a struct to make things cleaner.
3. Global arrays - have a global array (maybe of structs as in option 2) to store the state information and pass the slice number to your function.
4. The OOP (C++) way - create a class with all the state variables and functions within it then instantiate a new object for each input. This is by far the cleanest and most self contained way but does require knowledge of creating and using classes.

Of the four options I would most likely choose option 4:

class RPS {
    private:
        uint8_t _pin;
        uint8_t _lastReedState;
        uint32_t _lastTransitionTime;
        uint32_t _rps;

    public:
        RPS(uint8_t pin) : _pin(pin) {}
        void begin() {
            pinMode(_pin, INPUT);
            _lastReedState = digitalRead(_pin);
            _lastTransitionTime = millis();
            _rps = 0;
        }
        void calc() {
            uint8_t reedState = digitalRead(_pin);
            if (reedState != _lastReedState) {
                uint32_t now = millis();
                _lastReedState = reedState;
                if (reedState == HIGH) {
                    uint32_t diff = now - _lastTransitionTime;
                    if (diff == 0) {
                        _rps = 0; // Avoid divide by zero
                    } else {
                        _rps = 1000 / diff;
                    }
                }
                _lastTransitionTime = now;
            }
        }
        uint32_t get() {
            return _rps;
        }
};

RPS rps1(1);
RPS rps2(2);

#define PRINT_PERIOD 1000

void setup() {
    rps1.begin();
    rps2.begin();
}
void loop() {
    static uint32_t lastTimePrinted = 0;

    rps1.calc();
    rps2.calc();

    if (millis() - lastTimePrinted >= PRINT_PERIOD) {
        lastTimePrinted = millis();
        Serial.print("RPS 1: ");
        Serial.println(rps1.get());
        Serial.print("RPS 2: ");
        Serial.println(rps2.get());
    }
}

(note: Untested. It may not even compile)