Variable type randomly changing when code is running

I'm trying to read a quadrature encoder data from few DC motors to control it's speed with a PID controller, in an Arduino Mega 2560. But when the code is running, the variable I'm using to store the encoder counts at a time period is randomly changing it's type from signed long to some unsigned type and back randomly, giving erroneous data for rest of the code to process.

for example, in the following code I'm just reading the encoder and showing the encoder count per 10ms in serial monitor:

#define sampling_time 10
#define motor1_pin  8
#define direction1_pin 9

volatile signed long encoder_count = 0;

unsigned long previous_time = 0;
unsigned long current_time =0;
int time_change =0;



void setup() {
 Serial.begin(2000000);     
 
 attachInterrupt(digitalPinToInterrupt(20),EncoderRead,RISING); 
 
 pinMode(direction1_pin,OUTPUT);
 digitalWrite(direction1_pin,LOW);


 }

void loop() {
 
  current_time = millis();
  time_change = current_time - previous_time;

  if (time_change > sampling_time)
{
  float data = encoder_count;
  encoder_count = 0;

  analogWrite(motor1_pin,170);
  
  Serial.print(data);Serial.print("  ");
  Serial.println("uT");

  previous_time = current_time;
}
}



//...................................Function to read the encoder 1 state once the interrupt occurs.............................................//
void EncoderRead(){                                                
  if (digitalRead(20)==HIGH && digitalRead(2)==LOW)                //Based on the forward square waves generated by the encoder. See diagram
  {           
    encoder_count++;
  }
  else{                                                             //If wheel is not going forward, reduce count
    encoder_count--;
  }
    
}

Now, when the motor turning forward and the encoder count is positive, everything works as it should. But when the motor is turning backward, the encoder count should be some negative value (close to -30 at the set speed). But it randomly shoots up to some very large positive value, like shown below.

-31.00  uT
-32.00  uT
-28.00  uT
-32.00  uT
-31.00  uT
-32.00  uT
-31.00  uT
-32.00  uT
-31.00  uT
-29.00  uT
-31.00  uT
-32.00  uT
-31.00  uT
-29.00  uT
-31.00  uT
-32.00  uT
-31.00  uT
-29.00  uT
-31.00  uT
-31.00  uT
65504.00  uT
-28.00  uT
-32.00  uT
-31.00  uT
-32.00  uT
-28.00  uT
-32.00  uT
-31.00  uT
-32.00  uT
-28.00  uT
-32.00  uT
-31.00  uT
-32.00  uT
-31.00  uT
-32.00  uT
-31.00  uT
-29.00  uT
-31.00  uT
-31.00  uT
-32.00  uT
-29.00  uT
-31.00  uT
-31.00  uT
-32.00  uT
-29.00  uT

More interestingly, the high values are sometime in the range of 65k, sometimes in the range of 16M, and sometimes in the range of 224, and always binary 2's complement for the expected value. For example,

16777187.00 =111111111111111111100011=-29(24 bit 2's complement)
65507.00 = 1111111111100011 = -29 (16 bit 2's complement)
223.00 =11011111= -33(8 bit 2's complement)

But my variable, "encoder_count" is declared as long(32 bit in arduino mega2560).

Which makes me think that my program is sometimes considering the variable as some different unsigned type and interpreting the binary value differently. But I have no idea why and how to fix it.

So far I have tried changing motors, using different pins, different arduino modules to eliminate hardware issues.

Interrupts need to be disabled while you are accessing the variable used within the interrupt routine. You are occasionally having an interrupt occur while you are using encoder_count in your code, because a 4-byte variable takes several instruction cycles to access. You are getting part of the variable before the interrupt and the rest after the interrupt has altered its value.

Using a signed long for a 10mS sampling period seems excessive, I'd use an integer or byte if possible. Also, it would probably take less time to copy encoder_count to another variable of the same type, instead of converting it to a float. You want to minimize the amount of time you have interrupts disabled or you might get a false direction indication.

Thanks. I tried changing the encoder_count variable to a BYTE, and it seems to be working fine without the random spikes for now. I’m doing further testing to see if it holds.

Could you point me towards how can I disable interrupts when reading the variable? For the current problem using a smaller variable type works as the encoder reading will not cross the limits of a BYTE. But in future I might need to deal with larger values where I’d need to disable the interrupt.

Here is an extensive description of interrupts:

Gammon Forum : Interrupts

Using a byte variable generally will eliminate the problem you were having, because it can be accessed in a single operation, which cannot be interrupted. The only problem you might encounter is if an interrupt occurs between when you are accessing the variable, and the following instruction where you are setting it to zero, but that would be limited to the loss of a single count on the rare occasion that it happens.

david_2018:
Here is an extensive description of interrupts:

Gammon Forum : Interrupts

Using a byte variable generally will eliminate the problem you were having, because it can be accessed in a single operation, which cannot be interrupted. The only problem you might encounter is if an interrupt occurs between when you are accessing the variable, and the following instruction where you are setting it to zero, but that would be limited to the loss of a single count on the rare occasion that it happens.

Thanks. I did this:

  ...
  noInterrupts();
  float data = encoder_count;
  encoder_count = 0;
  interrupts();
  ...

And it's running for more than 10 minutes now without any spikes. I think I can move forward with this solution now.

Why are you making 'data' a float type? You have 'encoder_count' as 'signed long' assigning it to a float provides no advantage.

gfvalvo:
Why are you making 'data' a float type? You have 'encoder_count' as 'signed long' assigning it to a float provides no advantage.

The data is converted to float because on the full version of the code there is a second order filter and a PI controller that works with this data. To avoid any hidden errors due to data type conversion on that portion, I'm converting all data to float at the beginning. So far, I haven't run out of memory, yet. When I do, then I'll have to check and start optimizing data types.