Motor Encoder Random Data

Hello!

I have some issues with an encoder on a motor from Pololu Motor, however I have been having some issues. First of all, this is a diagram of the motor circuit:


I'm going to ignore the circuit for the controller as this works fines (and this problems shows up even when the motor gets current directly from the voltage generator at 12V).
The pins A and B are connected to the arduino pins 2 and 3 respectively. While the encoder VCC and GND are connected to a voltage generator (3.3V). Where I see that the encoder draws 5mA of current*.
When I turn on my Arduino I see that I get some random data quite randomly: I see a lot of it when I'm handling the motor. I made sure that there's no touching between different wires and no wires get disconnected. Additionally, I looked online and saw that it's normal to get some noise, especially if the wires carrying current o the motor are next to the encoder wires, so I set the pins as "INPUT_PULLUP"** and this got rid of most of the noise. However, when the motor runs, I still RARELY see any position data.

I have tried different Arduinos as well, so the problem isn't in the Arduino or any of the components of my circuit.

#define readA bitRead(PIND,2)//faster than digitalRead()
#define readB bitRead(PIND,3)//faster than digitalRead()

#define ENCA 2
#define ENCB 3

long posi = 0;
long last_x_pos = -1;

void setup() {
  // Wait for a second to ensure stable power and connections
  delay(1000);
  mcSerial.begin(assignBaudRate);  // Start communication with motor controller
  Serial.begin(assignBaudRate);
  Serial.println("Starting setup");
  // pinMode(ENCA, INPUT);
  // pinMode(ENCB, INPUT);
  pinMode(2, INPUT_PULLUP); // Enable internal pull-up resistor on pin 2
  pinMode(3, INPUT_PULLUP); // Enable internal pull-up resistor on pin 3
  attachInterrupt(digitalPinToInterrupt(ENCA),readEncoderA,CHANGE);
  attachInterrupt(digitalPinToInterrupt(ENCB),readEncoderB,CHANGE);
  Serial.println("Setup complete");
}

void loop() {
  if (posi != last_x_pos) {
    Serial.println(posi);
    last_x_pos = posi;
  }

  // delay(10);
}

// Interrupt service routine for reading the encoder
void readEncoderA(){
  if(readB != readA) {
    posi ++;
  } else {
    posi --;
  }
}
  
void readEncoderB(){
    if (readA == readB) {
    posi ++;
  } else {
    posi --;
  }
}

Any help would be appreciated.

Extras

While the code above gives me issues I also added the entirety of the code that's used to control the motor, just in case. In my opinion it can be ignored, and the code above is all that matters.

#include <Motoron.h>
#include <SoftwareSerial.h>

#define readA bitRead(PIND,2)//faster than digitalRead()
#define readB bitRead(PIND,3)//faster than digitalRead()

#define ENCA 2
#define ENCB 3

long posi = 0;
long last_x_pos = -1;

SoftwareSerial mySerial(10, 11);  // RX, TX

#define mcSerial mySerial  // Use SoftwareSerial for the Motoron controller

MotoronSerial mc;

const uint32_t assignBaudRate = 115200;

static_assert(assignBaudRate >= MOTORON_MIN_BAUD_RATE &&
  assignBaudRate <= MOTORON_MAX_BAUD_RATE, "Invalid baud rate.");

bool initializeMotoren() {
  mc.reinitialize();
  mc.disableCrc();
  mc.clearResetFlag();
  
  // Verify initialization
  uint16_t errors = mc.getErrorMask();
  return (errors == 0);
}

void setup() {
  // Wait for a second to ensure stable power and connections
  delay(1000);
  mcSerial.begin(assignBaudRate);  // Start communication with motor controller
  Serial.begin(assignBaudRate);
  // pinMode(ENCA, INPUT);
  // pinMode(ENCB, INPUT);
  pinMode(2, INPUT_PULLUP); // Enable internal pull-up resistor on pin 2
  pinMode(3, INPUT_PULLUP); // Enable internal pull-up resistor on pin 3
  attachInterrupt(digitalPinToInterrupt(ENCA),readEncoderA,CHANGE);
  attachInterrupt(digitalPinToInterrupt(ENCB),readEncoderB,CHANGE);

  Serial.println("Starting setup");

  mcSerial.setTimeout(1); //Can be changed to get better values for pos (the lower the more precise)
  mc.setPort(&mcSerial);

  // Initialize the Motoron
  initializeMotoren();

  // Configure the motor (motor 1 only)
  mc.setMaxAcceleration(1, 0);
  mc.setMaxDeceleration(1, 0);

  Serial.println("Setup complete");
}

void processCommandInt(int speed){
  mc.setSpeed(1, speed);
}

void loop() {
  while (Serial.available() > 0) {
    String input = Serial.readStringUntil('\n');
    input.trim();  // Remove any leading/trailing whitespace
    // Check if the input is an integer or string
    if (input.toInt() != 0 || input == "0") {  // toInt() returns 0 if not a valid integer
      int intInput = input.toInt();
      processCommandInt(intInput);
      Running = true;
      input = "";
    }

    // delay(100);
  }

  if (posi != last_x_pos) {
    Serial.println(posi);
    last_x_pos = posi;
  }

  // delay(10);
}

// Interrupt service routine for reading the encoder
void readEncoderA(){
  if(readB != readA) {
    posi ++;
  } else {
    posi --;
  }
}
  
void readEncoderB(){
    if (readA == readB) {
    posi ++;
  } else {
    posi --;
  }
}

*The encoder supports voltages between 2.7V and 18V, when I switch between 3V and 5V I see no different in the current drawn, shouldn't it change if the voltage changes?

**I have noticed that the arduino reports some position data when the encoder is not plugged in and I'm handling the arduino, whether that's touching the 2 and 3 pins or any other pins. However setting the pins as INPUT_PULLUP removes this behaviour.

Please post a complete wiring diagram of the Arduino+encoder+power supply, with pins and connections clearly labeled. Hand drawn is preferred.

Also, post complete code that compiles, runs and demonstrates the problem. The posted code has fatal errors.

I have noticed that the arduino reports some position data when the encoder is not plugged in and I'm handling the arduino.

That is expected behavior, due to floating inputs. Do not change wiring if the circuit is powered.

Sorry, here's the corrected code.

#define readA bitRead(PIND,2)//faster than digitalRead()
#define readB bitRead(PIND,3)//faster than digitalRead()

#define ENCA 2
#define ENCB 3

long posi = 0;
long last_x_pos = -1;

const uint32_t assignBaudRate = 115200;

void setup() {
  // Wait for a second to ensure stable power and connections
  delay(1000);
  Serial.begin(assignBaudRate);
  Serial.println("Starting setup");
  // pinMode(ENCA, INPUT);
  // pinMode(ENCB, INPUT);
  pinMode(2, INPUT_PULLUP); // Enable internal pull-up resistor on pin 2
  pinMode(3, INPUT_PULLUP); // Enable internal pull-up resistor on pin 3
  attachInterrupt(digitalPinToInterrupt(ENCA),readEncoderA,CHANGE);
  attachInterrupt(digitalPinToInterrupt(ENCB),readEncoderB,CHANGE);
  Serial.println("Setup complete");
}

void loop() {
  if (posi != last_x_pos) {
    Serial.println(posi);
    last_x_pos = posi;
  }

  // delay(10);
}

// Interrupt service routine for reading the encoder
void readEncoderA(){
  if(readB != readA) {
    posi ++;
  } else {
    posi --;
  }
}
  
void readEncoderB(){
    if (readA == readB) {
    posi ++;
  } else {
    posi --;
  }
}

Here's the output:


To note: the position should increase like the following, not go back and forth, like this:
image

Here's the hand-drawn diagram:

Let me know if you need anything else.

EDIT: By Voltage generator I mean Power Supply.

What is the "voltage generator"?

How is the motor supposed to be controlled?

You should use the Nano Vcc (5V output) to power the encoder, and there must be a common ground connection between the Nano and the encoder.

For voltage generator I mean Power Supply (I'm sorry english isn't my first language).

In this case the motor is running continuously, just to check the encoder. In the future I'll control it with a Controller.

I tried connecting the encoder VCC and GND to the 5V and GND pins respectively on the Arduino, but I don't remember any difference, I'll try again. Do you expect any difference? Why can't I power the encoder from a power supply?

I expect the encoder to work correctly, if it is wired correctly. According to your diagram and the symptoms you report, it is not wired correctly.

See the tutorial in the General Electronics forum category, which explains why common ground is required.

Assuming that the encoder is wired correctly, the Arduino may not be able to handle the high rate of encoder counts expected, if the motor is driven at full speed.

If you can, turn the (unpowered) motor shaft by hand, to check proper encoder function.

The above is a problem. Variables shared with interrupts MUST be declared volatile, like this:

volatile long posi = 0;

Furthermore, you must protect such variables from being corrupted by the interrupt routine, while being accessed by the main program. Make a copy with the interrupts turned off, like this:

  noInterrupts();
  long pos_copy = posi;
  interrupts();
if (pos_copy != last_x_pos) {
    Serial.println(pos_copy);
    last_x_pos = pos_copy;
  }

All of this could be avoided if you simply used the excellent Arduino Encoder library.

1 Like

By this you're talking about the wiring of VCC and GND of the encoder? As in, once I connect the encoder VCC and GND to the 5V and GND pins respectively on the Arduino it should be wired correctly, right?

What is the limit? I don't expect to go at very high speeds, but I'd prefer not lose any data. Is there any way to increase that limit in case it's too low? A better Arduino? Some other hardware?

Sadly I can't. It's too strong.

That is set by the MCU clock rate and instruction set.

If the ~30 year old MCU design used in the Nano is not fast enough, use an Arduino with a faster MCU (ESP32, SAMD21, etc.).

Sadly I can't. It's too strong.

Then use a very low motor voltage for initial tests. It should run at low speed on 2V.

Thank you. I'll fix the wiring and use the Encoder library.
The basic example of this library is this:

long oldPosition  = -999;

void loop() {
  long newPosition = myEnc.read();
  if (newPosition != oldPosition) {
    oldPosition = newPosition;
    Serial.println(newPosition);
  }
}

the variables "newPosition" and "oldPosition" and aren't "volatile", I assume that's because the function

myEnc.read()

returns a volatile variable, right?

And I should have

  pinMode(2, INPUT_PULLUP);
  pinMode(3, INPUT_PULLUP);

instead of:

 pinMode(ENCA, INPUT);
 pinMode(ENCB, INPUT);

Correct?

By the way, thank you very much for your help! I'll implement all of your suggestions on Monday.

No.
Very, very wrong. to the point of being nonsense.

Also do not connect the +5V from your power supply to any other 5V source in your circuit. Only the grounds must be common.

Okay so I assume everything is taken care of by the library, and I only declare the variables as shown in the example, i.e.

long oldPosition = -999;

Okay, then I'll power the Arduino with the power supply so I have a common ground.
Dumb questions, since the power supply has multiple outputs for different voltages, should I connect all the ground wires coming from the system into one before plugging it into the power supply or, since the power supply as one final ground, I don't have to worry about it?

NO!

YOU have to declare these variables as volatile.

And what about the power you will get from the USB connection?

Are all the outputs from the power supply independent, or they already comoned up? Disconnect everything fro you power supply and turn it off. Then measure, with your multi meter, if there is continuity between the supply outputs.

1 Like

[quote="Grumpy_Mike, post:13, topic:1320387, full:true"]
NO!
YOU have to declare these variables as volatile.

I see, then the basic example in the Encoder library is incorrect, as the variable should be:

volatile long oldPosition  = -999;

If I understood correctly Arduino prioritizes the current provided through its VIN pin, so the USB is used only to send and receive data.

Good point. I will do that.

No. Grumpy_Mike is incorrect. The library example is correct.

The Encoder library takes care of all the variable declarations that need to be volatile, and returns a copy of the position variable (as done in the last part of my response #7).

2 Likes

Thank you everybody for your suggestions.

For whomever may read this topic in the future looking for a solution, this it my final code:

#include <Motoron.h>
#include <SoftwareSerial.h>
#include <Encoder.h>

Encoder myEnc(2, 3);

long oldPosition  = -999;

SoftwareSerial mySerial(10, 11);  // RX, TX

#define mcSerial mySerial  // Use SoftwareSerial for the Motoron controller

MotoronSerial mc;

const uint32_t assignBaudRate = 115200;

static_assert(assignBaudRate >= MOTORON_MIN_BAUD_RATE &&
  assignBaudRate <= MOTORON_MAX_BAUD_RATE, "Invalid baud rate.");

bool initializeMotoren() {
  mc.reinitialize();
  mc.disableCrc();
  mc.clearResetFlag();
  
  // Verify initialization
  uint16_t errors = mc.getErrorMask();
  return (errors == 0);
}

void setup() {
  // Wait for a second to ensure stable power and connections
  delay(1000);
  mcSerial.begin(assignBaudRate);  // Start communication with motor controller
  Serial.begin(assignBaudRate);

  Serial.println("Starting setup");

  mcSerial.setTimeout(1); //Can be changed to get better values for pos (the lower the more precise)
  mc.setPort(&mcSerial);

  // Initialize the Motoron
  initializeMotoren();

  // Configure the motor (motor 1 only)
  mc.setMaxAcceleration(1, 0);
  mc.setMaxDeceleration(1, 0);

  Serial.println("Setup complete");
}

// Declare the processCommand function
void processCommandStr(String cmd) {
  Serial.println(cmd);
  if (cmd == "F"){
    mc.setSpeed(1, 400);  // Set motor 1 speed to 400
    Serial.println("Motor set to forward");
  } else if (cmd == "R") {
    mc.setSpeed(1, -400);  // Set motor 1 speed to reverse
    Serial.println("Motor set to reverse");
    }
  else if (cmd == "T") {
    Serial.println("Position Reset");
    long newPosition = 0;
    long oldPosition  = -999;
    Serial.print("Pos = ");
    Serial.println(newPosition);
  }
  else if (cmd == "S") {
    mc.setSpeed(1, 0);  // Stop motor 1
    Serial.println("Motor set to stop");
  }
  else {
    Serial.println("Unknown command");
  }
}

void processCommandInt(int speed){
  mc.setSpeed(1, speed);
}

void loop() {
  while (Serial.available() > 0) {
    String input = Serial.readStringUntil('\n');
    input.trim();  // Remove any leading/trailing whitespace
    // Check if the input is an integer or string
    if (input.toInt() != 0 || input == "0") {  // toInt() returns 0 if not a valid integer
      int intInput = input.toInt();
      processCommandInt(intInput);
    } else {
      processCommandStr(input);         // Process the complete command
    }
    // delay(100);
  }

  long newPosition = myEnc.read();
  if (newPosition != oldPosition) {
    oldPosition = newPosition;
    Serial.println(newPosition);
  }

  // delay(10);
}

The main issue was related to the encoder not having a commong ground just like jremington mentioned first and Grumpy_Mike did later. I also added a 0.1µF between the two encoder leads, to lower any additional noise, as mentioned here. And in the future I might add others between each lead and the motor case, always as mentioned in the link. Finally, the Encoder library'e example is correct and it has been implemented in my code.
Good luck on your project!

Problems

The "T" command in my code doesn't work yet, so watch out if you decide to use my code.

1 Like

You meant motor leads, of course.

No. That's what I thought I should have done in the first place, but it didn't work like that.
When placing the capacitor between the two encoder leads I obtain this:


Not perfect, but what I am looking for.
When putting the capacitor between the two motor leads I get:

Which isn't incorrect. But, notice that in my code:

  long newPosition = myEnc.read();
  if (newPosition != oldPosition) {
    oldPosition = newPosition;
    Serial.println(newPosition);
  }

I am printing what I get out of myEnc.read(). Which are just 1s and 0s, but by putting a capacitor across the encoder leads, I obtain values higher than 1. I can modify my code to do:

  if (newPosition != oldPosition) {
    newPosition = newPosition + myEnc.read();
    oldPosition = newPosition;
    Serial.println(newPosition);
  }

where I declared newPosition before the setup. It is fundamental that newPosition addition is inside this loop. But with this method I get 1/4 of the precision I get with the other method.

To be 100% honest I am not sure how the capacitor works in the circuit, what I think happens is that it works as RAM. That is, the Encoder library does +1, if it detects a current, to whatever value it stores, without checking if it's already 1. And since the PIN is still high the value assigned to it just goes higher.

Please correct me if I'm wrong.

Bad idea. That makes no sense at all, regardless of whether it seems to work.

If you insist on stumbling around like this, with no idea about what you are doing, I wish you good luck with your projects.

I didn't say I'm stopping here, in fact I said:

Please correct me if I'm wrong.

and I gave the "correct" solution.
All I know so far is that it works better. I'm trying to figure out why.