Library files

When creating a library for the Arduino environment that does not use OOP is there any advantage in having both a .h file containing variable and function declarations and a .cpp file containing the actual functions rather than putting everything in a single .h file ?

I would never put code in a h file, only declarations. I don't think code belongs in a h file. Now if you ask me 'why?' I suggest both that to do so is untidy and inconsistent, and it's probably against the rules.

I agree with you and have used separate files in previous libraries that I have written, albeit they used classes. However, the one I am writing at the moment doesn’t use classes and having absentmindedly put functions into the .h file with no .cpp file present I wondered about the side effect, none of which I have seen yet.

To me it does not seem untidy and inconsistent at all, rather it seems perfectly natural and I know of at least one member here who does it all the time on purpose and who say that it causes no problems

I can't imagine it causes a problem, it just doesn't seem right to me, which might be little more than a style preference. Maybe someone will come along and teach us both something.

Maybe someone will come along and teach us both something.

I hope so, but there are commonly held beliefs here such as "never use the delay() function" which are simply not true and I wonder if this is another of them

UKHeliBob:
I hope so, but there are commonly held beliefs here such as "never use the delay() function" which are simply not true and I wonder if this is another of them

I feel sure you know my views on delay(), I an only assume you are trying to wind me up! :slight_smile:

If it's a simple library, especially if there is no OOP elements, I don't see anything wrong with only a header. However, anything more complicated should be broken out into separate .h and .cpp files.

I feel sure you know my views on delay(), I an only assume you are trying to wind me up!

No attempt to wind you up previously, but I would be interested in hearing what you think is wrong with using delay() in the Blink sketch ?

UKHeliBob:
No attempt to wind you up previously, but I would be interested in hearing what you think is wrong with using delay() in the Blink sketch ?

I feel I know you on here well enough that you are welcome to try to wind me up, I just hope any such thing doesn't annoy other readers :o

Blink is great for allowing someone completely new to micro-controllers and Arduino to get something working for the first time. I am sure many people have bought an Arduino unsure if they would ever be able to get it to do anything and then felt a surge of delight when they see the LED flashing for the first time, and even more delight when they find out how easy it is to change the rate of flashing.

The problem I have with that is those very same people turn up here shortly afterwards with complicated, often difficult to follow code full of delays and asking why the code is unresponsive, or they are trying to use interrupts for buttons so they can get a fast response from code that is stuck in a delay somewhere.

I really wish the first example was not blink with delay but blink with millis, written in a non-blocking way. Blink with delay teaches a bad habit from the start.

As I posted yesterday in another discussion: What people forget is this; maybe in well written code loop goes round in 1ms, that's 1000 times per second. A 19ms delay does not seem like much but it's not 19ms, it's 19ms every time around loop. So, the loop that was going round in 1ms without delay is now taking 20ms, so instead of 1000 times per second it is going round only 50 times per second with 95% of that time taken up in delay, so that 19ms delay is actually wasting 95% of the available processor time. Think what useful things you could be doing with all that wasted processor time.

If you are an experienced programmer and understand the consequences of using delay then you can use delay with care in the few rare cases where it is appropriate, for everyone else I wish it were not there.

So, Mr Moderator, do you and the other mods have the power to get blink with delay removed from the IDE examples and replaced with something non-blocking based on millis?

To summarise. There is nothing wrong with using delay() in a sketch as long as you understand the consequences, with which I fully agree.

do you and the other mods have the power to get blink with delay removed from the IDE examples and replaced with something non-blocking based on millis?

No, neither should it be removed in my opinion. Rather it should be revised to contain comments warning of the consequences of using delay()

So, in a similar vein, what are the consequences of putting code in a .h file #included in a sketch ? It is those consequences (if any) that I am wanting to find out about

UKHeliBob:
When creating a library for the Arduino environment that does not use OOP is there any advantage...

Yes. Your library will be usable by other libraries.

The digitalWriteFast library is entirely contained in a single .h file

The entire Python Bottle web framework is in a single .py file

IMHO having fewer files makes the housekeeping simpler. YMMV

...R

I thought the purpose of using .h file was to separate the interface of a library or class from its implementation. The library author can give the interface, the .h file, to other programmers or teams, but keep the implementation, the .c or .cpp file, hidden and give only the compiled binary. This prevents programmers using the library from knowing details of the implementation and making use of that in some way that they shouldn't. That's important because it would prevent the library author from changing the implementation later without fear of breaking the code that uses it.

This idea can work well in the PC ecosystem, for example, where binaries can be published because every consumer/target uses the same x86 architecture. Ok, you might have to publish 32-bit and 64-bit variants, but it's manageable.

Of course, this doesn't really work in the Arduino ecosystem. There is no mechanism for publishing binaries, and even if there were, the library author would have to publish so many versions for different MCU architectures it would not be manageable. So the implementation, the .c/.cpp files, are published along with the interface, the .h file, so it can be compiled by the consumer as needed for the particular architecture they are using. This defeats the ability of the library author to hide the implementation. All they can do is request and hope that the consumers only read the interface (.h) and don't read the implementation (.c/.cpp) and rely on it always working in the exact same way forever.

PaulRB
++Karma; // I knew I would learn something from this, thank you.

PaulRB:
All they can do is request and hope that the consumers only read the interface (.h) and don't read the implementation (.c/.cpp) and rely on it always working in the exact same way forever.

And how many times do we see advice in the Forum to study the code in the .cpp file because the author never bothered to write proper documentation :slight_smile:

...R

Robin2:
And how many times do we see advice in the Forum to study the code in the .cpp file because the author never bothered to write proper documentation :slight_smile:

Maybe the author thought "I can skimp a bit on the documentation, I'm going to have to publish the .cpp files anyway, they can read those".

PaulRB:
Maybe the author thought

I suspect that is a very generous interpretation :slight_smile:

...R

I'd love to hear the explanation on why a non-OOP lib won't work when included in other libs.

UKHeliBob:
So, in a similar vein, what are the consequences of putting code in a .h file #included in a sketch ? It is those consequences (if any) that I am wanting to find out about

Since #include-ing a header-only lib is basically telling the preprocessor to copy and paste the contents of the header to the top of the main.cpp file, I honestly don't think there are any consequences to doing this compared to #include-ing normal libs.

Robin2:
IMHO having fewer files makes the housekeeping simpler. YMMV

Any C++, OOP lib beyond 100 lines generally benefits from simpler housekeeping if it's broken out into separate h and cpp files. Header for a simple API list and cpp for the good stuff. Having multiple h/cpp files in a lib can also help if the lib is complicated enough. Imagine if the Arduino core was a single h file - it would be impossible to maintain!

Thanks for the explanations. Hiding code from others may be a valid reason in some environments but as pointed out it is not really applicable to the Arduino and there seems to be no compelling reason not to use a single .h file

However, as an exercise I decided to change my library to use a .h and .cpp file but I cannot get it to compile. I will upload the files and error messages after a bit more investigation and cutting the code down a little. I have probably made a simple mistake and may spot it in the process

Posted below is the problem code with error messages in the next post. You can probably tell that it is intended to control an LED cube, currently 4x4x4 but it was my original intention for it to work with a cube of any (reasonable) size hence the generous size (int) of many variables as although a 4x4x4 cube has only 64 LEDs the number rapidly increases as the cube size increases

Test sketch

#include <Cube444.h>
#include <HC4x4x4Cube.h>

void setup()
{
  CubeInit();
}

void loop()
{
  framesFromLeft();
}

void framesFromLeft()
{
  for (int slice = 0; slice < SIZE; slice++)
  {
    fullFrame(slice, FROM_LEFT, ON, 100);
    fullFrame(slice, FROM_LEFT, OFF, 0);
  }
  for (int slice = SIZE - 1; slice >= 0; slice--)
  {
    fullFrame(slice, FROM_LEFT, ON, 100);
    fullFrame(slice, FROM_LEFT, OFF, 0);
  }
}

The .h file

#ifndef Cube444_h
#define Cube444_h
#include <Arduino.h>
#include <HC4x4x4Cube.h>

int valueIndex;
int ledIndex;
int ledValue;

enum commands
{
  OFF,
  ON,
  TOGGLE,
  FROM_LEFT,
  FROM_RIGHT,
  FROM_BASE,
  FROM_TOP,
  FROM_BACK,
  FROM_FRONT
};

const int SIZE = 4;
const int LEDS_PER_SLICE = SIZE * SIZE;
const int NUMBER_OF_LEDS = SIZE * LEDS_PER_SLICE;

void getLedValues(int ledNumber);
void oneLed(int ledNumber, int command, unsigned long ledWait);
void oneStick(int ledNumber, int direction, int action, unsigned long stickWait, unsigned long ledWait, int length);
void fullFrameSweep(int slice, int offset, int howMany, int direction, int state, unsigned long ledSpeed, unsigned long frameSpeed);
void oneSlice(int slice, int direction, int state, unsigned long sliceWait, unsigned long stickWait, unsigned long ledWait);
void fullFrame(int slice, int direction, int state, unsigned long frameWait);
void debugPrint(int data);
void debugPrint(char * data);

#endif

The .cpp file

#include <Arduino.h>
#include <Cube444.h>

void getLedValues(int ledNumber)
{
  valueIndex = ledNumber / LEDS_PER_SLICE;
  ledIndex = ledNumber % LEDS_PER_SLICE;
  ledValue = 1 << ledIndex;
}

void oneLed(int ledNumber, int command, unsigned long ledWait)
{
  getLedValues(ledNumber);
  switch (command)
  {
    case ON:
      Matrix_Buffer[valueIndex] |= ledValue;  //turn on LED
      break;
    case OFF:
      Matrix_Buffer[valueIndex] &= ~ledValue; //turn off LED
      break;
    case TOGGLE:
      if (bitRead(Matrix_Buffer[valueIndex], ledIndex) == 0)  //LED is off
      {
        Matrix_Buffer[valueIndex] = Matrix_Buffer[valueIndex] | ledValue;  //turn on LED
      }
      else
      {
        Matrix_Buffer[valueIndex] = Matrix_Buffer[valueIndex] ^ ledValue;  //turn off LED
      }
      break;
  }
  if (ledWait)
  {
    delay(ledWait);
  }
}

void oneStick(int ledNumber, int direction, int action, unsigned long stickWait, unsigned long ledWait, int length)
{
  switch (direction)
  {
    case FROM_BASE :
      for (int slice = 0; slice < length; slice++)
      {
        int currentLed = ledNumber + (slice * 16);
        oneLed(currentLed, action, ledWait);
      }
      break;
    case FROM_LEFT:
      for (int slice = 0; slice < length; slice++)
      {
        int currentLed = (slice * 4) + ledNumber;
        oneLed(currentLed, action, ledWait);
      }
      break;
    case FROM_BACK:
      for (int slice = 0; slice < length; slice++)
      {
        int currentLed = slice + ledNumber;
        oneLed(currentLed, action, ledWait);
      }
      break;
  }
  if (stickWait)
  {
    delay(stickWait);
  }
}

void fullFrameSweep(int slice, int offset, int howMany, int direction, int state, unsigned long ledSpeed, unsigned long frameSpeed)
{
  switch (direction)
  {
    case FROM_LEFT:
      {
        int baseLeds[] = {0, 16, 32, 48, 49, 50, 51, 35 , 19, 3, 2, 1};
        int index = offset;
        for (int count = 0; count < howMany; count++)
        {
          int currentLed = baseLeds[index] + (4 * slice);
          oneLed(currentLed, state, ledSpeed);
          index++;
          if (index >= 12)
          {
            index = 0;
          }
        }
      }
      break;
    case FROM_BACK:
      {
        int baseLeds[] = {0, 16, 32, 48, 52, 56, 60, 44 , 28, 12, 8, 4 };
        int index = offset;
        for (int count = 0; count < howMany; count++)
        {
          int currentLed = baseLeds[index] + slice;
          oneLed(currentLed, state, ledSpeed);
          index++;
          if (index >= 12)
          {
            index = 0;
          }
        }
      }
      break;
    case FROM_BASE:
      {
        int baseLeds[] = {0, 1, 2, 3, 7, 11, 15, 14, 13, 12, 8, 4};
        int index = offset;
        for (int count = 0; count < howMany; count++)
        {
          int currentLed = baseLeds[index] + (slice * 16);
          oneLed(currentLed, state, ledSpeed);
          index++;
          if (index >= 12)
          {
            index = 0;
          }
        }
      }
      break;
  }
  if (frameSpeed)
  {
    delay(frameSpeed);
  }
}


void oneSlice(int slice, int direction, int state, unsigned long sliceWait, unsigned long stickWait, unsigned long ledWait)
{
  int firstLed;
  switch (direction)
  {
    case FROM_LEFT:
      firstLed = slice * 16; //0, 16, 32, 64
      for (int led = firstLed; led < firstLed + SIZE; led++)
      {
        oneStick(led , FROM_BASE, state, stickWait, ledWait, SIZE);
      }
      break;
    case FROM_BASE:
      firstLed = slice * 4; //0, 1, 2, 3
      for (int led = firstLed; led < firstLed + SIZE; led++)
      {
        oneStick(led , FROM_LEFT, state, stickWait, ledWait, SIZE);
      }
      break;
    case FROM_BACK:
      firstLed = slice; //0, 1, 2, 3
      for (int led = firstLed; led < 16 ; led += 4)
      {
        oneStick(led , FROM_LEFT, state, stickWait, ledWait, SIZE);
      }
      break;
  }
  if (sliceWait)
  {
    delay(sliceWait);
  }
}

void fullFrame(int slice, int direction, int state, unsigned long frameWait)
{
  fullFrameSweep(slice, 0, 12, direction, state, 0, frameWait);
}

void debugPrint(int data)
{
  Serial.begin(2000000);
  Serial.print(data);
  Serial.end();
}

void debugPrint(char * data)
{
  Serial.begin(2000000);
  Serial.print(data);
  Serial.end();
}