control and store a value from an led that can be used in an if statement.

hello I have been trying to control a servo via a potentiometer but using an LED as middle tier control element. my idea was to create an if statement that turned the led on when the potentiometer went above a certain value and off when below a certain value. I then thought I’d write an if statement that used the state of the led to control the servo. I know there are more direct ways of facilitating this kind of control but this would be useful for me in other projects and I am also curious.

The result is that the potentiometer controls the led and ledstate is printed as 0 or 1 accordingly but this fails to move the servo ?

here is my code

#include <Servo.h> 
 
Servo myservo;  // create servo object to control a servo 
           // a maximum of eight servo objects can be created 
 
int pos ; // a value for the servo
int led = 13; // the led
int sensorValue; // the potentiometer 


 
void setup() 
{ Serial.begin(9600);
  myservo.attach(6);  // attaches the servo on pin 6 to the servo object 
  pinMode (led, INPUT );
  pinMode (led, OUTPUT );
} 


 void loop() {
 sensorValue =analogRead(A0);

   
 
 
 
 
 if (sensorValue < 50) {
   digitalWrite(led, HIGH);
 }
 if (sensorValue > 50) {
    digitalWrite(led, LOW); // if statments controlling the led via the potentiometer
 }
 
 int ledstate = digitalRead(led);
 Serial.println (ledstate); // reading and printing the led state 
 
 if (ledstate =1) {
   pos= 120;
   myservo.write (pos);
   delay (100);
 }
 
  if (ledstate = 0) {
   pos= 60;
   myservo.write (pos);
   delay (100); // attempting to use the led state to control my servo
 }
 }

I first tried just if ( 13==HIGH) and the opposite but this did not work.
I feel this is simple and not something that I should not be able to do. is the problem something to do with defining led as OUTPUT and INPUT.
any help would be much appreciated
cheers
sam

 if (ledstate =1) { // should be == so it is a comparison rather than an assignment, and it would be better to use HIGH rather than 1
   pos= 120;
   myservo.write (pos);
   delay (100);
 }

  if (ledstate = 0) { // this should be an 'else' since we know the state will be either high or low
   pos= 60;
   myservo.write (pos);
   delay (100); // attempting to use the led state to control my servo
 }

thanks peter I made the changes and it now works fine

cheers sam

Whilst it seems to work OK it has always seemed wrong somehow to read the state of an output pin to determine its current state. After all, it is the program that puts it into that state so you should know what it is at all times.

My preferred method of working would be to set the ledState variable HIGH or LOW as appropriate and then to write ledState to the output pin, rather than the other way round. This usually means that writing to the LED can be done after the test of inputs/whatever and setting of ledState to suit, thus making maintenance easier because the digitalWrite() command is only in the code in one place.

Taking things further, the pos variable could be set at the same time as ledState and used later to move the servo.

In the case of this program we are only concerned whether the reading from A0 is above or below 50 so can cut out some if tests. I note too that the original program had no code for when the pos value was exactly equal to 50.

Some of the variables can be defined as const as their value will not be changed and others need only a byte instead of an int which will save RAM.

Incorporating these ideas into a program I came up with

#include <Servo.h> 
Servo myservo;

byte pos;
const byte led = 13; 
int sensorValue; 
byte ledState;

void setup() 
{ 
  Serial.begin(115200);
  myservo.attach(6);
  pinMode (led, OUTPUT );
} 

void loop() 
{
  sensorValue =analogRead(A0);
  if (sensorValue < 50) 
  {
    ledState = HIGH;
    pos= 120;
  }
  else
  {
    ledState = LOW;
    pos= 60;
  }
  digitalWrite(led, ledState);
  myservo.write(pos);
  delay (100);
}

Oh, and I also reformatted the code to my preferred layout (all braces on their own line) and used Auto Format