Why? Arduino Servo. Pot. LED

[metrologist]

Arduino sets the servo position via a pot. The LED lights when the servo function moves the servo to a new position.

When I physically twist the servo, the LED comes on and I don't know why?

My loop just calls these two functions. The first reads a pot pin 5 times and maps an average to degrees. The second moves the servo.

Code: [Select]

void pot3(){ 
  pot = 0;
  for (int i=1; i <= 5; i++){
    pot = pot + analogRead(potpin);
  }
  pot = map(pot/5, 0, 1023, 0, 180);
}

void servo1x(int pos, int dlyx) {
  if (pos < pos1)
  {
    digitalWrite(13, HIGH);  // turn the LED on (HIGH is the voltage level)
    for(int i = pos1; i >= pos && i >= servo1start; i--)    // goes from degrees to degrees
    {                               
      servo1.write(i);              // tell servo to go to position in variable 'pos'
      delay(dlyx);                      // waits ms for the servo to reach the position
      pos1 = i;
    }
    digitalWrite(13, LOW);  // turn the LED off (LOW is the voltage level)
  }
  else if (pos > pos1) {
    digitalWrite(13, HIGH);  // turn the LED on (HIGH is the voltage level)
    for(int i = pos1; i <= pos && i <= servo1stop; i++) // goes from degrees to degrees
    {                                  // in steps of 1 degree
      servo1.write(i);              // tell servo to go to position in variable 'pos'
      delay(dlyx);                      // waits ms for the servo to reach the position
      pos1 = i;
    }
    digitalWrite(13, LOW);  // turn the LED off (LOW is the voltage level)
  }
}

Just because I apply pressure to the servo there is no way Arduino can know about it, so why does LED come on? It is very consistent with pressure too. It goes from a flicker to strong brightness with more pressure.

Also, at the endpoints of my pot, the servo is not stable and twitches constantly.

Finally, I have an LCD function that will print the current angle. If I include a call to that function, none of the above happens and the servo takes consistent coarse steps to any new position.

it's more complex than it needs to be and works fine, but maybe takes a bit of time...?

Code: [Select]

void  lcd2(){
  // picture loop
  u8g.firstPage(); 
  do {
    draw2();
  }
  while( u8g.nextPage() );
  xPos = xPos + 14;
  if (xPos >= 128) xPos = 0;
  if (++yPos >= 76) yPos = 0;
}


void draw2() {
  //graphic commands to redraw the complete screen should be placed here 
  //u8g.setFontPosTop();
  //u8g.setFont(u8g_font_unifontr);
  u8g.setFont(u8g_font_fub11); //fub11, 14, 17, 20, 25
  u8g.setPrintPos(0, 12);
  u8g.print(pot);
  u8g.print("deg ");
  u8g.drawStr(xPos, 62, "."); //scrolling marq2uee
}


[Simon]

Does the servo not have any feedback ? like a built in pot that the arduino reads to see it's position? or is the servo a self contained closed loop one?

[metrologist]

It is a regular servo, with PWM input pin and two power pins. Arduino just sets PWM on a pin and there is no read function. The servo has a pot to know where it is and some circuit to read that and move the motor to the PWM position.

I take it back, if I include LCD function. I just have to twist servo a lot. AND, the degrees change on my LCD as I fight the servo...

[ebastler]

The servo has an internal feedback loop, of course. So as you (try to) turn it manually, it will sense that and drive its motor, to try and stay at the position which the PWM signal from the Arduino requests. I'm wondering whether you might be dealing with a hardware issue here? Spikes from the motor current messing with the local ground level, and hence causing the LED to flicker?

[metrologist]

Oh, I think it's simple. The servo is powered from Arduino and loads the supply, changes the ADC reference and thinks the pot moved.

[ebastler]

Oh, I think it's simple. The servo is powered from Arduino and loads the supply, changes the ADC reference and thinks the pot moved.

Are you saying you just realized this, or knew it all along and posted this as a little quiz? 

[Simon]

do you have a multimeter or better scope to verify. If you force the servo you will effectively make it's motor work at full torque / stall current and yes the common power supply could go all over the place. But the pot is on the same supply is it not? but granted the timing of the program could cause such a thing or even a crash and reboot of the chip.

[sokoloff]

Couple of code tips that might be relevant.

You can use the map function to do the smoothing rather than doing the division yourself. Doing it this way means only one integer truncation rather than two, leading to generally smoother results.

Code: [Select]

void pot3() { 
  pot = 0;
  for (int i=1; i <= 5; i++){
    pot = pot + analogRead(potpin);
  }
  pot = map(pot, 0, 1023*5, 0, 180);
}

I presume pos1 is a global variable. If it's only used in servo1x, you could make it a static variable in that function.
You are also commanding the servo to its (presumed) existing position as the first step in every loop.
To fix that and to reduce code duplication (which tends to allow bugs to hide), I'd cleanup the servo1x function as follows:

Code: [Select]

void servo1x(int pos, int dlyx) {
  if (pos < servo1start)            // These two ifs may prevent the flickering
    pos = servo1start;              // at the pot extremes, if the issue is
  if (pos > servo1stop)             // that you were commanding to a position
    pos = servo1stop;     // outside the servo1 limits.
  if (pos == pos1)                  // Nothing to do
    return;
  int step = (pos < pos1) ? -1 : 1; // determine which direction to go
  digitalWrite(13, HIGH);           // turn the LED on (HIGH is the voltage level)
  for ( bool skipfirst = true; pos1 >= servo1start && pos1 <= servo1stop; pos1 += step)    // goes from degrees to degrees
    {
      if (skipfirst) {             
skipfirst = false;          // Skip first position command and associated delay
continue;
      }
      servo1.write(pos1);           // tell servo to go towards target position in variable 'pos'
      delay(dlyx);                  // waits ms for the servo to reach the position
      if (pos1 == pos)              // when reaching target position, break out of the loop
        break;
    }
  digitalWrite(13, LOW);            // turn the LED off (LOW is the voltage level)
}

Note that if you do this, you will need to ensure there is an initialization of pos1 that causes the variable to be within the range [servo1start, servo1stop] and puts the servo there.

[sokoloff]

You can handle initialization in one of two ways:

Code: [Select]

void servo1x(int pos, int dlyx) {
  if (pos1 < servo1start)            // These two ifs allow for self-initialization
    pos1 = servo1start;
  if (pos1 > servo1stop)
    pos1 = servo1stop;
  if (pos < servo1start)            // These two ifs may prevent the flickering
    pos = servo1start;              // at the pot extremes, if the issue is
  if (pos > servo1stop)             // that you were commanding to a position
    pos = servo1stop;     // outside the servo1 limits.
  if (pos == pos1)                  // Nothing to do
    return;
  int step = (pos < pos1) ? -1 : 1; // determine which direction to go
  digitalWrite(13, HIGH);           // turn the LED on (HIGH is the voltage level)
  for ( bool skipfirst = true; pos1 >= servo1start && pos1 <= servo1stop; pos1 += step)    // goes from degrees to degrees
    {
      if (skipfirst) {             
skipfirst = false;          // Skip first position command and associated delay
continue;
      }
      servo1.write(pos1);           // tell servo to go towards target position in variable 'pos'
      delay(dlyx);                  // waits ms for the servo to reach the position
      if (pos1 == pos)              // when reaching target position, break out of the loop
        break;
    }
  digitalWrite(13, LOW);            // turn the LED off (LOW is the voltage level)
}
and make a call to servo1x(90, dlyx) or servo1x(180, dlyx), servo1x(0, dlyx) to initialize the servo to a "known" position prior to having it follow the pot.

Alternately, you could write a servo init function like this and call it once before looping.

Code: [Select]

void servo1x_init(int dlyx) {
  if (pos1 < servo1start)            // These two ifs ensure pos1 is initialized in range
    pos1 = servo1start;
  if (pos1 > servo1stop)
    pos1 = servo1stop;
  servo1x(180, dlyx);      // and command the servo to one extreme
  servo1x(90, dlyx);      // then the middle
}


[sokoloff]

(If you do the second method to initialize, you don't need the two pos1 ifs in the servo1x function and can use the one from my first post.)

Note that my code is not only not tested, I didn't even check to see if it compiles. I looked it over visually and simulated it in my head, but like everything that comes out of my fingers, it's subject to errors. 

[sokoloff]

You can also use the Arduino macro constrain for improved readability:
Replace the first two ifs in code sample 3 with

Code: [Select]

  pos1 = constrain(pos1, servo1start, servo1stop);

And the two "pos" ifs in all code samples with:

Code: [Select]

  pos  = constrain(pos , servo1start, servo1stop);


[metrologist]

I don't see where I am commanding the servo in every loop as the condition has to be greater or less than, so if the pot does not change, then both of the if conditions should be bypassed and the function exits.

But I do have a problem with correlating position degrees and dac value, so I changes the function.

Code: [Select]

void pot3(){ 
  for (int i=1; i <= 5; i++){
    pot = pot + map(analogRead(potpin), 0, 1023, 0, 180);
  }
  pot = pot/6;
}

Now the endpoints only flicker the LED when the pot is beyond the limit setting, which are are due to not every servo can actually move to 0 and 180 degrees without binding.[/code]

[metrologist]

Oh, I think it's simple. The servo is powered from Arduino and loads the supply, changes the ADC reference and thinks the pot moved.

Are you saying you just realized this, or knew it all along and posted this as a little quiz? 

I think there is no good answer to this kind of question, but I wouldn't attempt to quiz the forum.

[ebastler]

Are you saying you just realized this, or knew it all along and posted this as a little quiz? 

I think there is no good answer to this kind of question, but I wouldn't attempt to quiz the forum.

Hmm, I indeed think there would have been a "good answer".

You asked a question to the forum, I suggested an answer, you (instead of saying "thanks") responded "Oh, I think it's simple". Which makes me wonder whether you were feeling the need to explain something to me, namely the solution to a little quiz you had devised.

The "good answer" would have been: "No, I was really at a loss, but this may indeed be the root cause. Thank you."

[sokoloff]

I don't see where I am commanding the servo in every loop as the condition has to be greater or less than, so if the pot does not change, then both of the if conditions should be bypassed and the function exits.

What I mean is that, when you do have a command to issue, you always issue the current position [needlessly].

Say pos1 = 10 and pos = 15.

Your original code commands 10, 11, 12, 13, 14, 15, with a delay at each comma position.
Presumably, you could command 11, 12, 13, 14, 15, with a delay at each comma.

But I do have a problem with correlating position degrees and dac value, so I changes the function.

Code: [Select]

void pot3(){ 
  for (int i=1; i <= 5; i++){
    pot = pot + map(analogRead(potpin), 0, 1023, 0, 180);
  }
  pot = pot/6;
}


Here, since you're not initializing pot to 0 before accumulating, you're mixing in the effect of a low pass filter (because you're adding 5 times the new measure and then dividing by 6) and thereby smoothing (taking 1 part the old pot angle and 5 parts the new).
If you want it work that way, this code is probably clearer and more cleanly tweakable by changing the DEFINE'd values:

Code: [Select]

#define NUM_ADC_READS 5
// Higher Smoothing factors are, well, smoother. Range is 0 to 127
#define SMOOTHING_FACTOR  21
void pot3() {
  int newPot = 0;
  for (int i=1; i <= NUM_ADC_READS; i++){
    newPot = newPot + analogRead(potpin);
  }
  newPot = map(newPot, 0, 1023*NUM_ADC_READS, 0, 180);

  // Now smooth the pot reading by mixing with the old reading.
  pot = (SMOOTHING_FACTOR*pot + (128-SMOOTHING_FACTOR)*newPot) / 128;
}It also avoids doing a division and associated truncation (in the map function) so many times.
21 is approximately the same amount of low pass filtering that your code immediately above does.

[sokoloff]

Another approach, designed to keep the latency through the loop() function constant is to move a maximum of a fixed number of steps in the servo positioning function:

Code: [Select]

//#define USE_HYSTERESIS
#define HYSTERESIS_X 1
#define MAX_STEP_X 1

void servo1x(int pos) {
#ifdef USE_HYSTERESIS
  if (constrain(pos, pos1-HYSTERESIS_X, pos1+HYSTERESIS_X) == pos) {
    digitalWrite(13, LOW);
    return;
  }
#endif
  pos  = constrain(pos, servo1start, servo1stop);  // Constrain to servo limits
  if (pos == pos1) {             // Nothing to do but turn off the LED
    digitalWrite(13, LOW);
    return;
  }
  pos1 = constrain(pos, pos1-MAX_STEP_X, pos1+MAX_STEP_X); // Constrain to max of MAX_STEP_X away from current position.
  digitalWrite(13, HIGH);           // turn the LED on (HIGH is the voltage level)
  servo1.write(pos1);             // tell servo to move towards target position passed in variable 'pos'
}
And move the delay into the loop function:

Code: [Select]

void loop() {
  // Do whatever you're doing now

  // Read the pot

  // Move Servo X
  servo1x(pot);

  // Update LCD

  // Whatever else
 
  // Add a delay *only* once in loop:
  delay(dlyx);
}
This makes the system more responsive when the servo has a long commanded move. It also allows "diagonal movement" for a system with multiple servos. (Basically, the X motion happens continuously but only one "step" at a time each time through the loop and the LCD keeps updating, the pot is still being read, if there are other servos, they can also be moving at the same time, etc.)

The hysteresis processing code is designed to reject a small amount of noise in the pot measurement (at the expense of final accuracy). It's optional and not compiled in in the sample code above.

[metrologist]

Are you saying you just realized this, or knew it all along and posted this as a little quiz? 

I think there is no good answer to this kind of question, but I wouldn't attempt to quiz the forum.

Hmm, I indeed think there would have been a "good answer".

You asked a question to the forum, I suggested an answer, you (instead of saying "thanks") responded "Oh, I think it's simple". Which makes me wonder whether you were feeling the need to explain something to me, namely the solution to a little quiz you had devised.

The "good answer" would have been: "No, I was really at a loss, but this may indeed be the root cause. Thank you."

Oh, sorry. I don't seem to be much good at anything. It was very much conversational and unfolding as we spoke, so to say... OK, yes, I just realized it as I posted, or as you posted. I guess an engineer would have known as readily as one would know to put on their pants.

Spikes on the ground line was not what I was thinking, but thanks. That might actually be what is going on rather than a weak sagging regulator, but I don't know. I was just perplexed there for a sec as I twisted the servo and the dac reading on the LCD responded proportionately along with the LED.

[metrologist]

Another approach, designed to keep the latency through the loop() function constant is to move a maximum of a fixed number of steps in the servo positioning function:

OK, that's interesting and I'll have to digest it and see how it works. I know my coding style is still very simpleton. This code was pieced together as a quick esc test. The servo function had a delay to control the servo speed. It was for a robot that all worked in tandem, but the function is blocking so nothing else works well with it.

I've been trying to think of the micro as spinning very fast and looking for a way to trigger shorter commands, such as every second check thermistor, every n seconds move servo one step, only write the part of the LCD when it changes, and put a timer on the marching time dot so it works with a RTC, etc. It seems you could have the effect of multitasking.

I ordered 20 servos for another 4 leg robot project so there will be time to explore this soon.

[sokoloff]

OK, that's interesting and I'll have to digest it and see how it works. I know my coding style is still very simple.

"Simple and working" or "simple and making progress" beats "complicated and not working" or "never written" every day of the week, so don't sweat it a bit.

Sorry for so many followup posts above, but I first focused on the micro problem (no pun intended) and only later thought "hey, if he's calling it 'servo-x', that probably means there's another servo in the mix and he probably doesn't want blocking movement"

[metrologist]

Well, your thought pretty much is spot on...

I called the function servo1x because servo1 caused an error that took me forever to discover. I already declared servo1 as the servo object.

This was actually a two-servo robot and the servos worked in sequence rather than tandem like I was thinking.