Please help with making display stable.

Hello everyone,
First off, as most have heard over and over, I'm a complete noob when it comes to arduino coding. I'm doing a project which should be so simple (and probably is for most) but I'm so stumped.

Here's what I'm doing.
This is for a 6 head shower system. I have a keypad that will turn/off each shower head by pressing each key. When any of the shower heads are turned on an oled display will turn on and display the water temperature.

My problem, I'm having trouble getting the display to stay on steady with live temperature readings. I either get stuck in the display loop and am not able to recognize the key presses. Or I can get the keys presses working but the display won't read live data.
I'm using an arduino mega. Below is my code

#include <Wire.h>
#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>
#include <math.h>


#define OLED_DC 19
#define OLED_CS 17
#define OLED_CLK 20
#define OLED_MOSI 21
#define OLED_RESET 18
#define ThermistorPIN A1                 // Analog Pin 1
Adafruit_SSD1306 display(OLED_MOSI, OLED_CLK, OLED_DC, OLED_RESET, OLED_CS);

float vcc = 4.91;                       // only used for display purposes, if used
                                        // set to the measured Vcc.
float pad = 9850;                       // balance/pad resistor value, set this to
                                        // the measured resistance of your pad resistor
float thermr = 10000;                   // thermistor nominal resistance

float Thermistor(int RawADC) {
  long Resistance;  
  float Temp;  // Dual-Purpose variable to save space.

  Resistance=((1024 * pad / RawADC) - pad);
  Temp = log(Resistance); // Saving the Log(resistance) so not to calculate  it 4 times later
  Temp = 1 / (0.001129148 + (0.000234125 * Temp) + (0.0000000876741 * Temp * Temp * Temp));
  Temp = Temp - 273.15;  // Convert Kelvin to Celsius                      


  return Temp;                                      // Return the Temperature
}
int Relay1 = 30; 
int Relay2 = 32;
int Relay3 = 34;
int Relay4 = 36;
int Relay5 = 38;
int Relay6 = 40;
const int key1 = 11;
const int key2 = 10;
const int key3 = 9;
const int key4 = 8;
const int key5 = 7;
const int key6 = 6;
int PowerOff = 5;
int PowerOn = 0;
int s1 = 0;
int s2 = 0;
int s3 = 0;
int s4 = 0;
int s5 = 0;
int s6 = 0;
boolean toggle = false;

void setup() {
  pinMode(Relay1, OUTPUT);      // sets the digital pin as output
  pinMode(Relay2, OUTPUT);
  pinMode(Relay3, OUTPUT);
  pinMode(Relay4, OUTPUT);
  pinMode(Relay5, OUTPUT);
  pinMode(Relay6, OUTPUT);
  pinMode(key1, INPUT_PULLUP);
  pinMode(key2, INPUT_PULLUP);
  pinMode(key3, INPUT_PULLUP);
  pinMode(key4, INPUT_PULLUP);
  pinMode(key5, INPUT_PULLUP);
  pinMode(key6, INPUT_PULLUP);
  pinMode(PowerOff, INPUT_PULLUP);
  digitalWrite(Relay1, HIGH);   
  digitalWrite(Relay2, HIGH);
  digitalWrite(Relay3, HIGH);
  digitalWrite(Relay4, HIGH);
  digitalWrite(Relay5, HIGH);
  digitalWrite(Relay6, HIGH);
  
  display.begin(SSD1306_SWITCHCAPVCC);
    display.clearDisplay();

}
void loop() {
  
 if (s1 == 0){
   if (digitalRead(key1) == LOW){
    PowerOn = 1;
    s1 = 1;
    delay (200);
    
    }
   }
  if (s2 == 0){
   if (digitalRead(key2) == LOW){
    PowerOn = 1;
    s2 = 1;
    delay (200);
    
    }
   }
  if (s3 == 0){
   if (digitalRead(key3) == LOW){
    PowerOn = 1;
    s3 = 1;
    delay (200);
   
    }
   }
  if (s4 == 0){
   if (digitalRead(key4) == LOW){
    PowerOn = 1;
    s4 = 1;
    delay (200);
   
    }
   }
  if (s5 == 0){
   if (digitalRead(key5) == LOW){
    PowerOn = 1;
    s5 = 1;
    delay (200);
    
    }
   }
  if (s6 == 0){
   if (digitalRead(key6) == LOW){
    PowerOn = 1;
    s6 = 1;
    delay (200);
    
    }
   }
  if (digitalRead(PowerOff) == LOW){
    PowerOn = 0;
    display.begin(SSD1306_SWITCHCAPVCC);
    display.clearDisplay();
    display.setTextSize(3);
    display.setCursor(0, 0);
    display.print(" ");
    display.display();
    digitalWrite(Relay1, HIGH);
    digitalWrite(Relay2, HIGH);
    digitalWrite(Relay3, HIGH);
    digitalWrite(Relay4, HIGH);
    digitalWrite(Relay5, HIGH);
    digitalWrite(Relay6, HIGH);
    s1=0;
    s2=0;
    s3=0;
    s4=0;
    s5=0;
    s6=0;
   }
   //if (PowerOn == 1){
     float temp;
    temp=Thermistor(analogRead(ThermistorPIN));       // Get Raw thermistor value
    temp = (temp * 9.0)/ 5.0 + 32.0;                  // converts to  Fahrenheit
    display.begin(SSD1306_SWITCHCAPVCC);
    display.clearDisplay();
    display.setTextSize(3);
    display.setCursor(32, 8);
    display.setTextColor(WHITE);
     if (s1 == 1){
      digitalWrite(Relay1, LOW);
      display.print(temp,1);
     
      }
      if (s2 == 1){
       digitalWrite(Relay2, LOW);
       display.print(temp,1);
       
      }
      if (s3 ==1){
       digitalWrite(Relay3, LOW);
       display.print(temp,1);
      
      }
      if (s4 ==1){
       digitalWrite(Relay4, LOW);
       display.print(temp,1);
      
       }
      if (s5 ==1){
       digitalWrite(Relay5, LOW);
       display.print(temp,1);
       }
      if (s6 ==1){
       digitalWrite(Relay6, LOW);
       display.print(temp,1);
      
  }
      display.print(temp);
      display.display();
//}
 gettemp();
 checkoffstate();

}
void gettemp(){
  
  if (PowerOn == 1){
  float temp;
    temp=Thermistor(analogRead(ThermistorPIN));       // Get Raw thermistor value
    temp = (temp * 9.0)/ 5.0 + 32.0;                  // converts to  Fahrenheit
    display.begin(SSD1306_SWITCHCAPVCC);
    display.clearDisplay();
    display.setTextSize(3);
    display.setCursor(32, 8);
    display.setTextColor(WHITE);
    display.print(temp,1);
    display.display();
    delay(500);
  }
 }
void getkey(){
   if (s1 == 0){
    if (digitalRead(key1) == LOW){
    PowerOn = 1;
    s1 = 1;
    delay (100);
    }
 }
  if (s2 == 0){
    if (digitalRead(key2) == LOW){
    PowerOn = 1;
    s2 = 1;
    delay (100);
    }
 }
 if (s3 == 0){
    if (digitalRead(key3) == LOW){
    PowerOn = 1;
    s3 = 1;
    delay (100);
    }
 }
 if (s4 == 0){
    if (digitalRead(key4) == LOW){
    PowerOn = 1;
    s4 = 1;
    delay (100);
    }
 }
 if (s5 == 0){
    if (digitalRead(key5) == LOW){
    PowerOn = 1;
    s5 = 1;
    delay (100);
    }
 }
 if (s6 == 0){
    if (digitalRead(key6) == LOW){
    PowerOn = 1;
    s6 = 1;
    delay (100);
    }
 }
}
void checkoffstate(){
  if (s1 == 1){ 
    if (digitalRead(key1) == LOW){
    digitalWrite(Relay1,HIGH);
    s1 = 0;
    delay (200);
    }
  }
  if (s2 == 1){ 
    if (digitalRead(key2) == LOW){
    digitalWrite(Relay2,HIGH);
    s2 = 0;
    delay (200);
    }
  }
  if (s3 == 1){ 
    if (digitalRead(key3) == LOW){
    digitalWrite(Relay3,HIGH);
    s3 = 0;
    delay (200);
    }
  }
  if (s4 == 1){ 
    if (digitalRead(key4) == LOW){
    digitalWrite(Relay4,HIGH);
    s4 = 0;
    delay (200);
    }
  }
  if (s5 == 1){ 
    if (digitalRead(key5) == LOW){
    digitalWrite(Relay5,HIGH);
    s5 = 0;
    delay (200);
    }
  }
  if (s6 == 1){ 
    if (digitalRead(key6) == LOW){
    digitalWrite(Relay6,HIGH);
    s6 = 0;
    delay (200);
    }
  }
}

Any help would be greatly appreciated. I know my code is not even close to optimal. I've written and re-written it about 50 times and I'm getting totally frustrated since this should be easy in my mind. :slight_smile:

First of all: loose the delays.
Keyphrase: Blink without delay.

I understand why you put those delays in there but in total that's a lot of delay.
You are initialising the display in your loop.
Is that what you need to do ?

Hello Mas3,

I agree I should loose the delays.
I would rather not initialize the display in the loop, but when I don't, the display pops up then goes blank.
the way the code is now, the display somewhat works, but has a steady "blink" to it.
All I want to do is when any button press sets an output on, the display turns on and reads the live water temperature. Once the display is on I still need to be able to turn on/off other outputs via the other buttons.

Thanks for the reply and any help.

Did you try what happens if you comment out the display part in your powerOff section ?
So skipping the erasing of your screen.
If that helps, try using other values than 0 and 1 for powerOn and powerOff.

I'll give that a shot when I get home. I'll also redo the code to get rid of all the delays.
Once I'm done I'll re-post my code and any results.

Thanks

ok, I've re-written my code. Now everything is working better, but the display still continually blinks. It will blink at the rate The millis interval is setup for. So If I have 100ms set that's the rate the display will flash.

In my display portion of the code I tried commenting out the clear.display but then I get the adafruit logo popping up and the display doesn't clear. Am I just not setting up the display code correctly?
Below is my re-written code:

#include <Keypad.h>
#include <Wire.h>
#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>
#include <math.h>

#define OLED_DC 19
#define OLED_CS 17
#define OLED_CLK 20
#define OLED_MOSI 21
#define OLED_RESET 18
#define ThermistorPIN A1                 // Analog Pin 1
Adafruit_SSD1306 spidisplay(OLED_MOSI, OLED_CLK, OLED_DC, OLED_RESET, OLED_CS);

float vcc = 4.91;                       // only used for display purposes, if used
                                        // set to the measured Vcc.
float pad = 9850;                       // balance/pad resistor value, set this to
                                        // the measured resistance of your pad resistor
float thermr = 10000;                   // thermistor nominal resistance

float Thermistor(int RawADC) {
  long Resistance;  
  float Temp;                           // Dual-Purpose variable to save space.

  Resistance=((1024 * pad / RawADC) - pad);
  Temp = log(Resistance);               // Saving the Log(resistance) so not to calculate  it 4 times later
  Temp = 1 / (0.001129148 + (0.000234125 * Temp) + (0.0000000876741 * Temp * Temp * Temp));
  Temp = Temp - 273.15;                 // Convert Kelvin to Celsius                      

 
  return Temp;                          // Return the Temperature
}


// variables will change:
int buttonState = 0;                    // variable for reading the pushbutton status
int setdisplayoff =1;


const byte ROWS = 7;                    //seven rows
const byte COLS = 1;                    //one columns
char keys[ROWS][COLS] = {
	{'1'},
	{'2'},
	{'3'},
	{'4'},
        {'5'},
        {'6'},
        {'7'},
	};
byte rowPins[ROWS] = {11, 10, 9, 8, 7, 6, 5}; //connect to the row pinouts of the keypad
byte colPins[COLS] = {4};                     //connect to the column pinouts of the keypad
	
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );
int Relay1 = 30; 
int Relay2 = 32;
int Relay3 = 34;
int Relay4 = 36;
int Relay5 = 38;
int Relay6 = 40;
int displaystate = 14;
long previousMillis = 0;
long interval = 100;

void setup(){
  Serial.begin(9600);
  pinMode(Relay1, OUTPUT);      // sets the digital pin as output
  pinMode(Relay2, OUTPUT);
  pinMode(Relay3, OUTPUT);
  pinMode(Relay4, OUTPUT);
  pinMode(Relay5, OUTPUT);
  pinMode(Relay6, OUTPUT);
  pinMode(displaystate, OUTPUT);
  digitalWrite(Relay1, HIGH);   
  digitalWrite(Relay2, HIGH);
  digitalWrite(Relay3, HIGH);
  digitalWrite(Relay4, HIGH);
  digitalWrite(Relay5, HIGH);
  digitalWrite(Relay6, HIGH);
  digitalWrite(displaystate, LOW);
  keypad.addEventListener(keypadEvent); //add an event listener for this keypad
}
  
void loop(){
  unsigned long currentMillis = millis();
  unsigned long currentMillis1 = millis();
 
  char key = keypad.getKey();
  
    if(currentMillis - previousMillis > interval) {
    previousMillis = currentMillis;  
    if (buttonState == HIGH){
    getTemp();
    }
      if (buttonState == LOW){
      digitalWrite(displaystate,LOW);
      }
    }
 }



void keypadEvent(KeypadEvent key){             //get keypad events
  switch (keypad.getState()){
    case PRESSED:
      switch (key){
        case '1': digitalWrite(Relay1,!digitalRead(Relay1));
                  buttonState = HIGH;
                  
                  break;
        case '2': digitalWrite(Relay2,!digitalRead(Relay2));
                  buttonState = HIGH;
                  
                  break;
        case '3': digitalWrite(Relay3,!digitalRead(Relay3));
                  buttonState = HIGH;
                  
                  break;
        case '4': digitalWrite(Relay4,!digitalRead(Relay4));
                  buttonState = HIGH;
                  
                  break;
        case '5': digitalWrite(Relay5,!digitalRead(Relay5));
                  buttonState = HIGH;
                  
                  break;
        case '6': digitalWrite(Relay6,!digitalRead(Relay6));
                  buttonState = HIGH;
                  
                  break;
        case '7': buttonState = LOW;
                  digitalWrite(Relay1, HIGH);   
                  digitalWrite(Relay2, HIGH);
                  digitalWrite(Relay3, HIGH);
                  digitalWrite(Relay4, HIGH);
                  digitalWrite(Relay5, HIGH);
                  digitalWrite(Relay6, HIGH);
                  break;
      }
    break;
  }
}
void getTemp() {
  digitalWrite(displaystate, HIGH);
  float temp;
  temp=Thermistor(analogRead(ThermistorPIN));       // read ADC and  convert it to Celsius
  temp = (temp * 9.0)/ 5.0 + 32.0;                  // converts to  Fahrenheit
  
  if (temp <= 72.0){
  spidisplay.begin(SSD1306_SWITCHCAPVCC);
  spidisplay.clearDisplay();
  spidisplay.setTextSize(2);
  spidisplay.setTextColor(WHITE);
  spidisplay.setCursor(0,0);
  spidisplay.println("Cold:");
  spidisplay.setCursor(64,0);
  spidisplay.println(temp,1);
  spidisplay.display();
  
  }
  if (temp > 72.0){
  spidisplay.begin(SSD1306_SWITCHCAPVCC);
  spidisplay.clearDisplay();
  spidisplay.setTextSize(2);
  spidisplay.setTextColor(WHITE);
  spidisplay.setCursor(0,0);
  spidisplay.println("warm:");
  spidisplay.setTextSize(2);
  spidisplay.setCursor(64,0);
  spidisplay.println(temp,1);
  spidisplay.display();
  
  }
}

This code is total different from the first we saw.
Now you're using a keypad approach to get the key, but you chose to have 1 column of 7 rows.
Easier to wire and to handle perhaps.
But you are using 7 or 8 pins now (depends on the way you wired it up).
You could have chosen 3 rows and 3 columns to have 7 to 9 keys to press and save a pin.

You're writing to the display in void.getTemp.
I guess the name should have been void.handleTemp.
Point is to get your value in void.getTemp, and handle it in your void.loop, if you're writing a longer sketch this will help you to keep track of what's going on.
But all this is not really a problem, the code will still work.

You should know why the display blinks as that has been explained.

The reason I used 1 row 7 columns is because that is what the keypad actually looks like. I use six keys to turn on/off valves and one key as a shutdown turning everything off including the display.
I thought I'd take a different approach at how I was writing the code hence the complete re-write.

Also, Yes I know why the display is blinking, because it only runs when I jump to the 'gettemp' routine.
My question,and where I'm stuck is:
1.Where should I put the display code to keep it active and still be able to recognize a key press?
2. Does the display code look correct? should it be written differently?
3. Is the way I've re-written the code to use keypad events better or worse than the first way I wrote the code?

If I put the display code in the main loop the key presses are never recognized.
I am hoping you or someone could give me some sort of short example?

I know I must be close, I just need to get across the finish line.

I don't know why your display just flashes.
You told that you have to use "spidisplay.begin(SSD1306_SWITCHCAPVCC);" and " spidisplay.clearDisplay();" every time you write to your screen to get any result at all.
I've been told that the clearDisplay command is slow because it writes spaces one by one.
I've had a lot of trouble to get my 128*64 OLED screen with the SSD1306 (in I2C mode) to work, but never saw the screen erased for no reason.
Data stays as long as i don't overwrite it.

You have divided your sketch in different sections, but the way you did that doesn't make a lot of sense to me.
In void.getTemp, you are not only fetching the temperature, but are also processing it.
I would get the temperature and eventually convert it, but the processing would be done in void.loop
In void.loop, you are declaring currentMillis and currentMillis1 over and over again.
You could declare them in void.setup, and just renew them in void.loop, but i don't know if that will be any improvement for your sketch.

Also i see you made void.keypadEvent, but i don't see you call it.
As i've never played with keypads yet, i can't tell how well you did with that.