Heterogenous register types in yaml schema

[bruno-f-cruz]

@Poofjunior Thanks for the great input and raising this requirement, I agree having more flexibility in passing around packed custom structures with possibly heterogeneous types is important.

Following the meeting, I was playing around with the YAML schema and actually there is a small kink that doesn't allow it to represent heterogeneous types out-of-the-box (yet). It is definitely fixable though and this is a use case we should indeed support so I will flag an issue to fix it. Nevertheless, for aligned types, you could do:

%YAML 1.1
---
# yaml-language-server: $schema=https://raw.githubusercontent.com/harp-tech/reflex-generator/main/schema/device.json
device: harpy
whoAmI: 0000
firmwareVersion: "0.1"
hardwareTargets: "0.0"
registers:
  Accelerometer:
    address: 32
    type: S16
    length: 3
    access: Event
    payloadSpec:
      X:
        offset: 0
        description: Represents the x-coordinate of the payload.
      Y:
        offset: 1
        description: Represents the y-coordinate of the payload.
      Z:
        offset: 2
        description: Represents the z-coordinate of the payload.

This will generate in the high-level interface the type AccelerometerPayload with the following signature (edited for conciseness):

public struct AccelerometerPayload
{
    public AccelerometerPayload(short x, short y, short z)
    {
        X = x;
        Y = y;
        Z = z;
    }

    public short X;
    public short Y;
    public short Z;
}

And the auto-generated converters:

static AccelerometerPayload ParsePayload(short[] payload)
{
    AccelerometerPayload result;
    result.X = payload[0];
    result.Y = payload[1];
    result.Z = payload[2];
    return result;
}

static short[] FormatPayload(AccelerometerPayload value)
{
    short[] result;
    result = new short[3];
    result[0] = value.X;
    result[1] = value.Y;
    result[2] = value.Z;
    return result;
}

It is actually possible using the message manipulation API to marshal arbitrary payload structs directly (with the equivalent of memcpy), and having an easy way to specify something like this in the spec is definitely interesting. I think I have a solution but need to go through a few edge cases more carefully to make sure it will be workable. Thanks for the push! I'll post here once I have something more complete.

Originally posted by @glopesdev in #27 (comment)

[glopesdev]

Diving further into this issue, I can see two possible options which we can discuss at the next SRM.

Bulk conversion of array payload

The payload of a Harp message is always available as a byte[], so as long as the heterogeneous structure is well-defined and matches the binary layout of the message payload, a bulk conversion might be simple and efficient, e.g. see bonsai-rx/bonsai#2110 and example ParsePayload below:

unsafe static BinaryDataPayload ParsePayload(byte[] payload)
{
    BinaryDataPayload result;
    var resultPtr = &result;
    fixed (byte* payloadPtr = payload)
        Buffer.MemoryCopy(payloadPtr, resultPtr, sizeof(BinaryDataPayload), payload.Length);
    return result;
}

Generalized conversion following payload specification

We could generalize the existing conversion procedure and expand it to allow more flexible assignment. Reusing the existing mechanism would naturally extend all bit masking and handling to heterogeneous types. Although less efficient than the bulk copy conversion presented above, this has the flexibility of allowing much more complex conversion logics.

The main missing ingredient would be to allow either specifying the length of a payloadSpec member, or its native Harp type (i.e. U8, U16, F32, etc). This new attribute would be interpreted in terms of the native type of the register and used to slice the array prior to the conversion proper. This would allow for example reading multi-byte integers from a byte array payload, or more complex packing of primitives (e.g. nesting).

Example:

registers:
  BinaryData:
    address: 32
    type: U8
    length: 5
    access: Event
    payloadSpec:
      OperationMode:
        offset: 0
        maskType: OperationMode
        mask: 0x3
      DumpRegisters:
        offset: 0
        interfaceType: bool
        mask: 0x8
      Counter:
        offset: 1
        length: 4
        interfaceType: int

The above would generate the payload:

public struct BinaryDataPayload
{
    public OperationMode OperationMode;
    public bool DumpRegisters;
    public int Counter;
}

Alternatively, we could support both approaches by allowing an attribute toggle in the device metadata YML file.

[bruno-f-cruz]

For reference, this is how we ended up implementing a custom solution in one of our interfaces:

https://github.com/AllenNeuralDynamics/harp.device.cuttlefish/blob/main/software/bonsai/Interface/AllenNeuralDynamics.Cuttlefish/ConfigurePwm.cs

[bruno-f-cruz]

If one sees the device.yml as the spec for the device interface, I find the second solution far more interesting. I am sure we would need to review edge cases in the yaml definition, but having the ability to trickle these definitions to docs and data ingestion pipelines is an exciting possibility.
Regarding the device.yml, I am wondering if a recursive register definition would be an easy way to approach the syntax. There are obviously properties that are not necessary (e.g. MessageType and Address), but the rest of the data model could in theory be re-used (length, payloadspec, bitmasks/groupmasks, etc...)

[glopesdev]

Regarding the device.yml, I am wondering if a recursive register definition would be an easy way to approach the syntax. There are obviously properties that are not necessary (e.g. MessageType and Address), but the rest of the data model could in theory be re-used (length, payloadspec, bitmasks/groupmasks, etc...)

@bruno-f-cruz I agree a recursive definition might be well suited here. I wouldn't use the definition of register given the extraneous properties you described, but payloadMember might be a good candidate, or we can distill further the recursive part of the schema into its own definition:

protocol/schema/registers.json

Line 108 in 55342cf

"payloadMember": {

This might take a while to implement, but perhaps we can agree on at least a draft specification for the schema.

[glopesdev]

@bruno-f-cruz I agree a recursive definition might be well suited here.

There is only one thing we have to be careful when discussing this, which is the potential for feature-creep complications. Nested payload definitions would greatly complicate the constructor operators since you would have properties which are themselves made up of other data types.

Simply recursing on the schema would also not resolve issues of shared payload types, so it might in practice be more useful to discuss how to handle references to explicitly defined payload types which could be part of the schema the same way we do with bitmasks and group masks.

[glopesdev]

Following harp-tech/reflex-generator#87, we introduced support for allowing payload specs to be composed of multiple array elements from a single array register. This would allow being explicit about type conversion generation and validation.

This would allow more easily specifying types of different sizes, e.g. int created from a U16, a bool from a U32, or one ushort[] from eight U8 (total 4 elements).

If this is found useful, we can add it explicitly to the core device schema (payloadMember).

One outstanding question is whether the length should be in register words, or in interfaceType number of elements. Currently it is defined in register words, which makes it the same unit as offset, and probably is more intuitive. Arrays could be inferred automatically by simply determining automatically how many elements of interfaceType fit into the stated number of words.