unexpected behaviour of encoder with increasing rpm

Question

I am using a DC motor with encoder, and I am trying to calculate how many pulses are recorded in every one second interval. (note that the power source is of 12V for the motor)

Circuit and code:

int encpin=3;
volatile long npc=0; //new pulse count
volatile long opc=0;  //original pulse count
volatile long pulsecount;
unsigned long int newtime;
unsigned long int prevtime=0;
unsigned long int time;
void setup()
{
  pinMode(encpin, INPUT_PULLUP);
  Serial.begin(9600);
  attachInterrupt(digitalPinToInterrupt(encpin),function,RISING);
}
void function(){
  npc++;
}
void loop()
{
 newtime=millis();
 time=newtime-prevtime;
 pulsecount=npc-opc;
  if(time==1000){
    Serial.println(pulsecount);
    prevtime+=1000;
    opc=npc;
  }
}

It seems to be rather intuitive that increasing the rpm should increase the number of pulses recorded in one second. However, the results are rather erratic:

 RPM  PULSECOUNT
  26    2365
  32    2367
  38    2400
  44    2375
  52    2400
 116    2412
 142    2384
 195    2416
 280    2406
 350    3008
 416    1580
 520    1975
 624    2370
 730    2334
2737    2188

Increasing RPM sometimes increases the pulsecount, but sometimes decreases it....what could be the reason for this behaviour?

Answer 1

I think the problem is in the tinkercad simulation: The encoder is attached to the motor and the motor is always rotating at the same speed. Selecting a different RPM on the component changes the gear ratio and the RPM of the shaft, but not of the motor. That means that the number output after 1s should be always the same. Why it is not exactly the same, only the programmers of the simulation can tell. It might have to do with simulation speed, because I get totally different numbers there.

To tell exactly what is going on, a documentation of the components in the simulation and of the simulation would be necessary.

Answer 2

One important principle to learn when working with interrupts is that of atomic operations. It is vital, when working with any data that cannot be manipulated by one single assembly instruction, that you protect against that data being changed midway through any reading of that data.

In your situation you are working with 32-bit values. On an 8-bit microcontroller those take many many instructions to manipulate, and at any time during that manipulation the interrupt can be triggered and change the value, corrupting your readings.

There is a concept in programming called critical sections. These are "blocked off" bits of code where no interrupts are allowed to happen while you do whatever is needed to your shared variables. For obvious reasons you want to keep those sections as brief as possible.

Also you don't need to calculate the pulse count by comparing it to the previous count - just start from 0 each second. And on the topic of a second, it's best to check if at least a second has passed. There's always a chance you may see 1001ms instead of 1000ms. A slim chance, but best to be safe.

Here's an example, using your code as a base:

void loop() {

    if (millis() - prevtime >= 1000) { // you should check for more time passing than you expect
        prevtime += 1000; // Don't assume you always had 1 second, force it instead
   
        // This block is the "critical section". Keep it as short as possible
        // so just grab the data and reset the count.
        noInterrupts(); // Stop any interrupts happening
        uint32_t currentCount = npc; // Grab this second's count
        npc = 0; // And reset the count to zero
        interrupts(); // Release the block on interrupts

        // And output the results.
        Serial.println(currentCount);
    }

}