My first sketch, did i do it right?

Hi all,

This is my first program to test my new mega, does it look correct for arduino code? how should I improve it?

I have a light dependent resistor, thermistor, buzzer, 16x2 LCD and a 7-segment 4-digit LED display with a 74HC595 8-bit shift register driving it.

It beeps with power on, then flips displays between temperature and light levels.

// LCD
#include <LiquidCrystal.h> // include the library code
LiquidCrystal lcd(53, 52, 51, 50, 49, 48, 47, 46, 45, 44, 43); // initialize the library with the numbers of the interface pins
// DEFINES
int ledPin = 13;   // select the pin for the LED
int slow = 0;  // slow down sensor readings and flip between
int num = 0;  // the number for display
int buzzerPin = 4;  //power on beep

// LIGHT DEPENDENT RESISTOR 
int LDR = 15;       // select the input pin for the LDR
int val = 0;       // variable to store the value coming from the sensor
int ldrval =0;
int ldrval2 =0;

// THERMISTER CONTROL + MATH FOR CELCIUS CONVERTION 
const float rinfinit = 93.79645;   // This you should calculate yours
const float voltage = 5.04;        // should change it to 5040 for 5.04V
const float B = 4100;
const float unodivr = 93.79645;
const float resistor = 5900;       // use this to set temp via resistor value
int sensorPin = 14;                // select the input pin for the thermister
int sensorValue = 0;               // variable to store the value coming from the sensor

// 7 SEGMENT 4 DIGIT LED - 8 BIT SHIFT REGISTER
int latchPin = 31;  //Pin connected to ST_CP of 74HC595 (aka RCLK)
int clockPin = 32;  //Pin connected to SH_CP of 74HC595 (aka SRCLK)
int dataPin = 30;  //Pin connected to DS of 74HC595 (aka SER)
int digit1Pin = 33;  //4 digits LED to light
int digit2Pin = 34;
int digit3Pin = 35;
int digit4Pin = 36;
int digits[10]={63,6,91,79,102,109,125,7,127,111};  // array to hold 10 bit binary 0-9 to swap for alphanumeric 0-9:

//MAIN SETUP
void setup() {
Serial.begin(115200);

// SETUP PINMODES
pinMode(buzzerPin, OUTPUT);  // power on beep
pinMode(LDR, INPUT);       // declare the LDR as an INPUT
pinMode(ledPin, OUTPUT);  // declare the ledPin as an OUTPUT

//  7 SEGMENT 4 DIGIT LED - 8 BIT SHIFT REGISTER SETUP
pinMode(digit1Pin, OUTPUT);
pinMode(digit2Pin, OUTPUT);
pinMode(digit3Pin, OUTPUT);
pinMode(digit4Pin, OUTPUT);
pinMode(latchPin, OUTPUT);
pinMode(clockPin, OUTPUT);
pinMode(dataPin, OUTPUT);

// LCD SETUP
lcd.begin(16, 2);  // set up the LCD's number of columns and rows
lcd.print("hello");  // Print a welcome message to the LCD

// SETUP COMPLETE BEEP + PAUSE
tone(4, 2000, 100);
delay(2000);
}
// MAIN PROGRAM LOOP
void loop(){
  
// THERMISTER MAIN LOOP
if (slow<500){
  // Convert the value to volts
  float v2 = (voltage*float(sensorValue))/1024.0f;    // calculate volts in an awesome world
  float r1a = (voltage*float(resistor))/v2;  // sums to convert to temp
  float r1 =r1a - resistor;
  float T = B/log(r1*unodivr);
  T=T-273.15;
  num=T;  // temp output in oC
  lcd.setCursor(0,0);
  lcd.print("temperature is c");  // Print a message to the LCD
    digitalWrite(ledPin, LOW);   // turn the ledPin off
}
// LDR MAIN LOOP NUMBER REWORK SO SCALE FITS TO 0-9999ISH
if (slow>500){
  ldrval2 = (ldrval * 8);  //
  num = (8150-ldrval2);
  lcd.setCursor(0,0);
  lcd.print("Light levels are");  // Print a message to the LCD
}
// SWAP OVER DISPLAY + TAKE READINGS
slow=(slow-1);
if (slow<0){
  digitalWrite(ledPin, HIGH);  // turn the ledPin on
  ldrval = analogRead(LDR);       // read the value from the ldr sensor on pin 15
  sensorValue = analogRead(sensorPin);     //read temp from thermister    
  slow=1000; 
} 
//  7 SEGMENT 4 DIGIT LED - 8 BIT SHIFT REGISTER LOOP
int thousand, hundred, ten, one;  // breakdown number into columns
thousand = num/1000;
hundred = (num-(thousand*1000))/100;
ten = (num-((thousand*1000)+(hundred*100)))/10;
one = num-((thousand*1000)+(hundred*100)+(ten*10));
// now drive the LED's via shift register:
digitalWrite(latchPin, LOW);  // take the latchPin low so the LEDs don't change while you're sending
shiftOut(dataPin, clockPin, MSBFIRST, digits[one]);  // shift out the data bits for digit via array 0-9
digitalWrite(latchPin, HIGH);  //take the latch pin high so the LEDs will light up
// select digit to light up:
digitalWrite(digit1Pin, HIGH);
digitalWrite(digit2Pin, HIGH);
digitalWrite(digit3Pin, HIGH);
digitalWrite(digit4Pin, LOW);
delay(1);// and pause before next value so LED gets time to light

// repeat for all digits:
if (hundred>0 || thousand>0 || ten>0){ //only turn on if number is over 1 digit ?MAY ONLY COMPARE 2 AS SHOWS 0 HERE STILL?
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, digits[ten]);
  digitalWrite(latchPin, HIGH);
  digitalWrite(digit1Pin, HIGH);
  digitalWrite(digit2Pin, HIGH);
  digitalWrite(digit3Pin, LOW);
  digitalWrite(digit4Pin, HIGH);
  delay(1);
}
if (hundred>0 || thousand>0){ // WORKS HERE BUT NOT WITH 3?
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, digits[hundred]);
  digitalWrite(latchPin, HIGH);
  digitalWrite(digit1Pin, HIGH);
  digitalWrite(digit2Pin, LOW);
  digitalWrite(digit3Pin, HIGH);
  digitalWrite(digit4Pin, HIGH);
  delay(1);
}
if (thousand>0){  // WORKS HERE BUT NOT WITH 3?
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, digits[thousand]);
  digitalWrite(latchPin, HIGH);
  digitalWrite(digit1Pin, LOW);
  digitalWrite(digit2Pin, HIGH);
  digitalWrite(digit3Pin, HIGH);
  digitalWrite(digit4Pin, HIGH);
  delay(1);
}
// UNCOMMENT FOR DEBUGING
/* 
Serial.println"DEBUGING";
Serial.println"LDR value";
Serial.println(val);
Serial.println"temperature";
Serial.println(T);
Serial.println"temperature or light levels";
Serial.println(num);        
Serial.println"display digits 1000,100,10,1";
Serial.println(thousand);
Serial.println(hundred);
Serial.println(ten); 
Serial.println(one); 
Serial.println"END";
Serial.println"";
delay(2000);
*/
}

My problems any help would be cool,

  1. Mega2560 r2 wont upload my sketch with 1.0 IDE but works with 0023?
  2. 16x2 screen only works on full 8-bit, 4-bit displays garbage?
  3. 16x2 screen wont display 2 line or numbers and replaces spaces with tiddle?
  4. The second digit on the 7 segment LED still illuminates even if it is a 0? Can I use "if (hundred>0 || thousand>0 || ten>0)" with 2 only?
  5. Is the code correct or bad overall for arduino?

Thanks, Matt

a, does it look correct for arduino code? h

You could make more use of the "const" qualified for things like your pin identifiers.

 Serial.println"display digits 1000,100,10,1";

That's not good

AWOL:

a, does it look correct for arduino code? h

You could make more use of the "const" qualified for things like your pin identifiers.

Ah yes, that will save memory! thanks

 Serial.println"display digits 1000,100,10,1";

That's not good

That's just a debug line to tell me that the next lines through will be in order of thousands, hundreds, tens, units?

Thanks I will use "const" for non-changing variables from now on, I didn't even know I could!

That's just a debug line to tell me that the next lines through will be in order of thousands, hundreds, tens, units?

But does it compile?

Oh,
Serial.println("display digits 1000,100,10,1");

That works better! shows I should have de-bugged the de-bug even if the program worked! lol

thanks