getting wrong numbers

hi, for some reason the last analog pin doesn’t do anything, it supposed to be a park that shows the closest parking spot, i tried to connect it with less parks but always the last park isn’t working,

int i = 0, counter = 1, val;
#define GreenLed 9
#define RedLed 10
#define BlueLed 11
#include <LiquidCrystal.h>
#include <Servo.h>
LiquidCrystal lcd( 2, 3, 4, 5, 6, 7);
Servo myServo;

void setup() {
  Serial.begin(9600);
  myServo.attach(8);
  lcd.begin(16, 2);
}

void loop() {
  lcd.setCursor(0,1);
  counter = 1;
  Serial.println(i);
  for (i = 0; i < 5; i++)
  {
    val = analogRead(i);
    if (val < 550)
      counter++;
    else
    {
      analogWrite(RedLed, 0);
      analogWrite(GreenLed, 255);
      analogWrite(BlueLed, 0);
      myServo.write(90);
      lcd.clear();
      lcd.print("Parking bay ");
      lcd.print(counter);
      lcd.setCursor(0,2);
      lcd.print("is available");
      delay(200);
      break;
    }

    if (counter == 5)
    {
      lcd.clear();
      lcd.print("The parking lot");
      lcd.setCursor(0,2);
      lcd.print("is full");
      delay(200);
      analogWrite(RedLed, 255);
      analogWrite(GreenLed, 0);
      analogWrite(BlueLed, 0);
      myServo.write(0);
      break;
    }
  }
}

if you are able to help can you explain what to do?

But what makes you think that? What does happen, what do you expect?

septillion:
But what makes you think that? What does happen, what do you expect?

i mean that when the last park (analogpin5) is empty and the LCD should say that its the closest empty park it says that its full.

if (counter == 6)

Because you start counting at 1 and computers don't.

septillion:

if (counter == 6)

Because you start counting at 1 and computers don't.

notice that I Sayed

counter=1;

so the first park will be named 1 and not 0

alonkvetny1:
notice that I Sayed

counter=1;

so the first park will be named 1 and not 0

oooooohhh I see I'm so dumb sorry for saying that.

but I'm still having a problem, somewhy the value in tha last park (A4) needs higher value (800+in shadow) then others (550 anywhere)

did you try swapping sensors?

Some extra tips:

Don’t just make every variable global. Here, non of them needs to be global.

Why do you use PWM pins if you just going to turn them on and off? Or even, the use of PWM pins if fine, but why not use digitalWrite()? And this is a real problem because when using the Servo library PWM on pins 9 and 10 does not work.

The use of const is the saver variant of the pin definitions.

You can follow the variable names now but try to use more explaining names. That will help if things get more complex.

And for debugging, print more info, not only just the bay you’re checking.

I just gave it a little overhaul

#include <LiquidCrystal.h>
#include <Servo.h>

const byte GreenLedPin = 9;
const byte RedLedPin = 10;
const byte BlueLedPin = 11;
const byte ServoPin = 8;

//and I like to define all pins just to make it clear
const byte ParkingSensorPins[] = {A0, A1, A2, A3, A4};
const byte NumberOfBays = sizeof(ParkingSensorPins);

LiquidCrystal lcd( 2, 3, 4, 5, 6, 7);

const unsigned int ParkingSensorThreshold = 550;

//I'm guessing your controlling a barrier with it?
//then just call it that :)
Servo barrier;

void setup() {
  Serial.begin(9600);
  barrier.attach(ServoPin);
  lcd.begin(16, 2);
}

void loop() {
  lcd.setCursor(0,1);
  for (byte i = 0; i < NumberOfBays; i++){
    unsigned int val = analogRead(ParkingSensorPins[i]);
    
    Serial.print(F("Testing bay "));
    Serial.print(i);
    Serial.print(F(" value = "));
    Serial.println(val);
    
    if (val >= ParkingSensorThreshold){
      //Print info to LCD
      lcd.clear();
      lcd.print("Parking bay ");
      lcd.print(i + 1); //because computers start counting at 0
      lcd.setCursor(0,2);
      lcd.print("is available");
      
      delay(200);
      
      //No need to check for another bay if we found an empty one
      break;
    }

    //no need for a check here, because if nothing broke out of the for-loop
    //you know all spaces are full
      
    //set leds
    digitalWrite(RedLedPin, HIGH);
    digitalWrite(GreenLedPin, LOW);
    digitalWrite(BlueLedPin, LOW);
    
    //set barrier down
    barrier.write(0);
    
    //Print info to lcd
    lcd.clear();
    lcd.print("The parking lot");
    lcd.setCursor(0,2);
    lcd.print("is full");
    
    delay(200);
    }
  }
}

alonkvetny1:
notice that I Sayed

counter=1;

so the first park will be named 1 and not 0

I did notice. But that means that if you 5th bay is empty the counter will read 5. But when it's 5 you start printing it's full already.

BulldogLowell:
did you try swapping sensors?

yes I did

septillion:
Some extra tips:

Don’t just make every variable global. Here, non of them needs to be global.

Why do you use PWM pins if you just going to turn them on and off? Or even, the use of PWM pins if fine, but why not use digitalWrite()? And this is a real problem because when using the Servo library PWM on pins 9 and 10 does not work.

The use of const is the saver variant of the pin definitions.

You can follow the variable names now but try to use more explaining names. That will help if things get more complex.

And for debugging, print more info, not only just the bay you’re checking.

I just gave it a little overhaul

#include <LiquidCrystal.h>

#include <Servo.h>

const byte GreenLedPin = 9;
const byte RedLedPin = 10;
const byte BlueLedPin = 11;
const byte ServoPin = 8;

//and I like to define all pins just to make it clear
const byte ParkingSensorPins = {A0, A1, A2, A3, A4};
const byte NumberOfBays = sizeof(ParkingSensorPins);

LiquidCrystal lcd( 2, 3, 4, 5, 6, 7);

const unsigned int ParkingSensorThreshold = 550;

//I’m guessing your controlling a barrier with it?
//then just call it that :slight_smile:
Servo barrier;

void setup() {
  Serial.begin(9600);
  barrier.attach(ServoPin);
  lcd.begin(16, 2);
}

void loop() {
  lcd.setCursor(0,1);
  for (byte i = 0; i < NumberOfBays; i++){
    unsigned int val = analogRead(ParkingSensorPins[i]);
   
    Serial.print(F("Testing bay “));
    Serial.print(i);
    Serial.print(F(” value = "));
    Serial.println(val);
   
    if (val >= ParkingSensorThreshold){
      //Print info to LCD
      lcd.clear();
      lcd.print("Parking bay ");
      lcd.print(i + 1); //because computers start counting at 0
      lcd.setCursor(0,2);
      lcd.print(“is available”);
     
      delay(200);
     
      //No need to check for another bay if we found an empty one
      break;
    }

//no need for a check here, because if nothing broke out of the for-loop
    //you know all spaces are full
     
    //set leds
    digitalWrite(RedLedPin, HIGH);
    digitalWrite(GreenLedPin, LOW);
    digitalWrite(BlueLedPin, LOW);
   
    //set barrier down
    barrier.write(0);
   
    //Print info to lcd
    lcd.clear();
    lcd.print(“The parking lot”);
    lcd.setCursor(0,2);
    lcd.print(“is full”);
   
    delay(200);
    }
  }
}

I will try it soon I will write here what I got

Not only try it, try to learn from it as well :wink:

septillion:
Not only try it, try to learn from it as well :wink:

the code you wrote is art, looks so clean and beautifull but the LCD is blinking between park 5 is empty and park is full, i got my code to work but thanks for showing me another way of doing it!
if you don't mind to explain me why you did those:

const byte
NumberOfBays = sizeof(ParkingSensorPins); ( what does sizeof do? i read on wiki but still don't get it )
unsigned int
Serial.print(F(..)); what does the F stands for?

Although okay, I did made some errors :confused:

We do need a check for parking lot is full. Because if 1 isn’t available it will print “parking lot full” before checking the next slot… This may be the blinking?

And I think you already found it, but there is a closing bracket to much…

Didn’t add F() macro everywhere…

Barrier up and traffic light green disappeared…

#include <LiquidCrystal.h>
#include <Servo.h>

const byte GreenLedPin = 9;
const byte RedLedPin = 10;
const byte BlueLedPin = 11;
const byte ServoPin = 8;

//and I like to define all pins just to make it clear
const byte ParkingSensorPins[] = {A0, A1, A2, A3, A4};
const byte NumberOfBays = sizeof(ParkingSensorPins);

LiquidCrystal lcd( 2, 3, 4, 5, 6, 7);

//Now all setting are nicely grouped at the top
const unsigned int ParkingSensorThreshold = 550;
const unsigned int LoopInterval = 200;

const byte BarrierUpValue = 90;
const byte BarrierDownValue = 0;

//I'm guessing your controlling a barrier with it?
//then just call it that :)
Servo barrier;

void setup() {
  Serial.begin(9600);
  barrier.attach(ServoPin);
  lcd.begin(16, 2);
}

void loop() {
  lcd.setCursor(0,1);
  for (byte i = 0; i < NumberOfBays; i++){
    unsigned int val = analogRead(ParkingSensorPins[i]);
    
    Serial.print(F("Testing bay "));
    Serial.print(i);
    Serial.print(F(" value = "));
    Serial.println(val);
    
    if (val >= ParkingSensorThreshold){
      //set leds
      digitalWrite(RedLedPin, LOW);
      digitalWrite(GreenLedPin, HIGH);
      digitalWrite(BlueLedPin, LOW);
      
      //set barrier
      barrier.write(BarrierUpValue);
      
      //Print info to LCD
      lcd.clear();
      lcd.print(F("Parking bay "));
      lcd.print(i + 1); //because computers start counting at 0
      lcd.setCursor(0,2);
      lcd.print(F("is available"));
      
      //No need to check for another bay if we found an empty one
      break;
    }

    //If we are at the last bay (starting at 0)
    if( (i + 1) == NumberOfBays){
      //set leds
      digitalWrite(RedLedPin, HIGH);
      digitalWrite(GreenLedPin, LOW);
      digitalWrite(BlueLedPin, LOW);
      
      //set barrier
      barrier.write(BarrierDownValue);
      
      //Print info to lcd
      lcd.clear();
      lcd.print(F("The parking lot"));
      lcd.setCursor(0,2);
      lcd.print(F("is full"));
    }
  }
  
  //waiting in a single spot. Now also easy to change it to millis() ;)
  delay(LoopInterval)
}

sizeof() gives you the size of a variable in bytes. Applied to an array it will give the total size of the array, again in bytes. But because each entry in the array is a byte, it gives the number of elements in the array.

And that’s the F() macro. That will keep the text in program memory (on a Uno 32kB) instead if precious SRAM (2kB).

septillion:
Although okay, I did made some errors :confused:

We do need a check for parking lot is full. Because if 1 isn’t available it will print “parking lot full” before checking the next slot… This may be the blinking?

And I think you already found it, but there is a closing bracket to much…

Didn’t add F() macro everywhere…

Barrier up and traffic light green disappeared…

#include <LiquidCrystal.h>

#include <Servo.h>

const byte GreenLedPin = 9;
const byte RedLedPin = 10;
const byte BlueLedPin = 11;
const byte ServoPin = 8;

//and I like to define all pins just to make it clear
const byte ParkingSensorPins = {A0, A1, A2, A3, A4};
const byte NumberOfBays = sizeof(ParkingSensorPins);

LiquidCrystal lcd( 2, 3, 4, 5, 6, 7);

//Now all setting are nicely grouped at the top
const unsigned int ParkingSensorThreshold = 550;
const unsigned int LoopInterval = 200;

const byte BarrierUpValue = 90;
const byte BarrierDownValue = 0;

//I’m guessing your controlling a barrier with it?
//then just call it that :slight_smile:
Servo barrier;

void setup() {
  Serial.begin(9600);
  barrier.attach(ServoPin);
  lcd.begin(16, 2);
}

void loop() {
  lcd.setCursor(0,1);
  for (byte i = 0; i < NumberOfBays; i++){
    unsigned int val = analogRead(ParkingSensorPins[i]);
   
    Serial.print(F("Testing bay “));
    Serial.print(i);
    Serial.print(F(” value = "));
    Serial.println(val);
   
    if (val >= ParkingSensorThreshold){
      //set leds
      digitalWrite(RedLedPin, LOW);
      digitalWrite(GreenLedPin, HIGH);
      digitalWrite(BlueLedPin, LOW);
     
      //set barrier
      barrier.write(BarrierUpValue);
     
      //Print info to LCD
      lcd.clear();
      lcd.print(F("Parking bay "));
      lcd.print(i + 1); //because computers start counting at 0
      lcd.setCursor(0,2);
      lcd.print(F(“is available”));
     
      //No need to check for another bay if we found an empty one
      break;
    }

//If we are at the last bay (starting at 0)
    if( (i + 1) == NumberOfBays){
      //set leds
      digitalWrite(RedLedPin, HIGH);
      digitalWrite(GreenLedPin, LOW);
      digitalWrite(BlueLedPin, LOW);
     
      //set barrier
      barrier.write(BarrierDownValue);
     
      //Print info to lcd
      lcd.clear();
      lcd.print(F(“The parking lot”));
      lcd.setCursor(0,2);
      lcd.print(F(“is full”));
    }
  }
 
  //waiting in a single spot. Now also easy to change it to millis() :wink:
  delay(LoopInterval)
}




sizeof() gives you the size of a variable in bytes. Applied to an array it will give the total size of the array, again in bytes. But because each entry in the array is a byte, it gives the number of elements in the array.

And that's the [F() macro](https://playground.arduino.cc/Learning/Memory). That will keep the text in program memory (on a Uno 32kB) instead if precious SRAM (2kB).

works great thanks for teaching me something new
I don’t think I can use this code because it looks too good to be true that I made it, I mean its my first code on Arduino and I will need to explain the code too, and I think its better if I stay with my code. ( it works too but takes less space on the Arduino)
thanks again for your help !!! :))

septillion:
Although okay, I did made some errors :confused:

then maybe your won't get an "A" by proxy... :confused:

BulldogLowell:
then maybe your won’t get an “A” by proxy… :confused:

Damnit :frowning: And does it count as cheating to use a smart text editor? I remember I had to do my frist programming exams with pen and paper :stuck_out_tongue:

@alonkvetny1, but you learned didn’t you? As long as you can explain why I did certain things you know them know as well. For example, why are a lot of variables const?

That is the main key to really understanding something, the moment you can explain it to somebody else :slight_smile:

septillion:
For example, why are a lot of variables const?

because they don't need to be changed?

you making me nervous that I will sound like an idiot in the final exam lol :))

septillion:
That is the main key to really understanding something, the moment you can explain it to somebody else :slight_smile:

... and be correct.

Correct. And you let the compiler know. This has two advantages 1) You can't accidentally change it yourself. 2) The compiler may look at it and think, okay, so no need to put it in SRAM then.

Just test questions you can ask yourself:

Why did I put all numbers like 550 etc in variables?

Why did I rename myServo to barrier?

Why did I rename GreenLed to GreenLedPin?

Why did I remove counter?

Why did I add F()?

etc.

If you can answer those questions you have a better understanding on how to structure your program more clearly.