Random nunbers displaying

Hi,

I have been working on this code here trying to create a GUI for the user to input their PIN. Every time the user gets to the third number, the lcd displays random numbers (See video link).

Any ideas? I am 99.99% sure it has nothing to do with my lcd library as I have used it for other programs without problems, but ill post the code to that just in case.

I think the problem may lie outside the while loop, but to tell you the truth I am completely lost right now. I might have just placed a bracket in the wrong spot or done some other stupid mistake.

Any help is appreciated.

Video: http://ubuntuone.com/p/VMh/

Main program:

/*Sample code for the SerialLcd library which shows the time in millis on the top and bottom lines of the LCD.
 */
#include <Sparklcd.h>
Sparklcd lcd;


int inArray[3];

void setup()
{
  Serial.begin(9600);
  pinMode(0, INPUT); //POT input
  pinMode(4, INPUT);  //"Enter" key
  lcd.backlightOn();
  Serial.print("     ");
  delay(500);
}

void loop(){  

  input(inArray);
}

void input(int inArray[0]){
  int pot;
  int i;
  lcd.clearLCD();
  lcd.goTo(3);
  Serial.print("Enter PIN:");
  lcd.goTo(20);
  Serial.print("_ _ _ _");
  delay(500);
  
  for(i=0; i<4; i++){
    while(digitalRead(4) == LOW){          //While "Enter not being pressed.."
      pot = analogRead(0);
      if(pot <= 1023 && pot >= 774){        //value between 1023 - 774 display 0 etc..
        inArray[i] = 0;
        lcd.clearLCD();
        lcd.goTo(3);
        Serial.print("Enter PIN:");
        lcd.goTo(23);
        Serial.print(inArray[i]);
        delay(100);    
      }
      else{
        if(pot < 774 && pot >= 525){
          inArray[i] = 1;
          lcd.clearLCD();
          lcd.goTo(3);
          Serial.print("Enter PIN:");
          lcd.goTo(23);
          Serial.print(inArray[i]);
          delay(100);
        }
        else{
          if(pot < 525 && pot >= 190){
            inArray[i] = 2;
            lcd.clearLCD();
            lcd.goTo(3);
            Serial.print("Enter PIN:");
            lcd.goTo(23);
            Serial.print(inArray[i]);
            delay(100);
          }
          else{
            if(pot < 190 && pot >= 55){
              inArray[i] = 3;
              lcd.clearLCD();
              lcd.goTo(3);
              Serial.print("Enter PIN:");
              lcd.goTo(23);
              Serial.print(inArray[i]);
              delay(100);
            }
            else{
              if(pot < 55 && pot >= 28){
                inArray[i] = 4;
                lcd.clearLCD();
                lcd.goTo(3);
                Serial.print("Enter PIN:");
                lcd.goTo(23);
                Serial.print(inArray[i]);
                delay(100);
              }
              else{
                if(pot < 28 && pot >= 9){
                  inArray[i] = 5;
                  lcd.clearLCD();
                  lcd.goTo(3);
                  Serial.print("Enter PIN:");
                  lcd.goTo(23);
                  Serial.print(inArray[i]);
                  delay(100);
                }
                else{
                  if(pot < 9 && pot >= 4){
                    inArray[i] = 6;
                    lcd.clearLCD();
                    lcd.goTo(3);
                    Serial.print("Enter PIN:");
                    lcd.goTo(23);
                    Serial.print(inArray[i]);
                    delay(100);
                  }
                  else{
                    if(pot == 3){
                      inArray[i] = 7;
                      lcd.clearLCD();
                      lcd.goTo(3);
                      Serial.print("Enter PIN:");
                      lcd.goTo(23);
                      Serial.print(inArray[i]);
                      delay(100);
                    }
                    else{
                      if(pot == 2){
                        inArray[i] = 8;
                        lcd.clearLCD();
                        lcd.goTo(3);
                        Serial.print("Enter PIN:");
                        lcd.goTo(23);
                        Serial.print(inArray[i]);
                        delay(100);
                      }
                      else{
                        if(pot == 1){
                          inArray[i] = 9;
                          lcd.clearLCD();
                          lcd.goTo(3);
                          Serial.print("Enter PIN:");
                          lcd.goTo(23);
                          Serial.print(inArray[i]);
                          delay(100);
                        }
                      }
                    }
                  }
                }
              }
            }
          }
        }
      }
    }
    if(i == 0){                        //If "Enter" pressed
      lcd.clearLCD();
      lcd.goTo(3);
      Serial.print("Enter PIN:");
      lcd.goTo(20);
      Serial.print(inArray[i]);
      lcd.goTo(22);
      Serial.print("_ _ _");
      delay(500);
      lcd.clearLCD();
      lcd.goTo(3);
      Serial.print("Enter PIN:");
      lcd.goTo(20);
      Serial.print("*");
      lcd.goTo(22);
      Serial.print("_ _ _");
      delay(100);
    }
    else{
      if(i == 1){
        lcd.clearLCD();
        lcd.goTo(3);
        Serial.print("Enter PIN:");
        lcd.goTo(20);
        Serial.print("*");
        lcd.goTo(22);
        Serial.print(inArray[i]);
        lcd.goTo(24);
        Serial.print("_ _");
        delay(500);
        lcd.clearLCD();
        lcd.goTo(3);
        Serial.print("Enter PIN:");
        lcd.goTo(20);
        Serial.print("* *");
        lcd.goTo(24);
        Serial.print("_ _");
        delay(100);
      }
      else{
        if(i == 2){
          lcd.clearLCD();
          lcd.goTo(3);
          Serial.print("Enter PIN:");
          lcd.goTo(20);
          Serial.print("* *");
          lcd.goTo(24);
          Serial.print(inArray[i]);
          lcd.goTo(26);
          Serial.print("_");
          delay(500);
          lcd.clearLCD();
          lcd.goTo(3);
          Serial.print("Enter PIN:");
          lcd.goTo(20);
          Serial.print("* * *");
          lcd.goTo(26);
          Serial.print("_");
          delay(100);
        }
        else{
          lcd.clearLCD();
          lcd.goTo(3);
          Serial.print("Enter PIN:");
          lcd.goTo(20);
          Serial.print("* * *");
          lcd.goTo(26);
          Serial.print(inArray[i]);
          delay(500);
          lcd.clearLCD();
          lcd.goTo(3);
          Serial.print("Enter PIN:");
          lcd.goTo(20);
          Serial.print("* * * *");
          delay(500);
        }
      }
    }
  }
}

LCD .h:

/*Written by Frederick Hunter 17.12.2010 based on the Arduino.cc
website http://www.arduino.cc/playground/Learning/SparkFunSerLCD
*/
/*PRE & POST Conditions:
 * void selectLineOne();
 * PRE: NONE
 * POST: moves cursor to the first position in the top line
 * 
 * void selectLineTwo();
 * PRE: NONE
 * POST: moves cursor to the first position on the bottom line
 * 
 * void goTo(int position);
 * PRE: position is the block number you want to put the cursor to (0-15 is line 1 & 16-31 is line two, 31+ sets it back to 0)
 * POST: moves cursor to defined block position
 * 
 * void clearLCD();
 * PRE: NONE
 * POST: erases everything on the LCD screen
 * 
 * void backlighOn();
 * PRE: NONE
 * POST: Turns on the backlight
 * 
 * void backlightOff();
 * PRE: NONE
 * POST: Turns the backlight off
 * 
 * void serCommand();
 * PRE: NONE
 * POST: calls specified command
 */
 


#ifndef SPARKLCD_H
#define SPARKLCD_H

#include <WProgram.h>

class Sparklcd{
  public:
    Sparklcd();
    ~Sparklcd();
    void selectLineOne();
    void selectLineTwo();
    void goTo(int position);
    void clearLCD();
    void backlightOn();
    void backlightOff();
    void serCommand();
};

#endif

And Lcd.cpp

/*Written by Frederick Hunter 17.12.2010 based on the Arduino.cc
website http://www.arduino.cc/playground/Learning/SparkFunSerLCD
*/
#include "WProgram.h"
#include "Sparklcd.h"

Sparklcd::Sparklcd(){
}
Sparklcd::~Sparklcd(){
}

void Sparklcd::selectLineOne(){  
   Serial.print(0xFE, BYTE);  
   Serial.print(128, BYTE);    
}
void Sparklcd::selectLineTwo(){  
   Serial.print(0xFE, BYTE);   
   Serial.print(192, BYTE);   
}
void Sparklcd::goTo(int position) { 
if (position<16){
  Serial.print(0xFE, BYTE);   
  Serial.print((position+128), BYTE);   
}
else if (position<32){
  Serial.print(0xFE, BYTE);   
  Serial.print((position+48+128), BYTE);   
} 
else {
  goTo(0);
}
}

void Sparklcd::clearLCD(){
   Serial.print(0xFE, BYTE);   
   Serial.print(0x01, BYTE); 
}
void Sparklcd::backlightOn(){ 
    Serial.print(0x7C, BYTE);   
    Serial.print(157, BYTE);   
}
void Sparklcd::backlightOff(){  
    Serial.print(0x7C, BYTE);   
    Serial.print(128, BYTE);     
}
void Sparklcd::serCommand(){   
  Serial.print(0xFE, BYTE);
}

int inArray[3]; and you want to store 4 digits ... ::slight_smile:
int x[4] means the valid subscripts are 0, 1, 2, and 3; a total of 4 elements.
We're all mortal :wink:

Thank you Msquare, that solved my problem, but all I want are four values to be stored, so why do I need 5? I dont understand.

      if(pot <= 1023 && pot >= 774)
        if(pot < 774 && pot >= 525)
          if(pot < 525 && pot >= 190)
            if(pot < 190 && pot >= 55)
              if(pot < 55 && pot >= 28)
                if(pot < 28 && pot >= 9)
                  if(pot < 9 && pot >= 4)
                    if(pot == 3)
                      if(pot == 2)
                        if(pot == 1)

How is the user supposed to know what value is going to be read from the pot? Are you actually able to repeatedly get values of 1, 2, or 3 from the pot?
The value read from the pot can never be greater than 1023, so the first conditional is unnecessary.

How many times have you repeated this code:

              lcd.clearLCD();
              lcd.goTo(3);
              Serial.print("Enter PIN:");
              lcd.goTo(23);
              Serial.print(inArray[i]);
              delay(100);

It should be moved out of the if block.

The else clause does not need to be inside curly braces. The whole if/else if block should be much shorter.

additional to PaulS I would remark that you should “reuse tested values” the function becomes something like:

(not tested / compiled)

void input(int inArray[0])  [glow]// does this parameter work?????? [/glow]
{
  int pot;
  int i;

  lcd.clearLCD();
  lcd.goTo(3);
  Serial.print("Enter PIN:");
  lcd.goTo(20);
  Serial.print("_ _ _ _");
  delay(500);

  for(i=0; i<4; i++)
  {
    while (digitalRead(4) == LOW)    //While "Enter not being pressed.."
    {          
      pot = analogRead(0);
      if (pot >= 774) inArray[i] = 0;
      else if (pot >= 525) inArray[i] = 1;
      else if (pot >= 190) inArray[i] = 2;
      else if (pot >= 55) inArray[i] = 3;
      else if (pot >= 28) inArray[i] = 4;
      else if (pot >= 9) inArray[i] = 5;
      else if (pot >= 4) inArray[i] = 6;
      else if (pot == 3) inArray[i] = 7;
      else if (pot == 2) inArray[i] = 8;
      else if (pot == 1) inArray[i] = 9;
      else inArray[i] = 10;            [glow]// pot can be 0 - missed this one ??[/glow]

      // display chosen digit
      lcd.goTo(23);
      Serial.print(inArray[i]);
      delay(100); 
    }

    // remove last chosen digit
    lcd.goTo(23);
    Serial.print(" ");  

     for (int x=0; x < i; x++)     // change _ to *
    {
      lcd.goTo(20 +2*x);
      Serial.print("*");  
    }
    delay(500);
  }
}

Less code and easier to read imho

(Christmas gift, normally I do not recode whole blocks :wink:

but all I want are four values to be stored, so why do I need 5?

This is one of "C" or "C++" small quirks.
When you declare an array with

int a[4]

you get a[0], a[1], a[2] and a[3] to play with.
In other words when you say
** **a[4]** **
you are saying you want four positions to play with. It is not the "highest" subscript value.

And the reason things turned "random" is that when you did put something into your inArray[4] (which you had declared with inArray[4], you were overwriting a completly differnet variable (we do not really know which one the compiler put next to the inArray) because inArray[3] was the highest legal space you had reserved.