Creating a small arduino library best practice questions

[Gibbedy]

Hello.

I have a simple task I find myselft doing repeatedly in Arduino projects, timing sections of code to see how long they take to run.

I've finally (after 5 years) got the hardware of an 8x8x8 ws2811 LED cube mounted safely where the cat won't destroy it and i intend to work on animations for it when I get the chance.

I would like not to repeat the mistakes of a previous project involving thousands of leds where my codebase got so hard to understand I would spend a significant amount of time every year trying to remember how it works exactly.

The code I will post I believe is easy to follow for a novice like me, and I'm happy to hear ideas from anyone regarding it's structure, implementation, typo's, language i use in this post to describe it... whatever.

I was aiming for a simple bit of code I could re-use in my arduino projects and decided on a some variables and functions encapsulated in a namespace.

1. Given I can't see it meaningfull to have more than one timer I decided not to use a class/object and went for namespace. Is that a correct thought, are there other options here that are better or worse?

2. I can imaging using these timers and turning them off when I want to test my code without the timers, maybe need Serial output for other things or possibly just to free up some cycles. Is my method of turnOn() turnOff() a good strategy here and am I right in thinking calls to blank functions are the same as no call at all after the compiler has optimised this out? I suppose I'm still wasting some memory on variables I'm not using. Is it a better practice just to put #ifdef #endif around my timer stuff?

3. Is there another mechanism than micros() that might give me better resolution or less overhead or less error.

I'll post the complete code here and put a link to github if people would like to update changes with a pull request. I'm also trying to start a habbit of using github as I can see a benefit of it for my previously mentioned large led project.

GitHub - gibbedy/codeTimer: Time execution time of sections of code in an arduino program

Time execution time of sections of code in an arduino program - gibbedy/codeTimer

github.com

/*
 * On Arduino, I occasionally want to test how long a section or sections  of code takes to execute.
 * I do this because I often find I want to know how long some code blocks for or how much of every
 * loop I am left with to complete a certain task. Establishing a hard limit on the length/complexity of
 * things I might want to code.
 * In order to not fill up my program with debug global variables to store the results of my tests I
 * thought it would be better hide all the details in a namespace where I can time, store, and recall these
 * times for multiple sections of code with minimal extra text in my source code.
 *
 * The following is my CodeTimer functions. These will let me record the runtime of sections of code and print the results
 * to serial with a meaningfull name to allow easy understanding of how long some operations take in my program.
 * I can recall the time for a given id and print the time out to the serial console to see what is going on.
 *
 * I put them in their own namespace so i can call these functions through that name and not have to worry about naming conflicts.
 *
 * If I don't turnOn() all these functions will be empty and do nothing, so I can easily enable and disable this with one
 * line in the main program rather than having to put an #ifdef aroun every location where I use these functions.
 * I assume the empty functions are filtered out at compilation so I end up with the same result as if there was no function
 * calls at all?
 *
 * Usage would be:
 * CodeTimer::turnOn(); //enable timers in setup
 * CodeTimer::startTimer("description"); // put before the code you want to time
 * CodeTimer::stopTimer("description"); // put after the code I want to time. Must have the same name as the start time
 * CodeTimer::printResults(); //print to Serial the times of all the sections that have been timed.
 *
 */
 
 #ifndef CODETIMER_H
  #define CODETIMER_H
    namespace CodeTimer   
    {
      bool enableTests = false;  // Enable tests allows leaving timer starts and stops in your main code and disabling them with one change here
      const int MAX_TEST_NAME = 15;  // Maximum length of testname that is used to identify the section of code that is being timed
      const int MAX_TESTS = 20; // Maximum number of unique tests that can be performed.
      const int MAX_RECORDS = 100; // Maximum number of records for each test. When printing these will be averaged.
      
      void turnOn()
      {
        enableTests = true;
      }

      void turnOff()
      {
        enableTests = false;
      }
      // Each test stores its results in this structure:
      struct testResult
      {
        char testId[MAX_TEST_NAME];  // The name of the section of code under test
        unsigned long runTime[MAX_RECORDS];  // The runtime, in microseconds, of the section of code is put in here
        int recordNumber = 0; // Number of records currently stored for this testId.  Used so I know when the runtime array is full.
        unsigned long startTime; // Store the start time of the test currently being performed
        bool recordsFull = false; // If i fill the runTime array i set this flag.
                                  // This was added  because when averaging I stop at recordNumber (an array that is not full)
                                  // but if recordsFull is set I know the array is full (times are recorded over and over in array)
                                  // represents an array of runTimes thatso when averging I know to ignore recordNumber and average the whole Array
      };
      
      testResult tests[MAX_TESTS]; //tests for up to MAX_TESTS  are stored here
      
      int numberOfTests = 0; // how many tests are currently in the tests array.
      int checkRecordsForTest(const char recName[]);  // Check if a record name is already in the tests array and return the index of it or -1 if not
      unsigned long getAverage(int testsIndex); // get the average of all the runtimes for a given test by array index. Used by printTests function.
      
      int checkRecordsForTest(const char recName[])
      {
        if(enableTests)
        {
          int recordIndex = 0;
          for (; recordIndex < numberOfTests; recordIndex++)  // For each section of code that is being tested
          {
            if (strcmp(tests[recordIndex].testId, recName) == 0) return recordIndex;  //break out of loop if recName is found in the records
          }
          
          return -1; // If we got this far the loop was searched and no recName was found so return -1
                    // I can imagine more complex functions having common code that you would want to run regardless
                    // of whether a previous function return was made or not, and writing the function in a way so you
                    // have only one return function. By having one return function I would be helping avoid a situation
                    // where this common code was missed. Doing things that way would add a little overhead in this, and
                    // I think it is correct to say all cases. in this case It would add no benifit and an extra
                    // if check.  It was drummed into me from java days. Why was that particularly important in java?   
        }     
      }

      unsigned long getAverage(int testsIndex)
      {
        if(enableTests)
        {
          unsigned long sum = 0;
          int maxIndex = tests[testsIndex].recordNumber;
          if (tests[testsIndex].recordsFull) // The test located at 'testsIndex' is full (not on the first pass of filling the array)
          {
            maxIndex = MAX_RECORDS;  // So override maxIndex to be the index of a full records array
          }
          for (int i = 0; i < maxIndex; i++)
          {
            sum += tests[testsIndex].runTime[i];
          }
          
          return sum / maxIndex;
        }
      }

      // Record the start time of the test and give the record a name
      void startTimer(const char recName[])
      {
        if(enableTests)
        {
          int startTime = micros(); 
          int recordIndex = checkRecordsForTest(recName); //if the testname is already in the testResults then it returns the position of that test in the testResults array otherwise it returns -1

          if(recordIndex >= 0) //record exists
          {
            tests[recordIndex].startTime = startTime; //Store the start time in the appropriate test record
          }
          else //record doesn't exist so create one
          {
            if (numberOfTests <  MAX_TESTS)  //check we havne't maxed out our number of sections of code to test
            {
              strncpy(tests[numberOfTests].testId,recName,MAX_TEST_NAME - 1); // set name for test by copying char string
              tests[numberOfTests].testId[MAX_TEST_NAME - 1] = '\0'; //null terminate it. Only needed if I accidentally made the name longer than allowed by MAX_TESTNAME
              tests[numberOfTests].startTime = startTime;  //set the startTime we recorded at the start of this function.
              numberOfTests++;               
            }
            else
            {
              Serial.println("too many tests");
              Serial.println("Increase MAX_TESTS");
            }       
          }
        }       
      }

      // Stop the timer and specifiy an id to be used to store the result
      void stopTimer(const char recName[])
      {
        if(enableTests)
        {
            unsigned long runTime = micros(); // for now just put current time in there until i find the start time.
            int recordIndex = checkRecordsForTest(recName); //if the testname is already in the testResults then it returns the position of that test in the testResults array
            if(recordIndex >= 0) //record exists
            {
              tests[recordIndex].runTime[tests[recordIndex].recordNumber] = runTime - tests[recordIndex].startTime; //Store the start time in the appropriate test record
              tests[recordIndex].recordNumber++;

              if (tests[recordIndex].recordNumber >= MAX_RECORDS)
              {
                tests[recordIndex].recordNumber = 0;
                tests[recordIndex].recordsFull = true;
              }
            }
            else //record doesn't exist so we have called a stop on a start that doesn't exist
            {     
              Serial.print("Called a stop with an Id: ");
              Serial.print(recName);
              Serial.println (" that doesn't exist!");
            }
          }
        }

       void printResults()
      {
        if(enableTests)
        {
          Serial.print("CodeTimer results for ");
          Serial.print(numberOfTests);
          Serial.println(" tests");
          for (int i = 0; i < numberOfTests; i++)
          {
            Serial.print(tests[i].testId);
            Serial.print(" took on average ");
            unsigned long average = getAverage(i);
            Serial.print(average);
            Serial.println(" uS");         
          }
          Serial.println();
        }
      }
    }
#endif

 

[AndyA]

This is partly personal preference but I really dislike the "throw everything in a header file" approach to libraries. As soon as you have a project with more than one file you start hitting all sorts of redefinition type issues.
I much prefer the standard c / c++ approach, the header only contains declarations, all the definitions go in a separate .c / .cpp file. That way you can include the header as many times as you like in as many places as you want without any issues.

Since your code is already c++ rather than c I'd say define a class called CodeTimer and encapsulate all the code within that rather than a namespace. If you only want to have one of them then make it a singleton, this enforces only one instance of the class.

class CodeTimer   {
public:
  inline static CodeTimer  &Instance() {
    static CodeTimer  Inst;
    return Inst;
  };
  // any public functions
private:
  CodeTimer();
}

extern CodeTimer &MyCodeTimer;

// in the library .cpp file
CodeTimer &MyCodeTimer = CodeTimer::Instance();

In your code you could then use MyCodeTimer.SomePublicFunction() in the same way as when using a namespace only 1) you don't need to rebuild all your library code every compile 2) you can include the header in as many places as you want without causing compile errors.

 

[Gibbedy]

This is partly personal preference but I really dislike the "throw everything in a header file" approach to libraries. As soon as you have a project with more than one file you start hitting all sorts of redefinition type issues.
I much prefer the standard c / c++ approach, the header only contains declarations, all the definitions go in a separate .c / .cpp file. That way you can include the header as many times as you like in as many places as you want without any issues.

Since your code is already c++ rather than c I'd say define a class called CodeTimer and encapsulate all the code within that rather than a namespace. If you only want to have one of them then make it a singleton, this enforces only one instance of the class.

class CodeTimer   {
public:
  inline static CodeTimer  &Instance() {
    static CodeTimer  Inst;
    return Inst;
  };
  // any public functions
private:
  CodeTimer();
}

extern CodeTimer &MyCodeTimer;

// in the library .cpp file
CodeTimer &MyCodeTimer = CodeTimer::Instance();

In your code you could then use MyCodeTimer.SomePublicFunction() in the same way as when using a namespace only 1) you don't need to rebuild all your library code every compile 2) you can include the header in as many places as you want without causing compile errors.

I don’t think I’ve really understood the function of headers. If I’m understanding what you’re saying I’m now wondering why I see #ifndef,#define,#endif stuff around arduino headers.

The benefit of enforcing only one instance of the class is to prevent the programmer from accidentally doing something that we have decided doesn’t make sense, that is creating two objects of this type? Is it used for some other performance or memory benefits as well?

Is there such thing as a programming concepts book that goes through what I imagine are common ideas across many languages. I feel quite silly having been writing quite large messy programs that get job done but always hacking a solution rather than utilizing language features available that might make things allot easier.

 

[AndyA]

I don’t think I’ve really understood the function of headers. If I’m understanding what you’re saying I’m now wondering why I see #ifndef,#define,#endif stuff around arduino headers.

It's to do with how the c compiler and linker work.

The #ifndef .... pattern in headers is so that if a header is included in the same .cpp file twice (say once directly and once indirectly because another included header also includes it) then the code in the header is only included once.

When the compiler builds a .cpp file (the .ino file is in effect a .cpp file) the first thing it does is go through all the pre-processor commands, replacing #include lines with the contents of the included file, removing code if #if conditions aren't true, replacing macros with their code etc...
It then compiles that .cpp file into an object file. This is the final code but with placeholders pointing to any functions or variables defined in other .cpp files.
Finally it links all the object files together replacing those placeholders with the correct locations in the code as defined by the relevant object files.

In this process compiling each .cpp file into an object file is completely independent of any other .cpp files in the project. This has a couple of effects:
Firstly changing a single .cpp file means only that file (and any headers it uses) needs to be re-compiled, for other .cpp files the old results are still good. This can significantly speed up compile times. Changing a header file results in anything using that header needing to be re-compiled. This is why the first build of a new project takes a lot longer than if you then make a change and hit compile again. The first time it's having to compile all the libraries you used, the next time it doesn't need to.
Secondly if a variable or function is defined in a header file and that header is included in two different .cpp files then that variable / function gets defined in both object files. When it comes to link things together there are two copies of the same thing and the linker throws an error.

Basically if you only ever create simple projects that fit into a single .ino file without it becoming excessively long then putting everything in a header works fine. As soon as you want to create anything more complicated it becomes a massive handicap.

The benefit of enforcing only one instance of the class is to prevent the programmer from accidentally doing something that we have decided doesn’t make sense, that is creating two objects of this type? Is it used for some other performance or memory benefits as well?

It's mostly to prevent the programmer doing something that doesn't make sense. This method is often used when creating a class that handles the hardware e.g. the serial port or ethernet libraries create a single instance of the appropriate class to handle those interfaces. It means that unless you edit the library you can't accidentally create two different ethernet interfaces both trying to talk to the same hardware in the chip. Doing something like that never ends well, making is so that it's impossible to do is a good thing.

For this specific application you don't technically need to go this way since someone creating two instances isn't going to break things, it will use a tiny bit more memory and may cause some confusing situations but it would still work.
However my philosophy is that any time you can make it so that doing something silly causes a compiler error, or even better is simply impossible, that's a good thing. I'd much rather track down why the compiler is complaining than try to figure out why my program is doing something odd in one specific situation.

Is there such thing as a programming concepts book that goes through what I imagine are common ideas across many languages. I feel quite silly having been writing quite large messy programs that get job done but always hacking a solution rather than utilizing language features available that might make things allot easier.

Probably quite a few of them. ;-)
I've read as many of them as I've been on programming training courses. OK, that's a lie, I did do a course on VHDL around 25 years ago.
Mostly I've picked things up from working with some very good programmers over the years and seeing lots of code written in lots of different styles.
Some things people do don't make sense to me and, after checking that I've not missed something, I don't do things that way.

Some things make sense but I just don't like them. e.g. if (1 == variable) rather than if (variable == 1). I get the idea, if you accidentally miss a = so there is only 1 not 2 then you get a compile error rather than some unexpected behaviour. But I find the code a lot harder to read that way and so don't use it even though I can see why it has a benefit.

And some things just seem like a good idea to do. Like wrapping any self contained code up in a class and putting that class in a separate file so that it doesn't clutter up the rest of the code. Or spending a little bit more time on code while it's fresh in my mind to structure it in a way that makes it harder to use it incorrectly. Because if I come back to it later I'll probably have forgotten the limitations and restrictions that seemed so obvious at the time. Again, I'd rather get a compile error in that situation than have code that builds but then doesn't work as expected.

About the one thing that is always true is that any code you wrote more than 6 months ago is going to look like poorly structured garbage to you if you look at it again. Which is why hiding it away in a black box called a library is nice, as long as it works as intended you don't have to look at it again and then fight the urge to "fix" it.

 

[PaulStoffregen]

now wondering why I see #ifndef,#define,#endif stuff around arduino headers.

Each author may have specific reasons, but often #ifdef in Arduino libraries is used for code that is specific to certain hardware. Where the library is meant to work on more than 1 board will often have small piece of code which differ depending on which hardware you use.

decided not to use a class/object and went for namespace

For Arduino libraries, C++ classes are by far the most common way. The big advantage is you can declare which functions are the public API and which are the private stuff not meant to be used from outside the library. How easy the code is to later reuse depends on choosing a small intuitive set of public functions and keeping everything else private. The huge value of having the internal code private is you can later look at the library and only have to think about the public functions, assuming you just want to use it and you're not planning to dive back into its inner working.

 

[joepasquariello]

Good advice here from Andy and Paul. I have a couple of recommendations on good programming practices that go back to the very early days. One is a paper titled Structured Design, by Stevens, Myers, and Constantine. This is a classic paper cited hundreds of times that defines the method of partitioning a program as "maximize cohesion and minimize coupling". This link (https://essentials.xebia.com/maximize-cohesion-minimize-coupling/) provides a brief explanation, but to read the whole paper you have to buy a PDF or hard copy, which is too bad. It's very old (1974), so it's not about C or C++ specifically, but rather where to draw the lines and boundaries. And one more, probably my favorite book on software design, Introduction to Real-Time Software Design by S.T. Allworth. The first edition is the best, simply because it's the shortest. You can find it used online and it's really worth reading. Chapter 6 is Design Methodology.

 

[Gibbedy]

@AndyA
Thanks, that clears a few things up for me regarding headers. I’m motivated to go back to a project and re-organise with classes and separate compilation units if for nothing else than to see how much i can speed up compilation and understand this process better. I Think I will change to a codeTimer skeleton class. I don’t think you mention, but do I assume correctly that after creating multiple object files which could have there own copy of the same implementation of the code that is included for a specific header, that the linker then removes all but one of these implementations?

@PaulStoffregen Thanks Paul. I meant to say specifically why a whole header will be surrounded by an #ifndef,#define,#endif. And not that I cant see (at least clearly now) what its acheiving by surrounding the header with this, it was just the mechanism feels clunky and I wonder why there wouldn’t be some automatic mechanism to check when the same file is being included twice in one cpp file.

In the past I’ve felt some negativity towards classes and objects being used. I only really understand how to think about solving problems in that way as some Java at uni got me started with programming and was the only time I was forced to approach this subject from the top down. Lately I’m curious as to how you solve problems without using classes. That is having functions separate from your data that is processed by these functions. I looked into structures which contain data and function pointers… but that just felt like creating objects in a harder way. The point is I’m not understanding the higher level concepts well enough to be able to make an informed choice about what approach to use.

@joepasquariello I’m going to check the paper and that book out. It sounds like what I’m looking for and an interesting read if nothing else.

 

[MarkT]

@PaulStoffregen Thanks Paul. I meant to say specifically why a whole header will be surrounded by an #ifndef,#define,#endif. And not that I cant see (at least clearly now) what its acheiving by surrounding the header with this, it was just the mechanism feels clunky and I wonder why there wouldn’t be some automatic mechanism to check when the same file is being included twice in one cpp file.

These days in C++ we simply use:
#pragma once

To prevent multiple includes of the same header. I assume this works in Arduino/Teensy for any header included from C++ files, I haven't checked.