11
\$\begingroup\$

I'm currently trying to develop some code that will handle parsing and building of a custom binary protocol. The protocol itself is still fairly fluid but the basic core features are decided. These include

  • It has a start and end deliminator
  • It contains a header and payload set of data. The header is standard and contains information such as the payload type, time of sending etc
  • Any strings or chars will be Ascii characters

Basic format is: [STX][Header][Payload][Checksum][ETX]

Note: I can't use libraries like proto-buf .net as the protocol specification itself is outside of my control.

Any comments on code styling, design, implementation, best practices etc welcome.

Here is what I have come up with so far:

Data streams

public interface IDataInputStream
{
    string ReadString(int count);
    char ReadChar();
    int ReadInt32();
    short ReadIn16();
    byte ReadByte();
}

public class DataInputStream : IDataInputStream, IDisposable
{
    private readonly BinaryReader _reader;

    public DataInputStream(Stream stream)
    {
        _reader = new BinaryReader(stream, System.Text.Encoding.UTF8);
    }

    public string ReadString(int count)
    {
        return new string(_reader.ReadChars(count));
    }

    public char ReadChar()
    {
        return _reader.ReadChar();
    }

    public int ReadInt32()
    {
        return _reader.ReadInt32();
    }

    public short ReadIn16()
    {
        return _reader.ReadInt16();
    }

    public byte ReadByte()
    {
        return _reader.ReadByte();
    }

    public void Dispose()
    {
        _reader.Dispose();
    }
}

public interface IDataOutputStream
{
    void Write(char value);
    void Write(string value, int length);
    void Write(int value);
    void Write(short value);
    void Write(byte[] value);
}

public class DataOutputStream : IDataOutputStream, IDisposable
{
    private readonly BinaryWriter _writer;

    public DataOutputStream(Stream stream)
    {
        _writer = new BinaryWriter(stream);
    }

    public void Write(string value, int length)
    {
        var valueMinLength = value ?? string.Empty;

        if (valueMinLength.Length < length)
        {
            valueMinLength = valueMinLength.PadRight(length, '\0');
        }

        var bytes = System.Text.Encoding.UTF8.GetBytes(valueMinLength);
        Write(bytes);
    }

    public void Write(byte value)
    {
        Write(new byte[] { value });
    }

    public void Write(char value)
    {
        // In this protocol a char represents one byte however GetBytes of bitconverter treats the byte as unicode
        var bytes = BitConverter.GetBytes(value);
        Write(new byte[] { bytes[0] });
    }

    public void Write(int value)
    {
        // 4 bytes
        var bytes = BitConverter.GetBytes(value);
        Write(bytes);
    }

    public void Write(short value)
    {
        var bytes = BitConverter.GetBytes(value);
        Write(bytes);
    }

    public void Dispose()
    {
        _writer.Dispose();
    }

    public void Write(byte[] value)
    {
        WriteBytes(value);
    }

    private void WriteBytes(byte[] bytes)
    {
        _writer.Write(bytes);
    }
}

The main Packet envelope

public class Packet<T> : IPacketCheckSumWriter, IPacketReader where T : DataPayload
{
    public char Stx { get; private set; }
    public char Etx { get; private set; }
    public byte CheckSum { get; private set; }

    public HeaderPayload Header { get; private set; }
    public T Data { get; private set; }

    public Packet(
        HeaderPayload header,
        T payload)
        : this()
    {
        Header = header;
        Data = payload;
    }

    public Packet()
    {
        Stx = PacketConstants.Stx;
        Etx = PacketConstants.Etx;
    }

    public void Write(IDataOutputStream outputStream, IChecksum checkSumAlgorithm)
    {
        outputStream.Write(Stx);

        WritePacketData(outputStream);
        WriteChecksum(outputStream, checkSumAlgorithm);

        outputStream.Write(Etx);
    }

    public void Read(IDataInputStream inputStream)
    {
        Stx = inputStream.ReadChar();
        Header.Read(inputStream);
        Data.Read(inputStream);
        CheckSum = inputStream.ReadByte();
        Etx = inputStream.ReadChar();
    }

    private void WriteChecksum(IDataOutputStream outputStream, IChecksum algorithm)
    {
        var bytes = GetPacketData();
        CheckSum = algorithm.GetCheckSum(bytes);

        outputStream.Write(CheckSum);
    }

    private void WritePacketData(IDataOutputStream outputStream)
    {
        Header.Write(outputStream);
        Data.Write(outputStream);
    }

    private byte[] GetPacketData()
    {
        using (var stream = new MemoryStream())
        {
            using (var outputStream = new DataOutputStream(stream))
            {
                // First get the packet data bytes as this is what the algorithm is calculated on
                WritePacketData(outputStream);

                return stream.ToArray();
            }
        }
    }

Payload classes for the Header and Data

public class HeaderPayload : IPacketWriter, IPacketReader
{
    public char Identifier { get; set; }

    public char Command { get; set; }

    public int PacketId { get; set; }

    public int UnixTimeStamp { get; set; }

    public void Write(IDataOutputStream outputStream)
    {
        outputStream.Write(Identifier);
        outputStream.Write(Command);
        outputStream.Write(PacketId);
        outputStream.Write(UnixTimeStamp);
    }

    public void Read(IDataInputStream inputStream)
    {
        Identifier = inputStream.ReadChar();
        Command = inputStream.ReadChar();
        PacketId = inputStream.ReadInt32();
        UnixTimeStamp = inputStream.ReadInt32();
    }
}

public class FieldLength : Attribute
{
    public int Length { get; private set; }

    public FieldLength(int length)
    {
        Length = length;
    }
}

public abstract class DataPayload : IPacketWriter, IPacketReader
{
    public abstract void Write(IDataOutputStream outputStream);
    public abstract void Read(IDataInputStream inputStream);

    protected DataPayload()
    {
        // We want all strings to at least be an empty string so that we don't have to check for null all the time
        SetAllStringsEmpty();
    }

    protected void SetAllStringsEmpty()
    {
        var properties = GetType().GetProperties().Where(p => p.PropertyType == typeof(string));

        properties.ForEach(p => p.SetValue(this, string.Empty));
    }

    protected int GetFieldLength<T>(Expression<Func<T>> expr)
    {
        var body = ((MemberExpression) expr.Body);

        return GetFieldLength(body.Member.Name);
    }

    protected int GetFieldLength(string property)
    {
        var attribute = GetType().GetProperty(property).GetCustomAttribute<FieldLength>();

        if (attribute == null) return 1;

        return attribute.Length;
    }
}

Main Packet builder

public static class PacketConstants
{
    public static char Stx { get { return '$'; } }
    public static char Etx { get { return '*'; } }
}

public static class PacketCommands
{
    public const char RegistrationRequest = 'R';
}

public class PacketBuilder
{
    public Packet<T> CreateEmptyPayload<T>(char command, int packetId, DateTime dateCreated) where T: DataPayload
    {
        var payload = Activator.CreateInstance<T>();

        return new Packet<T>(new HeaderPayload
        {
            Command = command,
            Identifier = '1',
            PacketId = packetId,
            UnixTimeStamp = UnixTimeConverter.DateTimeToUnixTimestamp(dateCreated)
        }, 
        payload);
    }

    public byte[] Build<T>(Packet<T> packet, IChecksum checkSum) where T : DataPayload
    {
        using (var stream = new MemoryStream())
        {
            using (var dataWriter = new DataOutputStream(stream))
            {
                // First get the packet data bytes as this is what the checksum is calculated on
                packet.Write(dataWriter, checkSum);

                return stream.ToArray();
            }
        }
    }

    public Packet<T> Build<T>(byte[] data) where T : DataPayload
    {
        using (var stream = new MemoryStream(data))
        {
            using (var inputStream = new DataInputStream(stream))
            {
                var packet = new Packet<T>(new HeaderPayload(), Activator.CreateInstance<T>());
                packet.Read(inputStream);

                // TODO:  Validation on packet?

                return packet;
            }
        }
    }

    public char GetCommand(byte[] data)
    {
        return (char) data[1];
    }
}

Example data payload

public class RegistrationRequest : DataPayload
{
    [FieldLength(2)]
    public string FirmwareVersion { get; set; }

    public char HardwareVersion { get; set; }

    [FieldLength(15)]
    public string Imei { get; set; }

    [FieldLength(20)]
    public string Sim { get; set; }

    public char DeviceModel { get; set; }

    public override void Write(IDataOutputStream outputStream)
    {
        outputStream.Write(FirmwareVersion, GetFieldLength(() => FirmwareVersion ));
        outputStream.Write(HardwareVersion);
        outputStream.Write(Imei, GetFieldLength(() => Imei));
        outputStream.Write(Sim, GetFieldLength(() => Sim));
        outputStream.Write(DeviceModel);
    }

    public override void Read(IDataInputStream inputStream)
    {
        FirmwareVersion = inputStream.ReadString(GetFieldLength(() => FirmwareVersion));
        HardwareVersion = inputStream.ReadChar();
        Imei = inputStream.ReadString(GetFieldLength(() => Imei));
        Sim = inputStream.ReadString(GetFieldLength(() => Sim));
        DeviceModel = inputStream.ReadChar();
    }
}

And finally a couple of Unit tests

[TestClass]
public class RegistrationRequestTest
{
    [TestMethod]
    public void SerializesStxAndEtx()
    {
        var builder = new PacketBuilder();
        var packet = builder.CreateEmptyPayload<RegistrationRequest>(PacketCommands.RegistrationRequest, 1234, DateTime.Now);       

        var bytes = builder.Build(packet, new Checksum());

        Assert.AreEqual(PacketConstants.Stx, (char)bytes[0]);
        Assert.AreEqual(PacketConstants.Etx, (char)bytes[bytes.Length - 1]);
    }

    [TestMethod]
    public void SerializesHeader()
    {
        var dateCreated = DateTime.Now;
        var builder = new PacketBuilder();
        var packet = builder.CreateEmptyPayload<RegistrationRequest>(PacketCommands.RegistrationRequest, 1234, dateCreated);       

        var bytes = builder.Build(packet, new Checksum());

        Assert.AreEqual('1', (char)bytes[1]);
        Assert.AreEqual('R', (char)bytes[2]);
        Assert.AreEqual(1234, BitConverter.ToInt32(bytes, 3));
        Assert.AreEqual(UnixTimeConverter.DateTimeToUnixTimestamp(dateCreated), BitConverter.ToInt32(bytes, 7));
    }

    [TestMethod]
    public void SerializesDataPayloadToByteArray()
    {
        var builder = new PacketBuilder();

        var request = builder.CreateEmptyPayload<RegistrationRequest>(PacketCommands.RegistrationRequest, 1, DateTime.Now);       
        request.Data.HardwareVersion = 'A';
        request.Data.Imei = "123456789456789";
        request.Data.Sim = "12345678901234567890";
        request.Data.FirmwareVersion = "12";
        request.Data.DeviceModel = '3';

        var bytes = builder.Build(request, new Checksum());
        var deserialized = builder.Build<RegistrationRequest>(bytes);

        Assert.AreEqual(request.Data.HardwareVersion, deserialized.Data.HardwareVersion);
        Assert.AreEqual(request.Data.FirmwareVersion, deserialized.Data.FirmwareVersion);
        Assert.AreEqual(request.Data.Imei, deserialized.Data.Imei);
        Assert.AreEqual(request.Data.Sim, deserialized.Data.Sim);
        Assert.AreEqual(request.Data.DeviceModel, deserialized.Data.DeviceModel);
        Assert.AreEqual(request.CheckSum, deserialized.CheckSum);
    }
}

Example of a controller action

    [HttpPost]
    public HttpResponseMessage BinaryRequest(byte[] data)
    {
        var builder = new PacketBuilder();
        var command = builder.GetCommand(data);

        switch (command)
        {
            case PacketCommands.RegistrationRequest:
                var packet = builder.Build<RegistrationRequest>(data);
                // Do stuff with this packet
            default:
                throw new NotImplementedException("The command [" + command + "] is not currently supported");
        }
    }   
\$\endgroup\$
  • \$\begingroup\$ Am I right in assuming that you don't want any comments on the format of the protocol/packets, because "protocol specification itself is outside of my control"? \$\endgroup\$ – ChrisW Mar 18 '14 at 0:42
  • \$\begingroup\$ @ChrisW I guess comments won't hurt and if reasonable I might be able to sway the protocol writers, but in general yes, you are correct Chris. I'm mainly looking for comments on the code in regards to parsing the protocol. \$\endgroup\$ – dreza Mar 18 '14 at 1:01
  • \$\begingroup\$ @Jamal Why was my statement on the type of feedback removed? In the Ask Question page it specifically says to specify what type of feedback you are looking for? \$\endgroup\$ – dreza Mar 18 '14 at 1:02
  • \$\begingroup\$ I didn't remove it. It was sort of in an odd place at the very top, so I moved it just before the code block. That's also helpful so that the post preview will show the program's purpose. \$\endgroup\$ – Jamal Mar 18 '14 at 1:16
  • 2
    \$\begingroup\$ You're welcome. Sorry for the confusion. :-) \$\endgroup\$ – Jamal Mar 18 '14 at 1:24
4
\$\begingroup\$

Protocol

It's strange to use a "custom protocol". Has no-one invented one you can reuse?

In a binary protocol with STX and ETX I'm used to seeing DLE being used as well:

<DLE><STX> <PAYLOAD> <DLE><ETX>.

... with DLE in the payload being escaped as <DLE><DLE>.

I was surprised to find no "total packet length" or "total payload length" field in the packet header.

Your protocol doesn't handle variable-length strings in the payload well.

A one-byte checksum is pretty small.

This protocol doesn't recover from transmission errors well: if you lose a byte, then because you don't use <DLE>, you can't reliably find the start of the next packet (unless you guarantee that the STX char i.e. '$' is unique and never found in the header, not the payload, nor the checksum).

Text-based protocols can be easier to work with.

Code

Do you validate the received packet somewhere? Check that it starts with Stx, ends with Etx, and includes a valid checksum?

Your test code uses strings whose lengths are equal to the maximum field length; the receive code shows that short string will be zero-padded, but you don't have a unit-test for that.

BinaryReader is little-endian. Is your protocol little-endian too?

GetCommand returns data[1]. Should it return data[2]: maybe data[0] is the STX and data[1] is the Identifier?

\$\endgroup\$
  • \$\begingroup\$ 1) Validation is yet to be implemented. I was thinking of doing it in the PacketBuilder class. 2) Yes you are right. I do not have a unit test for that. I have not completed all of the unit tests yet. 3) Yes, it's little-endian. 4) You are right. I think GetCommand() is a bug. \$\endgroup\$ – dreza Mar 18 '14 at 1:59
  • \$\begingroup\$ Good points on the protocol. Yeah we wanted to use binary to safe on packet size as there are a-lot sent. I'm not sure if we are going to allow variable length strings but that is something I might follow up. \$\endgroup\$ – dreza Mar 18 '14 at 2:00
  • \$\begingroup\$ Sorry, but What is DLE? \$\endgroup\$ – dreza Mar 18 '14 at 2:01
  • \$\begingroup\$ @dreza DLE: see for example 2nd paragraph of page 2 of tools.ietf.org/html/rfc171 ... it's a way to ensure that your STX and ETX are unique in the stream: <DLE><STX> = start of packet; <DLE><ETX> are end of packet; <DLE> inside the packet is sent as <DLE><DLE> to ensure that <DLE><STX> and <DLE><ETX> are never sent within the packet. <DLE> is a byte value ('0x10'), and so are STX and ETX: en.wikipedia.org/wiki/Control_character \$\endgroup\$ – ChrisW Mar 18 '14 at 2:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.