all leds on, servo needs to work

Hello guys,

I’m very new to this but I’m working on a project for school, but i can’t make it work.
When all the leds are on (2 t/m 7), I want the servo to work and sweep properly. And when not all the leds are on, the servo stops sweeping.

In my code i get the servo to work, only when all the leds are on but it only works 1 time.
When the servo makes a turn all the leds turn off. I think it has something to do with the
voltage to the servo but im not sure.

I hope someone can help me with this.
This is my code till now:

#include <Servo.h>                                            //library voor servo 

Servo myservo;                                                //om de servo te besturen

int lichtWaarde;                                              //variabele om lichtwaarde te kunnen meten van lichtsensor
int lichtPin = A0;                                            //lichtpin krijgt naam zodat deze nog makkelijk aan te sluiten is op een andere poort
int ledON;
int ledSTART = 2;                                             //pin waar de eerste led begint
int ledtotaal = 6;                                            //aantal ledjes die aangesloten zijn


void setup() {
  Serial.begin(9600);                                         //leest waardes
  myservo.attach(8);                                          //servo op pin 8

  for (int i = ledSTART; i < ledSTART + ledtotaal; i++)       //led van pin 2 t/m pin 7 aan
    pinMode(i, OUTPUT);
}

void loop() {
  lichtWaarde = analogRead(lichtPin);                         //om de waardes van de pin uit te lezen
  delay(100);
  Serial.println(lichtWaarde);                                //Lichtwaarde is te zien in seriele monitor


  ledON = map(lichtWaarde, 1016, 970, 2, 7);                  //om ledjes aan te zetten eerst mappen naar goede waardes. De waardes worden verdeeld over 6 ledjes: 1023 =1016, 0=950
  for (int i = 2; i < ledON; i++) {
    digitalWrite(i, HIGH);
    delay(15);
  }

  for (int i = ledON; i < 8; i++) {                           //for loop om ledjes weer uit te zetten.
    digitalWrite(i, LOW);
  }

  if (ledON >= 8) {
    myservo.attach(8);
    myservo.write(0);                                         //positie servo op 0 graden
    delay(100);

    for (int i = 0; i <= 90; i += 1) {
      myservo.write(i);
      delay(15);
    }
    for (int i = 90; i >= 180; i -= 1) {
      myservo.write(i);
      delay(15);
    }

  } else if (ledON < 8) {
    myservo.detach();
  }
}

The variable ledON in this code

 for (int i = ledON; i < 8; i++) {                           //for loop om ledjes weer uit te zetten.
    digitalWrite(i, LOW);
  }

is a different variable from the one of the same name used elsewhere. That is because you preface it with int. If you want it to be the same variable you need to omit the int.

EDIT to say PLEASE take no notice of the preceding comments. I completely misread the OP’s code. Apologies to all

However I’m not sure if that will solve the problem because there seems to be no other part of your code that changes the value of leON so, after the FOR loop it will always be 7

…R

Robin2: However I'm not sure if that will solve the problem because there seems to be no other part of your code that changes the value of leON so, after the FOR loop it will always be 7

What do you mean it will always be 7?

I tried to remove the 'int' but when I do, I get an error

I think Robin2 mis-read the code a little in this case.

The "int" is necessary to define your "i" temporary variable, so you can leave it in.

But, you have this code:

  ledON = map(lichtWaarde, 1016, 970, 2, 7);                  //om ledjes aan te zetten eerst mappen naar goede waardes. De waardes worden verdeeld over 6 ledjes: 1023 =1016, 0=950

....


  if (ledON >= 8) {
....
}

The "map" function changes values between 970 and 1016 to values between 2 and 7. So, the only way ledON >= 8 is if you enter values outside that range. Is that what you intended?

arduinodlb: I think Robin2 mis-read the code a little in this case.

More than a little. Thanks for pointing it out. I have edited my earlier post.

...R

I don't understand what this is supposed to do?

for (int i = 90; i >= 180; i -= 1) {
      myservo.write(i);
      delay(15);
    }

"i" will always be less than 180 so the loop will never execute.

DuaneDegn: I don't understand what this is supposed to do?

for (int i = 90; i >= 180; i -= 1) {
      myservo.write(i);
      delay(15);
    }

"i" will always be less than 180 so the loop will never execute.

It will execute when "i" rolls over and becomes positive again.

aarg:
It will execute when “i” rolls over and becomes positive again.

The loop will immediately exit so “i” never rolls over.

I really have to stop doing all the other things I'm doing as well as hanging around on this forum. :)

aarg: I really have to stop doing all the other things I'm doing as well as hanging around on this forum. :)

I really need to start doing all the other things I should be doing instead of hanging around on this forum. :)

arduinodlb,

Must I divide the values 970 - 1016 over 2-8 instead of 7?

When I do this, the servo works 1 time, but then al the leds go off. Do you know something I can do so it doesnt happen?

leukfevj:
When I do this, the servo works 1 time, but then al the leds go off. Do you know something I can do so it doesnt happen?

You need to post your latest code so we can see what you are talking about.

…R

ledON = map(lichtWaarde, 1016, 970, 2, 8);
for (int i = 2; i < ledON; i++) {
digitalWrite(i, HIGH);
delay(15);
}

I only changed ‘map’, 1016,970, 2, 7 to 1016, 970, 2, 8

leukfevj: I only changed 'map', 1016,970, 2, 7 to 1016, 970, 2, 8

I think you'll have a better chance of getting help if you post code we can copy and paste to the IDE. I know I'm personally much too lazy to find and replace code as you suggest.

DuaneDegn: I don't understand what this is supposed to do?

for (int i = 90; i >= 180; i -= 1) {
      myservo.write(i);
      delay(15);
    }

"i" will always be less than 180 so the loop will never execute.

As I mentioned above, this for loop doesn't loop.

I changed my code & the loop. But now my servo goes insane every time it gets power. It shakes but I don’t do anything.

#include <Servo.h>                                           

Servo myservo;                                               

int lichtWaarde;                                              
int lichtPin = A0;                                            
int ledON;
int ledSTART = 2;                                            
int ledtotaal = 6;                                            


void setup() {
  Serial.begin(9600);                                         
  myservo.attach(8);                                         

  for (int i = ledSTART; i < ledSTART + ledtotaal; i++)      
    pinMode(i, OUTPUT);
}

void loop() {
  lichtWaarde = analogRead(lichtPin);                        
  delay(100);
  Serial.println(lichtWaarde);                                


  ledON = map(lichtWaarde, 1016, 970, 2, 8);                 
  for (int i = 2; i < ledON; i++) {
    digitalWrite(i, HIGH);
    delay(15);
  }

  for (int i = ledON; i < 8; i++) {                   .
    digitalWrite(i, LOW);
  }

  if (ledON >= 8) {
    myservo.attach(8);
    myservo.write(0);                                       
    delay(100);

    for (int i = 0; i <= 180; i += 1) {
      myservo.write(i);
      delay(15);
    }


  } else if (ledON < 8) {
    myservo.detach();
  }
}

But as I already said, i’m not good at this and i’m stuck here. I don’t even know if its possible what I want

leukfevj:
ledON = map(lichtWaarde, 1016, 970, 2, 8);

You need to always use code tags when posting code, like you did in the first post, even if it’s a short snippet and not a complete sketch. In this code, the 8 followed by a closing parenthesis was translated to a sunglasses smiley face by the forum formatting code. If the code were inside of code tags, that translation wouldn’t happen and the code could be read properly.

I don’t think your problem is the map function. The original version should properly return values up to 7, which is what you need for the LED portion of the code to work properly. The issue is the comparison which checks for a value of greater than or equal to 8, it should be comparing to 7 if you want it to allow the servo to move as soon as the last LED is lit.

Oops, I just re-read your code, and I see you are turning on LEDs up to, but not including, ledON. So yes, if you want that last LED to light, I think you need to run your map function up to 8.

The other issue is the second servo loop, the one that counts down. As DuaneDegn correctly points out, it will start with a value of 90, immediately compare that to the termination condition (i >= 180) and immediately exit the loop without doing anything. You are using an int for the loop control, not an unsigned value, so it is OK to let the loop control variable go negative, it won’t wrap around once you go past zero.

for (int i=90; i<=0; i--)

That for loop should work as you expect. i starts out at 90, and will be constantly decremented as long as the value is zero or above. After the last pass (when i is zero) it will be decremented to -1, which is less than zero, and the loop will drop out.

Those two changes (compare ledON >= 7 change the map function, and change the second loop) should get you working properly. But there are additional optimizations and cleanup you can do to make the code easier to read and maintain.

Near the bottom of your sketch, you have an else if clause. Cutting out the body statements of the if/else if, you get this:

if (ledON >= 8) {
  ...

  } else if (ledON < 8) {
    ...
  }

The else if clause is redundant. Either ledON is greater or equal to 8, or it is less than 8. There are no other possibilities. You can use a simple else clause there and get the same effect, with a little less code to write and execute:

if (ledON >= 8) {
  ...

  } else {
    ...
  }

The big advantage there is if you ever change the 8 in the first if to something else, you don’t have to make a similar change to the second if. With both if statements there, if you change one, you have to make sure you make the same change to both.

You can also simplify the code that controls the LEDs. You currently have a loop that goes from 2 through ledON that turns on LEDs, and another that goes from ledON to 7 to turn off LEDs. They can easily be combined:

 for (int i = 2; i < 8; i++) {
    if (i < ledON)
       digitalWrite(i, HIGH);
    else
      digitalWrite(i, LOW);
  }

This will always loop through all LEDs at once, turning on the ones that are below ledON, and turning off the ones that are at or above ledON. I think it’s easier to read.

I’m not sure what you’re trying to accomplish with the delay() you had in there. With a larger delay, it could cause the lights to turn on gradually when you make a big increase in the input value, but with such a short 15 ms delay, I don’t think you will be able to see anything. If you had a different purpose for the delay, let us know.

You could also eliminate the second call to digitalWrite:

 for (int i = 2; i < 8; i++)
       digitalWrite(i, (i < ledON) ? HIGH : LOW);

But this brings in the ternary conditional operator, which some people find obtuse and hard to follow. But it can also be eliminated because the result of the less than comparison operator will be either 1 if it’s true, or 0 if it’s false. HIGH and LOW are defined similarly: HIGH is 1, and LOW is 0. So this code will have exactly the same effect:

 for (int i = 2; i < 8; i++)
       digitalWrite(i, (i < ledON));

The extra parenthesis around the less than expression in the last two examples are not necessary, but I think it helps readability.

The last suggestion for this loop is that having the hard coded numbers 2 and 8 are not a good idea. If you ever change the number of LEDs you are using, you will have to make a lot of changes all over the code. You’ve already defined variables to hold the first LED number, and the number of LEDs, you should use them in this loop:

 for (int i = ledSTART ; i < ledSTART + ledtotaal ; i++)
       digitalWrite(i, (i < ledON));

Your map function should also use these LED number definitions instead of hard coded values.

Finally, you define ledSTART, ledtotaal, and lichtpin as variables. These really should be constants to prevent you from accidentally changing them. All you need to do is add the "const " keyword in from of the int type.

You should probably also define a constant for the servo pin, so all of the pin definitions are up there at the top of the code. Then, the myservo.attach() function should reference that constant rather than using a hard coded value.

leukfevj:
I changed my code & the loop. But now my servo goes insane every time it gets power. It shakes but I don’t do anything.

That is rather common when a servo has power, but is not getting a signal on the control line. It picks up some random noise, and thinks its a command, so it moves to that position. The next noise pulse that comes in will likely be a different length, so it tries to move to that position. The net result is that it jitters, shakes, and oscillates all over the place.

The solution is to make sure that the servo always gets a signal. Attach to the servo in setup(), and set an initial position. Then, don’t re-attach to the servo down in loop(), and never detach from the servo. Although I have controlled lots of servos using other processors, setting up the timer PWM outputs directly, I’ve not use the Servo library on an Arduino. So I may be missing some fine point of the Servo library, but I think these recommendations will help you.

All of the above was written against your initial code that you posted. Just as I finished typing this, you made another post. The comments above do not necessarily apply to your latest code. I’m going to look at that next.

But as I already said, i’m not good at this and i’m stuck here. I don’t even know if its possible what I want

It is most certainly possible! And I think you are getting very close to making it work.

Well, ShapeShifter, it already works much better! Thank you very much!

The leds still go out when the servo makes a turn but then after a second the leds go on again, instead of off all the time.

But, I dont understand this part of the servo & what you said.

The solution is to make sure that the servo always gets a signal. Attach to the servo in setup(), and set an initial position. Then, don't re-attach to the servo down in loop(), and never detach from the servo. Although I have controlled lots of servos using other processors, setting up the timer PWM outputs directly, I've not use the Servo library on an Arduino. So I may be missing some fine point of the Servo library, but I think these recommendations will help you.

Must I put an 'int' before myservo.attach() ?

leukfevj: The leds still go out when the servo makes a turn but then after a second the leds go on again, instead of off all the time.

Please post your current code. Someone can suggest that you make a change, and you can say that you made the change, but that doesn't mean the change was made correctly. The only way to know is to look at what you have now - saying what you changed (and you didn't even do that) isn't enough.

I can only make guesses at this point. Let us see the code and you can get some real answers.

But, I dont understand this part of the servo & what you said.

Must I put an 'int' before myservo.attach() ?

No! If you change it to:

int myservo.attach(8);

the 'int' in front will turn the statement calling attach() into a function declaration, and that won't compile:

  • You can't declare a function inside another function
  • You can't declare a function for a class (myservo) outside of the class declaration
  • You can't declare a function that has a numeric value as a parameter

What I'm saying is that the myservo.attach(8); call should be in setup(), where it is done once at the beginning of execution. You have that, and that's good. But you should also give the servo an initial position, by calling myservo.write(0); immediately after attaching to the servo (assuming zero is your desired initial position.)

Then, it should stay attached forever: do not call myservo.detach() in loop(). Doing so will stop the computer from sending position commands to the servo, and that can cause the random chattering that you are reporting. Now, if you never detach from the servo in loop(), and you attached in setup(), then you are always attached to the servo, and you do not have to call attach() inside of loop. So get rid of the attach() and detach() calls in loop().