Getting the motor encoders to work

Hi @robotbuilder,

POST POSTING EDIT ... Sorry, I take so long writing my replies, others (often Ron @zander), post the same point ... hence you may get a deja vue feeling about the second half of this .. apologies to all for another cross in the Ether. Dave

   Sorry if I overdid any of the explanations .. it is an 'occupational hazard' when you can't see the person you are talking to ... well it is for me anyway.

--------

re: pinMode(2,INPUT); // set as input
digitalWrite(2,HIGH); // enable internal pullup resister

Two concerns I have about this:

1. Microcontrollers do often have internal pull up resistors, which can be optionally enabled, but the value of the resistor is usually quite high ... e.g for the ATmega328P processor which is used in some of the 'classic' Arduinos, such as the UNO, the data sheet

https://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-7810-Automotive-Microcontrollers-ATmega328P_Datasheet.pdf says:

20k - 50k Ohms is fine for holding a pin, with nothing attached to it, high, and something like a push switch maybe ok with short wires, etc., that changes less than once per second.

But things with higher pulse rates, and possibly have extra components and wiring that add intrinisic capacitance, will struggle. Of course, there are situations where the amount of 'struggle' is modest, and it still works, but unless you can either measure the waveforms, which requires an oscilloscope, or accurately determine the pulse rates, capacitances, resistances, etc. of the associated input pin, then this is a potential troublespot. Hence I recommended a much lower value, starting at say 1 kOhm. The downside of a value this low is that the 5 mA current flow when the output is low, draining the battery, but that seems trivial compared to something that doesn't work.

Also note, the comparator circuit you are using relies on a positive feedback resistor, and that feedback resistor will further contribute to the 'load stress' placed on the pull up resistor.

Obviously, when you get it all working with 1 kOhm, then an 'optimisation' phase could try increasing the value, whilst making sure it still works, to find a value that provides both reliable operation and minimum battery current.

2. The syntax you are using looks wrong to me.See https://docs.arduino.cc/learn/microcontrollers/digital-pins

This says:

Prior to Arduino 1.0.1, it was possible to configure the internal pull-ups in the following manner:

digitalWrite(pin, HIGH);       // turn on pullup resistors

My guess is you are using a version later than 1.0.1 ...

The same page also says:

Properties of Pins Configured as INPUT_PULLUP

There are 20K pullup resistors built into the Atmega chip that can be accessed from software. These built-in pullup resistors are accessed by setting the pinMode() as INPUT_PULLUP.

And the example at https://docs.arduino.cc/learn/microcontrollers/digital-pins

includes the following lines in the code example:

  pinMode(2, INPUT_PULLUP);

which I think is the present syntax style.

---------

In brief, I recommend an explicit pull up resistor of say 1k Ohm, and checking the syntax.

----------

As for the mechanical construction, I am not surprised you found a gearbox. Obviously, when trying to help you, I do not have a robot cleaner to disassemble ... so I can only rely on what you tell me, plus the odd guess or nugget of experience ... and we have never had a robot cleaner, disassembled or otherwise...

Obviously, the gearbox could go some way to explaining why you got 700+ pulses per wheel revolution, but it might not be the only problem you are facing. So, if possible, I recommend you try to rotate the sensor wheel to measure the min and max voltages as I described, albeit you may have to do it with the wheel/gearbox partly disassembled to be able to move the sensor wheel in small angle steps.

-------------------

Best wishes, Dave

@davee Being of a certain ancestry, I am frugal with my words (NOT always though) and in many cases intentionally adhere to the idea that the person needing assistance should invest most of the effort. They don't learn much if anything if you give them all the answers. Dave on the other hand often delivers a sermonette response that I know is almost always accurate but may discourage a noob or at least lose them in so many details often not required. Maybe we need chatGPT to reword both our responses, one to elaborate and one to summarize. I might try that on some of these responses for a chuckle.

@robotbuilder @davee I am looking at this problem with noob eyes. I don't remember ever working with wheel encoders before but was aware of them. My primitive understanding was a wheel with slots, a light source, a light detector. Easy peasy right? After a very brief look at some web sites that google found, I see it is waaaaay more complex and they even can use PID's. Again I have no experience with PID's but have seen it discussed here a few times.

Here is my $0.02 worth of gut feel. Maybe you need to 'debounce' the signal much like a push button. I think the capacitor solution is a clue as that is similar to a hardware debounce but incomplete as you likely know.

In looking at the code, it appears incomplete and error prone. The ISR's should have the fast ram attribute (IRAM_ATTR) specified and in all the other examples I have seen the direction must be taken into account in order to inc or dec the count.

Sorry, this is just a quick gut check, I am not in a position at the moment to do a deep dive, but hopefully later though.

EDIT: This link looks interesting, does it help? LINK

EDIT2: How about this link as well LINK

@zander
@davee

Thanks for the responses. I will study the links and take on the suggestions and report back my ultimate successful working solution.

 @davee wrote: Sorry if I overdid any of the explanations .. it is an 'occupational hazard' when you can't see the person you are talking to ... well it is for me anyway.

That is fine if you can be bothered writing it all 🙂

@zander

By all means test out chatGPT's ability to answer a question if you have an account.

Hi @robotbuilder and Ron @zander,

   I don't have any special insight into these encoders, but I agree with Ron's suggestion that all sorts of filters, etc., can be envisaged. In many cases, they would be used to control motors to achieve a certain task, and it makes sense that PID or similar may be appropriate to optimise the speed and rotation  to meet some kind of profile.

However, I always believe in starting simple ... try to get the 'best' quality signals available at the front ... which in this case is the phototransistor and comparator.  Only consider filters, etc. after you are sure the first stage is doing its best.

Hence, I have been suggesting basic things to look at:

1. measure the maximum and minimum voltages produced by the phototransistor and presented at the input of the comparator - my response will depend upon the data - it might be fine or it might need modifying
2. Use a pull up resistor with an appropriate value at the output of the comparator, given that it is an open collector device - the resistors in input pullups on microcontrollers are not intended for this purpose and their resistance is usually too high
3. Add adequate decoupling to the power lines to minimise the chance of the comparator doing weird things, e.g. oscillate.

These are the equivalent to making sure all the wheel nuts on your car are present and appropriately tightened .. . ... tuning up the engine can come later!

Sorry, Ron I should have acknowledged your comment about debounce ... yes, possibly ... but let's start with being sure the signal is as clean as possible .. if the optics, slots, etc are "reasonable", and Schmitt trigger is doing its job, then the classic switch 'contact bounce' should not exist .. but there are lots of 'reasonable' and 'if' assumptions there. Similarly, I know you are better at the software side, but I was leaving the software at this moment since I think it only makes sense when the hardware is doing its best to feed it good data, not rubbish.

Best wishes, Dave

@zander
@davee

So I commented out the internal pullup resistor option and used a 2.2K on the Schmitt output.

pinMode(3,INPUT); // set as input
//digitalWrite(3,HIGH); // enable internal pullup resister
attachInterrupt(digitalPinToInterrupt(3), encoder1, RISING); // interrupt initialization

I also added a 1500uF decoupling capacitor across the battery but that didn't make any difference.

However I have made a discovery! This works perfectly,

countNumber = (number_of_desired_rotations * 800 - 64).

I tried it with different PWM values and the wheel turned exactly the number of times required!

while (counter1 < countNumber){
  Serial.print(counter1);
  Serial.print(" ");
  Serial.println(counter2);
}

Now I have another strange behaviour!

If the while loop doesn't have a Serial.print statement the counter1 value doesn't change? It is as if while looping the interrupt is turned off? The motors just keep turning. I tried other statements but no it wants a Serial.print statement?

while (counter1 < countNumber){
  // Serial.print(counter1);
  // Serial.print(" ");
  // Serial.println(counter2);
}

@robotbuilder That sounds like a volatile problem, in most cases I have seen the variable inside an ISR is often never referenced more than once so the compiler notices that and just ignores it.

Hi @robotbuilder,

  Ron @zander's suggestion sounds plausible and worth investigating. Lots of things have happened since your first listing, so if you are still stuck, perhaps it would be a good time to post a complete listing, if only as an attachment.

-----

As for decoupling, and the possible lack of it, it is one of those things that the requirement of a each circuit seems to change with the weather. Particularly with battery powered equipment, it is not unknown for it to be fine when the battery is new, but fail as the battery ages. Similarly, many commercial designs have worked perfectly for the prototype, then failed when in production when small changes in components between batches, or operation at different environmental temperatures, or .... .

So not seeing any immediate difference is not unusual ... but that does not mean that decoupling is unimportant.

Things to note are:

• One big capacitor cannot solve everything .. large electrolytics are good at dealing with low frequency - long time demands, but rubbish at high frequencies. Similarly, say small ceramics, 0.01 microFarad, are good at dealing with high frequencies, but do not store enough charge for the low frequency - long time time demands. Modern electronic parts work over an increasingly wide range of frequencies from DC upwards, so even if your circuit only intends to deal with low frequencies, the high frequency part of the devices' capability must be catered for. Hence my suggestion of two capacitors in parallel.
• Wire length is also important ... just a few millimetres of wire has enough inductance to mean the effect of capacitors chosen to affect the high frequency end is completely negated. The small value capacitors in particular must be connected with wires as short as possible, and if you have more than 1 chip, then aim for at least one small capacitor per chip. A higher value electrolytic can often cover a 'few' chips which are physically close together on the same board, but only for its 'speciality' of the lower frequency demands.

Best wishes, and well done for your progress so far, Dave

@zander
@davee

The empty while loop issue was a volatile problem and has been resolved by making the counters volatile although I don't know why a Serial.print executed in the loop worked without making them volatile.

volatile int counter1 = 0;
volatile int counter2 = 0;

It also fixed the 64 count thingy so now all you need do is multiply the number of complete wheel rotations desired by 800 and it works perfectly regardless of the PWM chosen or the number of rotations required.

I will clean up the code and post it later. My next task is to have both motors work at the same speed (as determined by their encoders) to have it go in a straight line. Then some fancy dead reckoning path following by continually computing the POSE of the robot relative to its starting POSE (x,y,direction).

@robotbuilder With the Serial.print out of the picture the compiler sees only the initialization statements and no other reference to the counter1 variable. This means the variable does not need to exist. The purpose of volatile is to defeat that compiler optimization.

Also, the now empty while is also removed since counter1 is always 0 it is always < countNumber so that code block is a null statement.

@zander 

Sadly it is playing up again.  I tried to clean up the code that seemed to be working perfectly and it failed. I found I had to again subtract 64 from the total count. I should have saved the code and modified a copy!!

So I have to give it a rest for a while and come back to it fresh at a later date.

I was looking at a motor encoder example in Gordon McComb's book "Arduino robot bonanza" and noticed he didn't use a Schmitt trigger just a comparator, the LM339. So I pulled out the feedback resistor from my Schmitt circuit to make it a simple comparator and the numbers coming from the program below didn't seem to change.

Here is the code again that I am trying to get working.

// Motor A
int enA = 11;
int in1 = 10;
int in2 = 9;
// Motor B
int in3 = 7;
int in4 = 6;
int enB = 5;

volatile int counter1 = 0;
volatile int counter2 = 0;

// button pins assignments
const int btn0 = 4;        // press button purple wire
const int btn1 = 3;        // state of encoder1 green wire
const int btn2 = 2;        // state of encoder2 yellow wire

void forwardA(int rate){
  digitalWrite(in1, HIGH);
  digitalWrite(in2, LOW);
  analogWrite(enA, rate);  
}

void forwardB(int rate){
  digitalWrite(in3, LOW);
  digitalWrite(in4, HIGH);
  analogWrite(enB, rate);  
}

void reverseA(int rate){
  digitalWrite(in1, LOW);
  digitalWrite(in2, HIGH);
  analogWrite(enA, rate);  
}

void reverseB(int rate){
  digitalWrite(in3, HIGH);
  digitalWrite(in4, LOW);
  analogWrite(enB, rate);  
}

void turnOffMotorA(){
  digitalWrite(in1, LOW);
  digitalWrite(in2, LOW);
}

void turnOffMotorB(){
  digitalWrite(in3, LOW);
  digitalWrite(in4, LOW);
}


void setup()
{

  // Set all the motor control pins to outputs
 
  pinMode(enA, OUTPUT);
  pinMode(enB, OUTPUT);
  pinMode(in1, OUTPUT);
  pinMode(in2, OUTPUT);
  pinMode(in3, OUTPUT);
  pinMode(in4, OUTPUT);

   pinMode(btn0,INPUT_PULLUP);

  pinMode(3,INPUT); // set as input
  //digitalWrite(3,HIGH); // enable internal pullup resister
  attachInterrupt(digitalPinToInterrupt(3), encoder1, RISING); // interrupt initialization
  
  pinMode(2,INPUT); // set as input
  //digitalWrite(2,HIGH); // enable internal pullup resister
  attachInterrupt(digitalPinToInterrupt(2), encoder2, RISING); // interrupt initialization
  
  Serial.begin( 9600 );
  //Serial.println("Starting up");
  
}

// interrrupt server routines for reading encoder

void encoder1()
{
  counter1++;
}

void encoder2()
{
  counter2++;
}


void loop()
{

  if (!digitalRead(btn0)){   // if button is pressed start motors

    counter1 = 0;
    counter2 = 0;

    forwardA(100);
    forwardB(100);

    while (counter1 < (8000-64)){  // 10 rotations
      Serial.print("counter1 =");
      Serial.print(counter1);
      Serial.print("  counter2 =");
      Serial.println(counter2);
    }

    Serial.println("======");
    Serial.print(counter1);
    Serial.print("  ");
    Serial.println(counter2);

    turnOffMotorA();
    turnOffMotorB();

  }
}

@robotbuilder I am reluctant to venture an opinion beyond what I already have because I don't understand what you are trying to do. That sketch looks nothing like what I have seen elsewhere.

@zander 

It is very simple what I am doing. I am counting the pulses from encoder1 attached to one of the wheels until it reaches a value that results in a wheel doing a complete rotation. I press a button. The motors are started and the encoder counters are set to zero. When a count value for counter1 is reached it drops out of the while loop, prints some data and waits again for another button press. I visually check to see if the wheel has returned to the exact same position which it does if the number selected is correct.

So if I want the robot base to move x inches I just convert that measure into a count value. There is 800 counts per rotation. One rotation equals about 204cm travel. I don't see anything hard to understand with regards to the code. It works fine. What may not be working properly is capturing all the pulses due to noise or some other factor yet to be determined. I have had the wheel turn and finish at its exact starting point.

The compiler may well be messing with my code without me knowing but the instructions are clear to me if not messed with.

To enlarge an image, right click image and choose Open link in new window.

@robotbuilder If the code is what you posted 2 posts back, it's missing the fast ram macro IRAM_ATTR and I think with so much Serial in the loop I would wrap the while in a critical section wrapper. HOWEVER, there is a good chance that the guts of the while take longer to execute than the interrupts are happening and then the critical section will cause dropped counts. I would still try that so I would know for sure. Maybe try a second test with just one motor to reduce the while guts overhead.

NOTE: I am unsure about the possibility of the Serial operations disrupting the ISR, if that is a possibility, then I would change the ISR to setting a bool and leave the counter++ to the main loop. There may be an ISR critical section macro as well, I have not looked into how that works but IIRC the setting of a bool flag is atomic and accomplishes the same thing.

I would like to set up the experiment here but I don't think I understand your hardware. Can you replicate it using a motor and driver like Bill used? If not, then I have no way of knowing if it is a hardware or software issue.

Would the following motor work as a test case? https://wiki.dfrobot.com/Micro_DC_Motor_with_Encoder-SJ02_SKU__FIT0458

Posted by: @zander

↑

NOTE: I am unsure about the possibility of the Serial operations disrupting the ISR

I think its the other way round, the ISR could cause the program to pause while it services the isr and thus the Serial ops.  (though in this case just incrementing a counter should not be an issue.. probably)

@robotbuilder

But, could it be the time to find your rpi pico and to consider using its pio which can do operations independent of the cpu, so it can be used as your encoder counter.   And as the pio is programmed in assembler it may be right up your ally. 

I link to some sample programs (C++ and assembler) to see if your taste buds are tickled. 😀 

https://github.com/GitJer/Rotary_encoder