making sketches more efficient

Hi,

I am trying to make my code more efficient and smaller.

As an example of something I want improve; I have a simple counter that I display on an LCD using the LiquidCrystal_I2C.h library.
Is it more efficient to simply keep display the counter every time I loop or is it better to have an oldCounter val and only display if the newCounter != oldCounter?

Displaying the counter without checking to see if it has changed means the same value is being displayed 10+ times a second.

Which method uses the least amount of memory? I assume it's the one without the extra variable but I'm not sure.

Are there any resources you can point me at to help me learn more about making Arduino code more efficient?
Is there anywhere that shows how long certain commands/instructions take?

You need to define if you want it to be smaller, or faster. Usually they are mutually incompatible.

Also, why do you want it to be smaller? You have a fixed amount of memory, you can use all of it for no extra charge. :slight_smile:

There are some aesthetics involved too. When you refresh an LCD you will see it blink,
When you only update changed parts then the overall appearance will be more smooth.

Therefore

if (oldValue != newValue)
{
  oldValue = newValue;
  lcd.print(newValue);
}

is better (imho)

I have a couple scripts which are starting to run out of memory and are also in need of tidying up. Both use fairly large menu systems which have a lot of text. I have already done things like moving strings to program memory and would like to know more advanced techniques.

As part of a general tidy I started to make the code more aesthetic and I also started to experiment with trying to make it more efficient. Unfortunately I do not understand the Arduino or C enough to know the best solutions. Again, I would like to know more.

robtillaart:
There are some aesthetics involved too. When you refresh an LCD you will see it blink,
When you only update changed parts then the overall appearance will be more smooth.

Therefore

if (oldValue != newValue)

{
 oldValue = newValue;
 lcd.print(newValue);
}



is better (imho)

The code is not the issue; I use this oldValue/newValue a lot, especially with sensor readings. What I am trying to find out, is it more efficient to just update the LCD every loop or check to see if the value has changed before updating. I am using the LCD through the LiquidCrystal_I2C library but can't work out what overhead the library has compared to checking the value and updating less.

I think you just posted your comment inside the quote.

writing bits to an LCD takes time and depending on the protocol this will take far more time than comparing 2 values.

you can measure this easily with a testscript

The code is not the issue; I use this oldValue/newValue a lot, especially with sensor readings. What I am trying to find out, is it more efficient to just update the LCD every loop or check to see if the value has changed before updating. I am using the LCD through the LiquidCrystal_I2C library but can't work out what overhead the library has compared to checking the value and updating less.

What does efficient mean in this context? If you use the old/new technique, it will cost you more program memory and more RAM. As noted above, it will likely be faster and look better (no flickering). Obviously, it's a tradeoff. If a shortage of RAM is your primary concern, live with the flicker.

As ever, your best bet is to post your code - there may be other areas where savings can be made.

Other than the notoriously inefficient String class, and using flash instead of RAM, there isn't a whole lot said about efficiency in terms of space.

As far as speed goes, there has been a lot of analysis of timing code, because people like being accurate. So there are some people that are insanely knowledgeable about how a lot of it works. Two things help. Looking at the C code of the system, and looking at the Assembly code that is produced. Finding the C code isn't always easy, but with some time and grep, you can usually track down most things. It isn't always intuitive. digitalRead() and digitalWrite(), for example, would seem to be very fast straightforward tasks. However, there's some lookup code that maps Arduino pins to ATmega328 pins and it's a lot faster to do port manipulation. Looking at the Assembly code for timing is pretty easy, and for the most part the more instructions it turns out the longer it will take. The place that tells exactly how long they take is going to be in the datasheet of the ATmega328, under Instruction Set Summary. The column on the far right tells how many clocks each instruction takes. If you want to take the more practical approach, the testscripts that robtillart talks about can be made to be very accurate. You can easily use millis() and micros() to time how long things take. Take a time stamp, run the routine a couple thousand times, and take another time stamp.

Sand_HK:
The code is not the issue; I use this oldValue/newValue a lot, especially with sensor readings. What I am trying to find out, is it more efficient to just update the LCD every loop or check to see if the value has changed before updating. I am using the LCD through the LiquidCrystal_I2C library but can't work out what overhead the library has compared to checking the value and updating less.

As many others have said, which is more "efficient" depends on whether you're more concerned with speed or memory usage. Take two examples:

void loop() {
    val = digitalRead(SomePin);
    LCD.print(val);
}
void loop() {
      newVal = digitalRead(SomePin);
      if (newVal != oldVal) {
        LCD.print(newVal);
        oldVal = newVal;
    }
}

The first is far less efficient in terms of speed because you'll update the display needlessly every time you loop. That takes time to do. The second is less efficient in terms of memory because you have to store an additional variable (newVal and oldVal vs. just val), but it's faster. There's no memory difference in terms of the LCD routines. You'll execute all the LCD and string handling code and then whatever memory was consumed in the stack will be released, and you end up at a net change of 0 bytes (assuming no memory leaks) either way.

The one exception is if calling the "update" routine requires follow-up interrupt calls or whatever, but typically if something needs interrupts, it'll need them on a continual basis, or in a one-shot manner, and whatever memory those calls consume will either be consumed on a periodic (timer) basis regardless of what you do, or will need to be available for the one-shot (like receive buffers) whether it's in-use at any given moment or not. You may as well consider that amount indefinitely allocated for all practical purposes, as your program will crash if it isn't available.

You mentioned that your sketch uses an oldVal/newVal approach for other reasons anyway, so the update-when-necessary approach costs you nothing, and gains you some spare clock cycles if you ever need them. No difference WRT memory.

I have a couple scripts which are starting to run out of memory and are also in need of tidying up. Both use fairly large menu systems which have a lot of text.

The ATmega series of Arduino UC's are RISC based, so a 16MHz UNO has the same processor internal bandwidth as the Arduino Mega2560, only SRAM and Flash are increased in the 2560. My suggestion is to simply move up from your current model to a larger one. While the intellectual exercise involved with code analysis may be stimulating, such rewrites generally make code more difficult to maintain. If you were writing a public library, then the time and effort are warranted, but for your own skeleton and glue code, it will turn fun into work. You state you are not a powers programmer in C, so I suspect your C++ knowledge is in a similar category - which is why the Arduino apeals to so many; you do not need to be a power programmer to be productive in the environment.

If you are really serious about code efficiency, forget all the Arduino-centric commands and functions, and drop down to using raw C++ at the AVR-GCC level. AVR Libc Home Page
However, I personally think the movement up in the Arduino board line is the most efficient use of your time... The products are inexpensive, generally multi-sources, and very well supported. Bigger, Better, Happier :smiley:

Ray

This won't help you save any memory, but with regard to screen updating, here is what I like to do:

int screenUpdatePeriodMillis = 200; // update every 200 ms
unsigned long lastScreenUpdateMillis;
unsigned long currentMillis;

void updateScreen() {
  if (lastScreenUpdateMillis + screenUpdatePeriodMillis > currentMillis)
    return; // do nothing

  lastScreenUpdateMillis = currentMillis;
  // update LCD here
}

void loop() {
  currentMillis = millis(); // save having to make multiple calls
  updateScreen();
}

The reason that I like this approach is that it avoids checking every single possible variable that might have changed, to see if the screen needs to be updated. It just updates the screen every 200 ms (or whatever interval you choose). What if the screen goes longer than 200 ms without needing to be updated? Aren't we updating unnecessarily? Sure. But by the time the update period is 100+ ms, it's so long relative to the time of a loop() that who cares? I find that a 100-250 ms update frequency is plenty fast enough for many applications. Also, flicker is eliminated.

I'm sorry, but that example has quite a few issues in and itself :slight_smile:
See comments marked with "[int2str]" in your code example:

// [int2str] NEVER do this! Never write "200ms" in the comment.
// [int2str] That comment will be outdated and wrong in no time flat....
// [int2str] Also don't waste 4 bytes in a global variable here; #define UPDATE_PERIOD 200
int screenUpdatePeriodMillis = 200; // update every 200 ms

unsigned long lastScreenUpdateMillis;
unsigned long currentMillis;

void updateScreen() {
  // [int2str] This causes millis overflow issues. Subtract and compare, don't add...
  if (lastScreenUpdateMillis + screenUpdatePeriodMillis > currentMillis)
    return; // do nothing

  lastScreenUpdateMillis = currentMillis;
  // update LCD here
}

void loop() {
  // [int2str] Unless you need millis() for other things, using it just
  // [int2str] for this will cause the program to become larger for 
  // [int2str] questionable gains here. Also not fond of using a global
  // [int2str] variable that's checked inside the function.
  currentMillis = millis(); // save having to make multiple calls
  updateScreen();
}

int2str:

// [int2str] NEVER do this! Never write "200ms" in the comment.

// [int2str] That comment will be outdated and wrong in no time flat....
// [int2str] Also don't waste 4 bytes in a global variable here; #define UPDATE_PERIOD 200

Or better still:

const int screenUpdatePeriodMillis = 200; // how often we update (mS)

Thanks for the tips, Nick. I didn't think of overflow issues with the millis() compare. I'll make sure to subtract from now on. Regarding the update period, I always use constants for that sort of thing, but I flubbed it because I was "just whipping out code for an example." No excuse, though. Example code should still be good code.

Am I correct that "const int" (or whatever) works out functionally identical to #define? I believe that defines are basically string-substituted by the compiler and so take up absolutely no memory whatsoever. Is that also true for consts?

Yes, the compiler optimizes them away, the same as a literal in a define. We've shown before that they are functionally equivalent.

They are equivalent for most uses.
However, there are some things that can be done with #define that can't be done with const variables.
For example, you can't do things like conditional compilation based on the existence const variables.
i..e.

#define SWITCHPIN 3 // comment out to disable SWITCHPIN code

#ifdef SWITCHPIN
// code that only exists if SWITCHPIN is defined
#endif

--- bill

bperrybap:
However, there are some things that can be done with #define that can't be done with const variables.
For example, you can't do things like conditional compilation based on the existence const variables.

Absolutely. Conditional code is the only place where I use #defines, but boy do I use the heck out of it. All of my debug code is inside #ifdefs and I have probably fifteen #defines at the top of the program commented in or out depending on what I'm doing at any given moment. Once the code got to a certain level of done-ness, I also started using #ifdefs to make certain changes, so that if I wanted to revert the change, I could just undo the #define, rather than having to revert to a saved version or what have you.

void loop() {
  // [int2str] Unless you need millis() for other things, using it just
  // [int2str] for this will cause the program to become larger for 
  // [int2str] questionable gains here. Also not fond of using a global
  // [int2str] variable that's checked inside the function.
  currentMillis = millis(); // save having to make multiple calls
  updateScreen();
}

I had some thoughts about this. I said in the comment, "save having to make multiple calls," but honestly, the millis() function probably isn't much of a performance hog. How much slower is it than reading a variable, really? (I ask rhetorically, but I suspect you may actually know the answer.) The real reason I got started calling millis() like that at the beginning of my loop is that I may have several conditionals that rely on millis() and I don't want them comparing a different value, such as:

if (millis() - x > threshold1)  // do some stuff
else if (millis() - y < threshold2) // do some stuff
else if (millis() / modulo = 0) // do some stuff

You get the idea. In that case, the elapsed time between the evaluation of the if and the evaluation of the else may cause cases that should be mutually exclusive not to be so, or cases that should always coincide not to. So I got in the habit of tagging millis() at the beginning of each loop and just referring to that variable most of the time, except for cases where real-time response was specifically required.

As for the global variables... this is one of my WORST habits as a programmer, and it always has been. I kick myself about it all the time because ... well, I have a degree in CS, although I have never coded professionally. So I don't even have the excuse of ignorance. But the flip-side is that if a given function would always take the same five parameters, and would never take any other parameters, what's the point of declaring those as local variables and then passing them to a function? Why not just declare them global and refer to them?

The situation is exacerbated, I think, by the Arduino environment, which, as far as I can tell, requires any variable that outlasts a single loop() to be global. So you are required to make a bunch of global variables, at which point, it's easy to get lazy and just make a bunch more.

Thanks for all the input. It has given me lots to think about and pointers on where to go next.

As has been pointed out, efficiency can be different things, in this case size or speed. I group everything together under the one title and for me it is about picking what is appropriate. With timings and accuracy, speed is important. With strings, space is important. I kind of see this as common sense. (I use strings not Stings by the way).

I have created test scripts and tried to time things but either my timings are out or what I tried hasn't made much difference. This is why I asked on the forum. I have also searched online and was surprised at how little I found. I really expected to have many articles on how to improve the code. Not the case. I now suspect that if you are serious about efficiency you probably don't use the Arduino IDE.

I'm not new to programming but I am relatively new to the Arduino (repeat beginner) and new to C/C++ which is not helping. I haven't posted actual code because I was after ideas and suggestions rather that wanting somebody to fix my code. I enjoy the puzzle solving aspect of programming but some times I need a nudge in the right direction.

joshuabardwell:
The situation is exacerbated, I think, by the Arduino environment, which, as far as I can tell, requires any variable that outlasts a single loop() to be global. So you are required to make a bunch of global variables, at which point, it's easy to get lazy and just make a bunch more.

I findd myself doing this out of convenience.

If you keep the variables under control and reuse as many as possible, is it better to have global variable or is it better to use local one? Which is better for memory? With other languages (no worries about memory) I would always use local variables. I was taught that functions should be fully self contained except for what goes in at the top and what comes out at the bottom.On the Arduino I find it very convenient to have variables I can use to pass data to the functions, especially when using strings.