Best practices for making a header/library

I run an escape room and am wanting to connect all of the Arduinos together using Nick Gammon's code for an rs485 network. I'd be using the rolling master code.

If I wanted to go about making the majority of that code into a class or separate header file for the sake of "cleanliness", what would be the best way to go about doing that? I am going back and forth between option a) having a "buffer" arduino with Nick's code that serves as the go between for the rs485 network and the actual Arduino in the room that does the puzzles/visual effects. This would be nice, because all of my existing code depends heavily on delays.

If I went with option b, all of my delays would need to be cut if the "room-side" Arduino also contained the rolling master code. Option b would also mean less than half the number of Arduinos, meaning slightly less cost and wiring/soldering on my part.

I tend to prefer option a, but the prospect of rewriting my code and fitting it around Nick's code feels daunting, unless I can reduce his code to a neat little header or library or something.

Thanks in advance for any advice.

I've created several Arduino libraries (found here) and I can say there are several things to keep in mind:

  • Above all --> your library should include BOTH a .h and a .cpp file

  • You can use "#pragma once" instead of traditional include guards in your .h file

  • For constant values, use "const" and do NOT use #define

  • There should be a class within the library that has a name similar to the name of the library. All functionality offered to the user should be accessed/implemented through this class

  • If your library requires the use of an external class (i.e. the Serial or Servo classes), you should make the user pass an instance of the external class to the library class as an argument. The argument can be for the library class constructor or for the class's member functions individually - your choice.

  • Lastly --> Take a look at example libraries. I make a concerted effort to write easy to understand libraries, so if you want some easy libraries to reference, take a look at these:
    -- SerialTransfer.h
    -- DFPlayerMini_Fast.h
    -- NEO-6M_GPS.h
    -- Teensy_3X_Multipurpose_Board.h

Power_Broker:

  • If your library requires the use of an external class (i.e. the Serial or Servo classes), you should make the user pass an instance of the external class to the library class as an argument.

Make that pass a pointer (or Reference) to the external object, not an instance since said instance will be copy-constructed from the original before being passed on the stack. Not only is that inefficient, but it may not work.

1 Like

gfvalvo:
Make that pass a pointer (or Reference) to the external object, not an instance since said instance will be copy-constructed from the original before being passed on the stack. Not only is that inefficient, but it may not work.

Yes, sorry, I meant pass by reference...

Power_Broker:

  • Above all --> your library should include BOTH a .h and a .cpp file

Some libraries are header-only. You don't need .cpp files to have a working library.

Header-only libraries are used for better optimizations using inline functions, and are the only option when using templates.

Pieter

How would a "buffer" Arduino work?

But really, I think you should just rewrite your code your code instead of digging a deeper whole trying to keep dalay()'s. You can put lipstick on a pig...

And as a library tip, use a unique name!

Make sure that each class / struct / const / method / procedure / field / variable the user has access to is documented in doxygen style - this makes it easier to extract a manual from the code. I prefer to have my doxygen in the header since this is the file exposed to the user.

Start with a standard. Here are some links.

MISRA C++ Standards

Barr Group Embedded C Coding Standards

SEI Secure Coding in C++11 and C++14

UTSL: Bjarne Stroustrup's C++ Style and Technique FAQ

The C++ Core Guidelines are a set of tried-and-true guidelines, rules, and best practices about coding in C++

PieterP:
Some libraries are header-only. You don't need .cpp files to have a working library.

Header-only libraries are used for better optimizations using inline functions, and are the only option when using templates.

Header-only - Wikipedia

Pieter

I know some do, but for the vast majority of Arduino libraries, it is a best practice to use both a header and source

septillion:
How would a "buffer" Arduino work?

That's probably not the best term. The "buffer" would have Gammon's code on it and communicate with the Arduino doing all the sensing/logic/effects for the room. I've basically decided to forgo this option anyways.

For this specific scenario, would it even make sense to make it a class, since there would not be multiple instances of the network? Or, since the user would need to pass in the specific pins, does that indicate a class is necessary..?

And if anyone would be willing to take a peek at the rolling master code to see if there's anything else I should be considering, that'd be great.

Bapstack:
For this specific scenario, would it even make sense to make it a class, since there would not be multiple instances of the network? Or, since the user would need to pass in the specific pins, does that indicate a class is necessary..?

Always try to design code that scales-up easily and is maintainable.

You might not care about having multiple instances right now, but maybe your project design will change later on. Do you want to rewrite your library every time you change your hardware design or add features? What if someone else wants to use your library and needs multiple instances?

Classes are easy to create and easy to use - why not make a class?

Power_Broker:

  • If your library requires the use of an external class (i.e. the Serial or Servo classes), you should make the user pass an instance of the external class to the library class as an argument. The argument can be for the library class constructor or for the class's member functions individually - your choice.

So if I'm needing the Software Serial library, I shouldn't have the user just pass in the rx and tx pin numbers? If the user declares the software serial class, what would the syntax look like to pass a reference to the software serial class to the constructor of my main library class?

Also, his code has his own library, the initial parameters of which don't depend on anything the user might enter in. Any reason that class needs to be declared outside my own class?

Delta_G:
Usually step one of this is to get it working all in one sketch. Then once you have that you can start pulling parts of it out into a header or a library.

I strongly disagree with this advice. This is a surefire way to create a tangled mess of interdependence that is extremely difficult to pull apart after the fact. In programs I've written like this (not just for Arduino), I've usually had to throw it out and restart from scratch to impose a good structure on the code.

It's much better to start pulling stuff out as you go along during your design. If you find yourself duplicating certain constructs, and especially if you're copy+pasting stuff around, that's usually a sign that you should pull that into a function or class. Doing the refactoring as you're constructing the code is 100x easier than trying to unravel the Gordian knot after it's already tied up.

Delta_G:
Don't have them pass an instance of SoftwareSerial. That limits the use of the class to just that. You want to also allow for some board that have more than one hardware serial. Or even someone using a different soft serial library.

So make it take a reference to a Stream object. That's the base type that all of those derive from.

class MyClass {

Stream* theStream;

MyClass (Stream* aStream) : theStream(aStream) {}

};

SoftwareSerial btSerial(3,4);
MyClass myClass(&btSerial);

// or if the code gets used later on a Mega you can

MyClass myMegaClass(&Serial2);

Alternatively, use references instead of pointers:

[color=#00979c]class[/color] [color=#000000]MyClass[/color] [color=#000000]{[/color]
  [color=#00979c]private[/color][color=#434f54]:[/color]
    [color=#d35400]Stream[/color][color=#434f54]&[/color] [color=#000000]theStream[/color][color=#000000];[/color]

  [color=#00979c]public[/color][color=#434f54]:[/color]
    [color=#000000]MyClass[/color] [color=#000000]([/color][color=#d35400]Stream[/color][color=#434f54]&[/color] [color=#000000]aStream[/color][color=#000000])[/color] [color=#434f54]:[/color] [color=#000000]theStream[/color][color=#000000]([/color][color=#000000]aStream[/color][color=#000000])[/color] [color=#000000]{[/color][color=#000000]}[/color]

  [color=#00979c]void[/color] [color=#000000]printHello[/color][color=#000000]([/color][color=#000000])[/color] [color=#000000]{[/color]
      [color=#000000]theStream[/color][color=#434f54].[/color][color=#d35400]println[/color][color=#000000]([/color][color=#000000]F[/color][color=#000000]([/color][color=#005c5f]"Hello!"[/color][color=#000000])[/color][color=#000000])[/color][color=#000000];[/color]
  [color=#000000]}[/color]
[color=#000000]}[/color][color=#000000];[/color]

[b][color=#d35400]SoftwareSerial[/color][/b] [color=#000000]btSerial[/color][color=#000000]([/color][color=#000000]3[/color][color=#434f54],[/color][color=#000000]4[/color][color=#000000])[/color][color=#000000];[/color]
[color=#000000]MyClass[/color] [color=#000000]myClass[/color][color=#000000]([/color][color=#000000]btSerial[/color][color=#000000])[/color][color=#000000];[/color]
[color=#000000]MyClass[/color] [color=#000000]myMegaClass[/color][color=#000000]([/color][b][color=#d35400]Serial2[/color][/b][color=#000000])[/color][color=#000000];[/color]

I've seen that single colon before but don't quite understand the notation. What would it look like if my constructor had additional parameters?

public:
    MyClass (Stream& aStream, int x, byte whatever) : theStream(aStream) {}

Just like so?

Bapstack:
I've seen that single colon before but don't quite understand the notation.

It's an Initializer List.

So I have a struct that contains all of the data that will be communicated in the message packet. The actual makeup of the struct will change depending on what the user decides to put in there (switch states, analog read values, whatever).

I want to pass that struct my reference to my class. Here's what I have.

struct msg {
  byte defaultHubArray[2];
  byte cashRegister;
  long blankKeypadEntry;
  byte lights;
} vaultMessage;

The constructor in my class header is as follows:

  RS485Network(Stream& RS485Serial_, Struct& message_, XMIT_ENABLE_PIN_,
    unsigned long BAUD_RATE_ = 9600, unsigned long TIME_BETWEEN_MESSAGES_ = 3000)
    : RS485Serial(RS485Serial_), message(message_), XMIT_ENABLE_PIN(XMIT_ENABLE_PIN_),
      BAUD_RATE(BAUD_RATE_), TIME_BETWEEN_MESSAGES(TIME_BETWEEN_MESSAGES_)
      {
        initialize();
      }

And I figure before setup() I would have

SoftwareSerial mySerial(rx, tx);

RS485Network network(mySerial, vaultMessage, xmit);
//presumably this would set baud to 9600 and time between messages to 3000

Am I on the right track here? Glaring issues?

Post the entire class declaration:

class RS485Network {
.
.
.
};

gfvalvo:
Post the entire class declaration

class RS485Network
{
private:
  Stream& RS485Serial;

  unsigned long BAUD_RATE, TIME_BETWEEN_MESSAGES;
  byte XMIT_ENABLE_PIN;
  byte myAddress;
  byte numberOfDevices;

  float TIME_PER_BYTE, PACKET_LENGTH, PACKET_TIME;

  unsigned long noMessagesTimeout;

  byte nextAddress;
  unsigned long lastMessageTime;
  unsigned long lastCommsTime;
  unsigned long randomTime;

  // Initial seed for JKISS32
  static unsigned long x = 123456789,
                       y = 234567891,
                       z = 345678912,
                       w = 456789123,
                       c = 0;

  void initialize();
  unsigned long JKISS32 ();
  void Seed_JKISS32(const unsigned long newseed);

  enum {
    STATE_NO_DEVICES,
    STATE_RECENT_RESPONSE,
    STATE_TIMED_OUT,
  } state;

  // callbacks for the non-blocking RS485 library
  size_t fWrite (const byte what) { rs485.write (what); }
  int fAvailable () { return rs485.available (); }
  int fRead () { lastCommsTime = micros (); return rs485.read ();}
  RS485 network (fRead, fAvailable, fWrite, 20);


public:
  RS485Network(Stream& RS485Serial_, Struct& message_, XMIT_ENABLE_PIN_,
    byte myAddress_, byte numberOfDevices_,
    unsigned long BAUD_RATE_ = 9600, unsigned long TIME_BETWEEN_MESSAGES_ = 3000)
    : RS485Serial(RS485Serial_), message(message_), XMIT_ENABLE_PIN(XMIT_ENABLE_PIN_),
      myAddress(myAddress_), numberOfDevices(numberOfDevices_),
      BAUD_RATE(BAUD_RATE_), TIME_BETWEEN_MESSAGES(TIME_BETWEEN_MESSAGES_)
      {
        initialize();
      }
};

Added a couple more parameters to my constructor.

Edit: Another follow up question. If I want to pass a second Serial in for debugging, but don't want the class to require it (in case I don't have a dedicated debug serial port, say), how would I make that Stream& optional? Just set it equal to null in my constructor?

You can't initialize non-const static members in the class declaration. The compiler should have told you this. i.e.:

 static unsigned long x = 123456789,
                       y = 234567891,
                       z = 345678912,
                       w = 456789123,
                       c = 0;