I have an embedded project and I am implementing a library to handle serial communication between 2 microprocessors. I have a structure in mind to have a scalable approach for packets that might get added as time passes.
There needs to be distinct packets that carry variable length information. Each type of packet is interpreted differently by the processor receiving it.
The reads are to be performed whenever a byte comes in. When a full packet comes consumer function is called to handle it asap.
I wanted to explain my approach and get some feedback on its' weaknesses and how It might be improved.
Current Approach:
- Create a structure that shows the data members and lengths.
#define PASSWORD_PACKET_BYTES 5
typedef union
{
uint8_t PasswordBuffer[PASSWORD_PACKET_BYTES];
struct
{
uint32_t Password;
uint8_t CRC;
};
}password_packet_t;
- Create a union that holds all the types of packets and a buffer of uint_8.
- Create a structure that holds bytes read, current packet type being read(tag for the union),the escape state.
#define DISPLAY_PACKET_BYTES 16
typedef struct
{
union
{
uint8_t SerialBuffer[DISPLAY_PACKET_BYTES];
password_packet_t PasswordPacket;
};
struct
{
uint8_t CurrentOperation;
uint8_t BytesRead;
bool EscapeNext;
};
}disp_receive_packet_t;
- Create an enum that shows the values for different types of packets(Also contains a value for the escape character).
enum
{
RECEIVE_NO_OPERATION = 0xA0,
ACK,
PASSWORD,
VERSION_REQUEST,
RECEIVE_ESCAPE,
}DISP_RECEIVE_OPERATIONS;
Implementation of the functions: -Whenever a byte is read, check if it is a new operation, if so change the union tag. -Else if escape character value set the escape flag. -Else push the data to the buffer, increment bytes read. Also call a check finished function for the current operation(switch statement). If finished, handle the packet and reset the control structure. If not, nothing happens.
void DispRead1B(void)
{
static uint8_t ReadByte;
ReadByte = (uint8_t)UARTCharGet(DISP_BASE);
if(DisplayPacket.EscapeNext)
{
DisplayPacketPushByte(ReadByte);
DisplayPacket.EscapeNext = false;
}
else
{
if(ReadByte == RECEIVE_ESCAPE)
{
DisplayPacket.EscapeNext = true;
}
else if((ReadByte < RECEIVE_ESCAPE) && (ReadByte > RECEIVE_NO_OPERATION))
{
DisplayPacketChangeOperation(ReadByte);
}
else
{
DisplayPacketPushByte(ReadByte);
}
}
}
void DisplayPacketChangeOperation(uint8_t Operation)
{
DisplayPacket.CurrentOperation = Operation;
DisplayPacketResetBytesRead();
DisplayPacket.EscapeNext = false;
DisplayPacketCheckOperationFinished();
}
void DisplayPacketPushByte(uint8_t Value)
{
DisplayPacket.SerialBuffer[DisplayPacket.BytesRead] = Value;
DisplayPacketIncrementBytesRead();
DisplayPacketCheckOperationFinished();
}
void DisplayPacketIncrementBytesRead(void)
{
DisplayPacket.BytesRead += 1;
if(DisplayPacket.CurrentOperation == RECEIVE_NO_OPERATION)
{
DisplayPacket.BytesRead = 0;
}
}
bool DisplayPacketCheckOperationFinished(void)
{
bool PacketFinished = false;
switch (DisplayPacket.CurrentOperation)
{
case ACK:
PacketFinished = DisplayPacketCheckAckFinished(); break;
case VERSION_REQUEST:
PacketFinished = DisplayPacketCheckVersionRequestFinished(); break;
case PASSWORD:
PacketFinished = DisplayPacketCheckPasswordFinished(); break;
default:
return false;
}
if(PacketFinished)
{
DisplayPacketChangeOperation(RECEIVE_NO_OPERATION);
}
return PacketFinished;
}
bool DisplayPacketCheckAckFinished(void)
{
if(DisplayPacket.BytesRead == ACK_PACKET_BYTES)
{
if(CheckSysFlagNotSet(KeepAliveAckReceived))
{
UpdateSysFlag(KeepAliveAckReceived,true);
StartAliveTimer();
}
else
{
UpdateSysFlag(ConnectionError,false);
ReloadAliveTimer();
}
return true;
}
return false;
}
bool DisplayPacketCheckVersionRequestFinished(void)
{
if(DisplayPacket.BytesRead == VER_REQ_PACKET_BYTES)
{
UpdateSysFlag(FirmwareVerRequested,true);
return true;
}
return false;
}
bool DisplayPacketCheckPasswordFinished(void)
{
if(DisplayPacket.BytesRead == PASSWORD_PACKET_BYTES)
{
PasswordPacket = DisplayPacket.PasswordPacket;
UpdateSysFlag(PasswordReceived,true);
return true;
}
return false;
}
With this approach there is no serialization necessary because the union handles it. Also, syntax for copying packets is easy if there needs to be a backed up copy of the data. ie. PacketType = CommUnion.PacketType;
When a new packet is added. A struct is created and added to the union, new enum field is added and a case to the check finished switch statement that calls the function for this packet is added.
This requires the source code of the library to change on multiple lines which I am not fond of.
Is there a way to implement a similar functionality in a simpler way that requires external change while the compiled comm structure could stay the same?
Second Approach In Mind(Using Function Pointers):
- Get rid of the union. The control structure still stays because we need to keep track of current packet type and bytes read, escape etc.
- Get rid of enum method to create packet values. Create functions to register check finished functions for different packets and assign them values like the enum method. The register function is an array of function pointers to check functions.
- On startup all the packet structures are initialized.(Change in main program or project specific module not on the library.)
- Then all the packets are registered using the register function. This way the number of packets and the value of the escape character can be computed on runtime (or compile time if the compiler is smart).
- When you push a data byte the Check finished function pointer is used to call the function. This function serializes (or calls such function.) and handles the packet if the packet is fully received.
The benefit of this method is the instance of the packet is created (which needs to be done either way) and register is called. I feel like there is less things to change which should reduce human errors(ideally).
If anyone would discuss the advantages or disadvantages of these two methods with me or point me to a more streamlined way of doing things, I would appreciate it.
#pragma packor similar non-standard extension. Such code won't be portable between compilers. If 100% portable standard C is required, then there is no way around writing serialization/de-serialization routines. - Lundin