Making this code more efficient and smaller... Possible?

Hi

I need some help to understand and catch up more smarter code and functions to make my code smaller and more time efficient. As my project will grow with more sensors and functions to controll it is vital that I can speed up as much as it is possible… For this code I’m using only 2 dallas ONeWire but when finished I will have 4 temp sensors, 2 waterflow sensors, 2 waterpump sensors and 8 fans both to controll with 25KHz PWm and reading their rpm, all this should be dislpayd onto a LCD and data will be stored for later reviewing … So that’s why I wants to learn to code smart and efficient from the beginning :smiley:

Come with advice :d

// Read and display temperatures with SPI based LCD and to serial monitor
// Johnny Lindberg
// Libs used
#include <SPI.h>
#include <LiquidCrystal.h>
#include <DallasTemperature.h>
#include <OneWire.h>
// Define witch Pin Dallas temp is connected to
#define ONE_WIRE_BUS 8

// Variables and sensors, timers and other
OneWire oneWire(ONE_WIRE_BUS);
DallasTemperature sensors(&oneWire);
LiquidCrystal lcd(10);
char* textArray[] = {"C Kylaren! ", "C Grafiken!", "  Powering UP!  ", "  White Rabbit  "};
float tempArray[2];
unsigned long Timer1;
unsigned long Timer2;
int lcdTimerIntVal = 5000;
int tempTimerIntVal = 4000;
char serByte;
boolean coldStart = true;

// Setup all start
void setup()
{
  lcd.begin(16, 2);
  sensors.begin();
  Serial.begin(9600);
}

// Main loop
void loop()
{
  // Do a coldstart routine to display temp without delay
  if(coldStart == true)
  {
    lcdPrintStart();
    delay(3000);
    readTemp();
    lcdPrintTemp();
    coldStart = !coldStart;
  }
  // Call Timers to read and display temperature on LCD
  tempTimer();
  lcdTimer();
  // Check if Serial read is incoming and send back temp value
  if(Serial.available())
  {
    serByte = Serial.read();
    // Check if it is the correct number recived
    if(serByte == '1')
    {
      // Send temp back to Serial monitor
      Serial.print(tempArray[0]);
      Serial.println(textArray[0]);
    }
    else if(serByte == '2')
    {
      Serial.print(tempArray[1]);
      Serial.println(textArray[1]);
    }
  }
}
    
// LCD Timer
void lcdTimer()
{
  if(millis() - Timer1 > lcdTimerIntVal)
  {
    lcdPrintTemp();
    Timer1 = millis();
  }
}

// Temp Timer
void tempTimer()
{
  if(millis() - Timer2 > tempTimerIntVal)
  {
    readTemp();
    Timer2 = millis();
  }
}

// Read Dallas temperature module
void readTemp()
{
  for(int x = 0; x < 2; x++)
  {
    sensors.requestTemperatures();
    tempArray[x] = sensors.getTempCByIndex(x);
  }
}

// LCD Printer Temperature function
void lcdPrintTemp()
{
  for(int x = 0; x < 2; x++)
  {
    lcd.setCursor(0, x);
    lcd.print(tempArray[x]);
    lcd.setCursor(5, x);
    lcd.print(textArray[x]);
  }
}

// LCD Printer Start Function
void lcdPrintStart()
{
  for(int x = 2; x < 4; x++)
  {
    lcd.setCursor(0, x - 2);
    lcd.print(textArray[x]);
  }
}
  // Do a coldstart routine to display temp without delay
  if(coldStart == true)
  {
    lcdPrintStart();
    delay(3000);

How ironic. I quit reading here.

NOt putting code like this in loop!

  if(coldStart == true)
  {
    lcdPrintStart();
    delay(3000);
    readTemp();
    lcdPrintTemp();
    coldStart = !coldStart;
  }

Mark

to make my code smaller and more time efficient

Often you'll find that larger code executes faster.

@holmes4 That part of the code is only done once. Its a lock out, so its not really slowing down his code.

What puzzles me is this why didn't the OP just put that in his setup function. Edit.

What puzzles me is this why didn't the OP just put that in his setup function and also this part here,

Precisely that reason I ask you all to help me :P It is easy when learning to overlook some smart and obvious ways to code it :D Well as HazardsMind explained the coldStart routine is just a way to check all sensors and do a initial reading of them all to have something to display on the LCD. That code doesn't really slow the program down... I need it when all sensors and fans are attached.

But HazardsMind I not quite following you about the Serial check routine, I use that to read the values from the arduino to my serialmonitor but only if I wants to and also with of the values I need ... Could I write it in some other way?

No sorry, at first I thought I saw both lines saying tempArray[0], because I saw "te" and "Array[0]", it wasnts until I posted my reply did I notice that one was [u]temp[/u]Array and the other was [u]text[/u]Array. After that, I removed it.

I don't see any glaring inefficiencies. The biggest problem may be those oneWire sensors, which are relatively slow and "atomic" to read, as far as I know. But reading four of them every 4 or five seconds shouldn't be a problem.

MADZerQ: Come with advice :d

For what it's worth a few thoughts (in no real order):

  • If you don't know about PROGMEM, check out http://arduino.cc/en/Reference/PROGMEM and put your text array in there (and use the 'F' macro for any literal text that you use). Will help save memory at run-time.
  • Put the timer values in an array like you did the temperatures, then combine tempTimer and lcdTimer. You could then cycle thru the array to check to see if it was time to do something. Would help reduce code if you add more timers (I'd think you would eventually).
  • The code to handle serial I/O is pretty.... simplistic and I don't think will scale very well. You might want to use something like http://www.gammon.com.au/forum/?id=11425 or check out PaulS' posts, he has a really nice serial handler that he posts from time to time.
  • You might be surprised how fast even real bad code can run as long as you avoid things like delay ;)

Regards,

Brad KF7FER

That code doesn't really slow the program down... I need it when all sensors and fans are attached.

Never the less put it in setup(), that's where it belongs and it is a little clearer.

Last I looks those temp sensors needed 750mS to provide a reading, does the library you are using just block until the reading is available? If so that will have a huge impact on your code's responsiveness, to the point of being unusable I would say, especially with more sensors.

One way to reduce the effect of this is to just read a single sensor at a time, not all 4 like you do now in readTemp(), that way at least loop() gets to function every 750mS not every 3 seconds as it will when you get 4 sensors.

The next step would be to come up with an algorithm that starts the reading and comes back 750mS later to get the value, but that would mean delving into the DallasTemperature library I suppose.


Rob