PLEASE sanity check my arduino code

[lilshawn]

I'm still pretty new to arduino. I've been working on a project for a couple days now and i'm having a hell of a time wrapping my head around some things.

I'm trying to use a button input to change a variable that i can use to do some maths later on in code. but I can't for the life of me get it to work.

basically I'm looking to increment or decrement a value (TEMPSET) for adjusting an on/off point.

So as far as I can tell you have to set a variable from the value of the input, (read the input and store the result in the variable)....then check if the variable has changed since the last time it was checked... if so then increment the TEMPSET +1 (or -1)

this is what I have in my initialization...

Code: [Select]

int TEMPSET = 35;
int VALDOWN = 0;
int VALUP = 0;
int LASTUP = 0;
int LASTDOWN = 0;
and the actual button routine in the main loop...

Code: [Select]

VALDOWN = digitalRead(4);
  if (VALDOWN != LASTDOWN) {
    if (VALDOWN == HIGH) {
    TEMPSET -1;
  } else {
   VALDOWN = LASTDOWN;
  }
  delay(100);
}


VALUP = digitalRead(5);
  if (VALUP != LASTUP) {
    if (VALUP == HIGH) {
     TEMPSET +1;
  } else {
    VALUP = LASTUP;
  }
  delay(100);
}
My inputs are tied to ground through a 10k resistor and pulled to +5v (high) when pressed so I think my wiring there is good... but the value of TEMPSET is not changing. I'm sure there is something wrong here but i'm just not seeing it... And I don;t know enough about it to have it be glaringly obvious.

thank you in advance

[baritonomarchetto]

You are not updating LASTDOWN there.

Remove those pulldown resistors and define pin 4 and 5 as INPUT_PULLUP instead of INPUT. You will spare some resistor for future projects
Then check the following code:

Code: [Select]

VALDOWN = digitalRead(4);
  if (VALDOWN != LASTDOWN) {
  LASTDOWN = VALDOWN;
    if (VALDOWN == LOW) {TEMPSET -1;}
delay(20);//cheap debounce
}

Apply the same form to VALUP.

Next time post the whole code so that it will be easier to check for missing variables and/or definitions

[lilshawn]

thanks for the help, after I posted the code snippets i'd realized i probably could have just a easily posted the whole thing and been done with it. I had no idea there was an already built in pullup input.

like I mentioned i'm really just stabbing at the dark and googling as I go. I haven "coded" since BASIC in my pre-teens and Hypertalk in my teens. I attempted C later on but was just too much for me. Making sure all these braces are closed is killing me. 

Code: [Select]

#include <Wire.h>
#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x3F, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);  // Set the LCD I2C address
#define THERMISTORPIN A0
#define THERMISTORNOMINAL 10000
#define TEMPERATURENOMINAL 25
#define NUMSAMPLES 10
#define BCOEFFICIENT 3950
#define SERIESRESISTOR 10000
#define TEMPMIN 30
#define TEMPMAX 42
int TEMPSET = 35;
int VALDOWN = 0;
int VALUP = 0;
int LASTUP = 0;
int LASTDOWN = 0;
int L = 0;
 
const int relay =11;

uint16_t samples[NUMSAMPLES];

void setup()
{
pinMode(11, OUTPUT);
pinMode(4, INPUT);
pinMode(5, INPUT);


  lcd.begin(16,2);
  lcd.backlight();
  lcd.clear();
}


void loop(void){

  for (int L=0; L<100; L++){
 

  uint8_t i;

  float average;
   for (i=0; i< NUMSAMPLES; i++) {
   samples[i] = analogRead(THERMISTORPIN);
   delay(10);
  }
 
    average = 0;
  for (i=0; i< NUMSAMPLES; i++) {
     average += samples[i];
  }
  average /= NUMSAMPLES;
 


 
  average = 1023 / average - 1;
  average = SERIESRESISTOR / average;
 
  float celcius;
  celcius = average / THERMISTORNOMINAL;     // (R/Ro)
  celcius = log(celcius);                  // ln(R/Ro)
  celcius /= BCOEFFICIENT;                   // 1/B * ln(R/Ro)
  celcius += 1.0 / (TEMPERATURENOMINAL + 273.15); // + (1/To)
  celcius = 1.0 / celcius;                 // Invert
  celcius -= 273.15;                         // convert to C

 VALDOWN = digitalRead(4);
  if (VALDOWN != LASTDOWN) {
    if (VALDOWN == HIGH) {
    TEMPSET -1;
  } else {
   VALDOWN = LASTDOWN;
  }
  delay(100);
}


VALUP = digitalRead(5);
  if (VALUP != LASTUP) {
    if (VALUP == HIGH) {
     TEMPSET +1;
  } else {
    VALUP = LASTUP;
  }
  delay(100);
}
   
 lcd.setCursor(0,0);
  lcd.print("Temp ");
  lcd.print (celcius);
  lcd.print((char)223);
  lcd.print("C   ");
  lcd.setCursor(0,1);
  lcd.print("Set  ");
  lcd.print (TEMPSET);
  lcd.print((char)223);
  lcd.print("C");

 
  if (celcius < TEMPSET) {
    digitalWrite(11, HIGH);
  } else {
    digitalWrite(11, LOW);
  }
  }
}

once i can get the increment/decrement working i'll be setting a minimum and maximum and doing a check for it to make sure the TEMPSET value does not exceed or fall below the established minimum/maximum (in this case the low value of 30 and maximum of 42)

[baritonomarchetto]

Did you check the code piece i wrote? I don't see those corrections I suggested you. Believe me, It should work

[lilshawn]

sorry, last night was a long ass 15 hour day with a whole lot of driving. stuff always breaks down in the worst possible way... 6 hours drive away.

I'll give it a try when I get home tonight. 

[lilshawn]

alright, got it... for some reason I had to do....

{TEMPSET = TEMPSET +1;}

instead of just

{TEMPSET +1;}


oh well onto the next part... thanks for the push 

[lilshawn]

i'm probably looking at rearranging my code to add this... but...

what are thoughts on having lcd.offBacklight() at the beginning of code, the use the buttons to insert an lcd.backlight() to turn on the backlight for a few seconds after the button press and FOR loop countdown back to lcd.offBacklight() ?

I'm probably looking at a convoluted for loop yeah?

[lilshawn]

okay... i've discovered I need to add some kind of hysteresis to the temperature.  the temperature reading sometimes fluctuates over the trigger set point causing my output to rapidly cycle on off on off on off as the reading goes from say, 34.99 to 40. i'll be switching a high current load with a secondary relay and I'll burn out the contacts in no time if it's rapidly switching like this.