Communication protocol implementation in embedded C++

Question

I have been implementing a CAN based proprietary communication protocol in C++. The protocol uses messages with following structure:

1       Control packet  (3 bytes header, 5 payload bytes)
0 - 124 Data packets    (2 bytes header, 6 payload bytes)
1       CRC packet      (2 bytes header, 4 payload bytes, 2 CRC bytes)

I don't have much experience with communication protocols implementation so I would like to discuss here my solution. I have started with transmission.

From the top view I have decided to divide the implementation of transmission into three classes. Each of them will implement Singleton design pattern. The classes interfaces are following:

// Main interface for communication over CAN bus.
class Manager
{
  public:

      // initializes the CAN periphery (baud rate and reception filters)
      bool init(can_baudrate_e, uint16_t*);
      // creates the message, divides the message into packets and puts the 
      // packets into transmission queue, message description (type, 
      // destination node ID) and message data are stored in a structure
      bool sendMsg(app_msg_data_t*);


  private:

      // appends unique message code to informations in app_msg_data_t 
      // structure
      void buildMsg(app_msg_data_t*, msg_t*);
      // calculates needed number of packets
      uint8_t calcNoPkts(msg_t*);
      // divides the message into packets
      void appMsg2Pkts(msg_t*, uint8_t, pkt_t*);
      // creates message Control packet
      void buildAppControlPkt(msg_t*, pkt_t*, uint8_t, uint8_t);
      // creates message Data packet
      void buildAppDataPkt(msg_t*, pkt_t*, uint8_t);
      // creates message CRC packet
      void buildAppCRCPkt(msg_t*, pkt_t*, uint8_t, uint8_t);
      // transform whole message into byte array
      uint16_t getAppMsgBytes(pkt_t*, uint8_t, uint8_t*);
      // returns the data content of the message
      uint8_t* getAppMsgData(msg_t*);
      // calculates the CRC for a message (message passed as array of bytes) 
      uint16_t calcCRC(uint8_t*, uint16_t);
} 


// Transmission buffer
class TxQueue
{

   public:
      // puts the packet into the transmission queue
      bool putPacket(Manager::pkt_t*);
      // retrieves the packet from the transmission queue
      bool getPacket(Manager::pkt_t*);

   private:
}

// Transmits the packets onto the CAN bus
class Transmitter
{

    public:
      // transmits one packet per call onto the CAN bus 
      bool transmit(void);

    private:
      // transforms the packet into CAN frame
      static void packet2CANFrame(Manager::pkt_t*, can_msg_t*);

}

I would very appreciate if somebody more experienced than me could asses my solution. I am afraid of taking the wrong path. Thank you in advance for any suggestions.

Answer 1

I'd recommend reading Guide to Implementing Communication Protocols in C++ (for Embedded Systems) free e-book first. The main concept mentioned in the book is that it is not uncommon for embedded systems to make an attempt to use the same protocol messages in the future over different additional I/O interfaces (such as wifi or bluethooth). As the result, it would be beneficial to separate application messages payload from the transport framing and especially from your CAN bus packatization.