Loop running slow

First a quick overview.

I have a sketch I have been developing for my beer brewing process.
It has two DS18B20 temp probes, one output for a transfer pump and one output for an electric heater element (not actually in use yet, just setting up the code).
Originally the pump output was a PWM but realized I didn’t need it so now it is just on or off.
The heater output is (will be) a PWM.

Recently I added an IR remote to replace mechanical buttons. It all seemed to be working fine but last weekend when I tried to use it, I noticed the response from the remote to be very slow. I had to hit the button many times before it would register.

If I hold the button, it should index up or down depending on the button. I would have to continually hit the button until it registered once, then I could hold it down and it would continue to change.

This is when I realized that the loop was running slow because each index would take about a second or more. In earlier versions of my code, it scrolled pretty fast.

Obviously there are some additions and upgrades in this most recent version but didn’t think it was anything that should cause it to slow down this bad.

Code is attached. Couldn’t insert, longer than 9000 characters. What in here could be causing it to run so slow?

Thanks for any help.

Mikes_HERMS_v2.2.ino (10.5 KB)

are you sure about the source code? seems the loop() function is within the setup() function from a {} perspective....

  probe1 = (sensorValue(HLTTMP));
  probe2 = (sensorValue(MLTTMP));

(What) (are) (the) (useless) (parentheses) (for)?

How is anyone supposed to guess what sensorValue() does?

float sensorValue(DeviceAddress deviceAddress)
{
  float tempC = sensors.getTempC(deviceAddress);
  return tempC;
}

Why in heavens name do you have a function that does nothing more than call a different function?

  //pumppct=((Output/255.0)*100.0);  //Convert output to percent of power. Decimal is required
  analogWrite(mltpmp, Output);

  /******************Set HLT Heater output ***********************/

  myPID.Compute();

Apply Output to the pump, then compute a value for Output. I give up. Why?

If I hold the button, it should index up or down depending on the button.

My remote sends 0xFFFFFFFF as the repeat code. You ignore that value. So, I disagree with your conclusion, unless your remote sends a different repeat code.

 while (irrecv.decode(&results)==LOW) {

I think it very unlikely that the decode() function returns a LOW.

Wow, thank you for the reply. I guess I have some work to do.

A lot of this was copied from examples or other peoples sketches that I found in google searches.

Like the useless parentheses, that was how I found it in the example, just didn't know any better I guess.
Same with "sensorValue". Copied from an instructable or somewhere, can't remember.

Never could understand the "float" function. Again, copied from somewhere else. I couldn't even see that it was a function calling a function. Any suggestion to improve?

I thought I had the repeat code covered here:

//*********** Read button presses from remote ****************

if(irrecv.decode(&results))
{
if(results.value != 0xFFFFFFFF)
{lastcode=results.value;}

Did I do that wrong too?

The pump output conversion was just to convert the 0-255 into a 0% to 100%. Thought it would be easier to read on the display.

Surprisingly enough, except for being slow and hard to get the button press to register, it does work.
I guess the people I have been copying haven't done me any favors.

I had read elsewhere that DigitalWrite can slow things down but I am currently not even using one, analogwrite yes but couldn't find anything bad about that listed in my searches.

Do you think that the float function is my culprit?

Thanks again for your help

J-M-L:
are you sure about the source code? seems the loop() function is within the setup() function from a {} perspective....

That is the way it is running right now and it does work (albeit slow).
I have robbed from so many different examples good or bad that I am sure there is a lot of room for improvement.

I am such a newbie at this that I don't even realize some of these errors.
Especially if it works, then it doesn't push me to try to fix it.

The function is sensorValue(), it returns a float.

Why not just do

probe1 = sensors.getTempC(HLTTMP)

All those LCD calls will together, slow things down. I doubt if you need to do all of them in every iteration of loop().

The calls to the Dallas temperature sensor can be slow (based on reading other Threads) but I think that is just due to the poor way the library has been written. I believe it hangs around waiting for an answer instead of starting a read and coming back later for the data. And there may be no need to read the temperature in every iteration of loop().

The call to PID.compute() can also take an appreciable time because it uses floating point maths.

...R

Thanks for the input.

I have already made the "probe1 = sensors.getTempC(HLTTMP)" change.
What affect does that have on the function at the bottom? Don't understand that thing in the first place so not sure how to edit it. Won't be able to test it until I get home but looking forward to it.

Robin2, I can agree that I probably don't need to refresh the screen or even update the PID in every loop. Assuming I can get the speed back up. Would I just use a loop counter and only refresh every so many loops or is there a better way of doing that?

I looked for a way to CONCENATE prints to the LCD but didn't look like that was a good idea. Besides not updating as often, is there another way to improve the writing to LCD?

I am also a C++ and Arduino newbie, and also fell into the trap of thinking that I'd find ready-made sketches to do what I wanted.

Big mistake.

Most sketches I tried would not even compile, and others used libraries that would make your brain hurt: not a comment in sight, and the phrase "Flow Chart" did not appear to exist.

10,000 characters seems a lot of code.

If you go back a step and describe the beer brewing process using flow chart techniques, on 2 A4 sheets MAX of paper, you will have a dozen "boxes" and decisions each of which should only take a few lines of C++.

The other thing is to start off with a working hardware circuit diagram for the manually controlled brewery, showing where the Arduino and associated hardware comes in.
If you could post that, it would make it much easier to figure out what code you would need.

Once you have it working, I will be your first customer.

Hi Pat,

A quick answer without doing the flow charts;
The sketch has 422 lines but a lot of that is actually comments.
There is also a few lines devoted to custom characters.
Another big piece is writing data to the LCD.
The real "meat" of the code I don't think is that big.

Like you said and others have pointed out, the code I copied from others isn't the most efficient way of accomplishing the task. Just don't know any better yet.

Besides the temperature probes, the only hardware right now is a 12v pump that circulates wort between two vessels. There is some code for a heating element but the hardware doesn't exist yet.

Functionally, I am monitoring temperature in two vessels and based on the difference between set point and actual, turn the pump on and off. That is basically it in a nutshell.

Why circulate the wort? Vessel 1 (HLT) temperature is controlled manually with a turkey fryer burner (future will be electric element). If the temperature in vessel 2 (MLT) drops below the set point (no heat on this vessel), the pump turns on circulating the liquid through a copper coil immersed in the water in the HLT which is currently maintained at a slightly higher temperature. Hence the desire to switch to electric and have the Arduino maintain with a PID. When the temperature reaches set point, the pump turns back off.

Through the code and the IR remote, I can adjust the set points for both vessels. Adjusting vessel 1 right now has no purpose until I switch to electric heat though, just getting it set up.

I am always open to suggestions.
Thanks

m-hickey95:
Robin2, I can agree that I probably don't need to refresh the screen or even update the PID in every loop. Assuming I can get the speed back up. Would I just use a loop counter and only refresh every so many loops or is there a better way of doing that?

You could do that.

Or you could use millis() to manage timing without blocking as illustrated in several things at a time

However, as well as not updating the LCD in every loop() you may also want to fragment the update so parts of it happen in different iterations of loop() rather than taking the time hit required to do it all at once. You may need to do some experiments to determine if that would be an advantage.

Another thing to consider is ensuring that only one of time-consuming activities happens in any one iteration of loop(). For example in an iteration where the LCD is updated there will be no call to the temperature sensors and no call to PID.

...R
Planning and Implementing a Program

Thanks Robin. Interesting ideas.

I have been studying your SeveralThingsAtTheSameTime sketch to figure out the millis() functionality. Haven't used it before but I did plan on adding it to the next version to include timer and auto temperature shift capabilities.

Looks like I have a bit of moving and adjusting to get everything to run at different times.

It did make me realize that my pump control (mltpmp) is set up as a PWM but I am no longer using it as such, just turning it on and off. In terms of processing speed, which would be faster, the PWM (analogwrite) or switch back to digitalwrite high or low?

Robin2:
All those LCD calls will together, slow things down. I doubt if you need to do all of them in every iteration of loop().

The calls to the Dallas temperature sensor can be slow (based on reading other Threads) but I think that is just due to the poor way the library has been written. I believe it hangs around waiting for an answer instead of starting a read and coming back later for the data. And there may be no need to read the temperature in every iteration of loop().

The call to PID.compute() can also take an appreciable time because it uses floating point maths.

...R

I can confirm the Dallas temp sensor reading delay, Robin. I just finished my drying oven controller and we computed the temp reading was taking 250-300 ms. to complete. I now read the temp once per second and have plenty of time to do other things during the remaining part of the second.

Paul

Thanks for the confirmation Paul. That is the first one I am moving. Come to think of it, I think it was after switching from thermistor to the Dallas sensors is when I first started noticing the slowdown.

I have set it for 1000millis for the first test. Will try it out after I get home from work.

Now incorporating the other tasks at different intervals, that will take a little more thought.

If I wanted two different functions to run on the same interval but maybe 500 millis apart, how would I get that started?

Start the first one, note the time (with millis (), of course), start the second 500 milliseconds later

m-hickey95:
If I wanted two different functions to run on the same interval but maybe 500 millis apart, how would I get that started?

I don't understand what you mean by "same interval" when they are 500 msecs apart because 500 msecs is an awfully long time for an Arduino.

...R

Paul_KD7HB:
I can confirm the Dallas temp sensor reading delay, Robin. I just finished my drying oven controller and we computed the temp reading was taking 250-300 ms. to complete. I now read the temp once per second and have plenty of time to do other things during the remaining part of the second.

Paul

Assuming that you are using the DallasTemperature library, you can use setWaitForConversion to eliminate the delay. You will have to request temperature and use millis to give the sensor enough time to collect it before reading it, but during that time you can be doing other things.

I hope someone is still reading this.

I am totally screwed. Even after implementing some of the timing suggestions, still didn’t work.
I thought I traced it to the temp probes but after disabling them, still didn’t work.

Went back to the drawing board and put together test button presses.
Nothing would register.

Looks like it is in the switch case section. I commented it out and put in if’s to read the buttons and it started working.

Any idea what’s wrong with the switch case method?

I also noticed that while no button is being pressed, random codes appear to be received as displayed in the fourth line of the data screen.

Another puzzler is the menu button goes back to the Splash screen but doesn’t pause like it does at reset or power up. It immediately goes on to the data screen.

#include <IRremote.h>
#include <Wire.h>
#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE); //Set LCD I2C address

int recv_pin=11;   //IR receive input pin
IRrecv irrecv(recv_pin);
decode_results results;
unsigned long lastcode;
int volup =0;
int voldn=0;
int chaup=0;
int chadn=0;
int right=0;
int left=0;

//**********************Void Setup*********************************
void setup() {
  irrecv.enableIRIn();  //Start the IR receiver
  lcd.begin(20, 4);
  splash();

}
//****************************************************************************
//                                Void Loop
//****************************************************************************
void loop() {

  //*********** Read button presses from remote ****************
  if(irrecv.decode(&results)){  
    if(results.value != 0xFFFFFFFF)
    {lastcode=results.value;}
 
  /*switch (lastcode) { //Adjust HLT or MLT setpoint based on menu value (Case select)
    case 0x2A71BFFD:  // Menu button, back to splash screen
      splash();
      break;
    case 0x636d99da:  //Right arrow
      right++;
      break;
    case 0xD20E2899:  //Left arrow
      left++;
      break;
    case 0x22d912bb:   // Volume up button
      volup++;
      break;
    case 0x776c6e7a:  // Volume down button
      voldn--;
      break;
  }
  irrecv.resume(); 
  } 
 */

  if(lastcode==0x22d912bb){   //5846514513  0x22d912bb
    volup++;
  }
  if(lastcode==0x776c6e7a){
    voldn++;
  }
  if(lastcode==0x636d99da){
    right++;
  }
  if(lastcode==0xD20E2899){
  left++;
}
  if(lastcode==0x2A71BFFD){
    splash();
  }
 irrecv.resume(); 
  } 

PrintData();  
}

//******************Below are Functions called from above******************************************

void PrintData(){ 
  // First Row
  lcd.setCursor(6, 0);
  lcd.print(volup);
  lcd.setCursor(17, 0);
  lcd.print(voldn);

  // Second Row
  lcd.setCursor(6, 1);
  lcd.print(chaup);
  lcd.setCursor(17, 1);
  lcd.print(chadn);

  // Third Row
  lcd.setCursor(6, 2);
  lcd.print(right);
  lcd.setCursor(17, 2);
  lcd.print(left);

  // Fourth Row
  lcd.setCursor(0,3);
  lcd.print(lastcode);
}

void Static(){
lcd.clear();
  lcd.home();
  // First Row
  lcd.setCursor(0, 0);
  lcd.print("Vol +");
  lcd.setCursor(10, 0);
  lcd.print("Vol -");

  // Second Row
  lcd.setCursor(0, 1);
  lcd.print("Cha +");
  lcd.setCursor(10, 1);
  lcd.print("Cha -");

  // Third Row
  lcd.setCursor(0, 2);
  lcd.print("Right");
  lcd.setCursor(10, 2);
  lcd.print("Left");
}

void splash(){
lcd.clear();
  lcd.home();
  lcd.setCursor(4, 0);
  lcd.print("WELCOME TO");
  lcd.setCursor(18, 0);
  lcd.write(3); //Upper left logo
  lcd.write(4); //Upper right logo
  lcd.setCursor(3, 1);
  lcd.print("Hickey Brewing");
  lcd.setCursor(18, 1);
  lcd.write(5); //Lower left logo
  lcd.write(6); //Lower right logo
  lcd.setCursor(3, 2);
  lcd.print("press any key");
  lcd.setCursor(0, 3);
  lcd.print("v2.2   to start");
 //This will pause program until a button is pressed, then continue with void loop.
  while (irrecv.decode(&results)==LOW) {
    delay(10);
  }
irrecv.resume();
    Static();
}

WildBill,
I am using the DallasTemperature Library. I read something on that this morning. Need to work on that too. I also realized I had the resolution of the probes set to the slowest setting which supposedly can take up to 750ms to read. I am reading two probes so assume that could be double that.

Found another problem with just reading the button presses which I posted a few minutes ago. I hate having to go back to the drawing board after getting so far along.

I don't see a single Serial.print() in your program to display values for debugging purposes or even to allow you to see which parts of the code are being reached.

And, if it does not affect the functionality of the program, it can be useful for debugging to temporarily put a big delay(500) in loop() so that you can see the values as they happen without them scrolling off the screen.

...R