HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Binary protocol variability

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
protocolbinaryvariability

Problem

Please note: An updated solution is here

Let’s imagine that we designed more or less universal firmware for a microcontroller unit to be used across multiple types of devices. So we have something common in the binary data exchange protocol, and something different.

The most common way to define incoming command parsing is usually about switching over one huge enum with all the command discriminators (command selection codes), so system knows what command to read. The problem is enum + switch are difficult to maintain and almost impossible to distribute code between helper library, shared controller, and specific device related projects.

Proposed solution has been divided into three libraries where we can redefine protocol specifics.

namespace Company.Hardware

First of all, classes to map commands to discriminators:

public class Commands
{
    public static readonly Commands None = new Commands();

    protected Commands()
    {
    }

    public virtual Type this[int discriminator]
    {
        get { throw new NotSupportedException(); }
    }

    public Commands Include(int discriminator) =>
        new Commands(discriminator, this);
}

class Commands : Commands
{
    public Commands(int discriminator, Commands next)
    {
        Discriminator = discriminator;
        Next = next;
    }

    public override Type this[int discriminator] =>
        discriminator == Discriminator ? 
            typeof(TCommand) : 
            Next[discriminator];

    int Discriminator { get; }
    Commands Next { get; }
}


Mapping schema might be defined as:

Commands Commands = Commands.None
        .Include(10),
        .Include(20);


We can get command Type back:

Type type = Commands[20];


Next class allows to read and log low level primitives of the protocol; command components like discriminator itself. We will inherit it later to define more specific versions of the protocol. It is basically a tokenizer.

```
public abstract class Pr

Solution

I dont like the method ProtocolReader.Write that logs a value and return them. IMHO that makes the code harder to read.

For instance compare:

public Version ReadVersion() =>
    Write("version={0}",
        new Version(
            Reader.ReadInt32(),
            Reader.ReadInt32()));
// ...

public Type ReadDiscriminator() =>
    Commands[
        Write("discriminator={0}", 
            Reader.ReadInt32())];


with

public Version ReadVersion()
{
    var major = Reader.ReadInt32();
    var minor = Reader.ReadInt32();
    var version = new Version(major, minor);
    Log("version={0}", version);
    return version;
}

// ...

public Type ReadDiscriminator()
{
    var discriminator = Reader.ReadInt32();
    Log("discriminator={0}", discriminator);
    return Commands[discriminator];
}


In general the code looks very abstract for me. I am not able to appraise if that is reasonable because I don't know the complexity of the use case...

For example

The ControllerReader knows how to read a Version and the VersionCommand gets the ControllerReader and calls the method to read the version. So, if a new command will be added, is it required to create a new command and add a method to the ControllerReader?

If that code is used like a framework I would expect, that extending new (or modifying existing) commands should be possible by adding (or modifing) just one class.

Code Snippets

public Version ReadVersion() =>
    Write("version={0}",
        new Version(
            Reader.ReadInt32(),
            Reader.ReadInt32()));
// ...

public Type ReadDiscriminator() =>
    Commands[
        Write("discriminator={0}", 
            Reader.ReadInt32())];
public Version ReadVersion()
{
    var major = Reader.ReadInt32();
    var minor = Reader.ReadInt32();
    var version = new Version(major, minor);
    Log("version={0}", version);
    return version;
}

// ...

public Type ReadDiscriminator()
{
    var discriminator = Reader.ReadInt32();
    Log("discriminator={0}", discriminator);
    return Commands[discriminator];
}

Context

StackExchange Code Review Q#131216, answer score: 3

Revisions (0)

No revisions yet.