jefffhaynes / BinarySerializer

A declarative serialization framework for controlling formatting of data at the byte and bit level using field bindings, converters, and code.
MIT License
290 stars 62 forks source link

Combination of [FieldLength] and [FieldCount] causes exception during serialization #203

Closed WilbertOnGithub closed 1 year ago

WilbertOnGithub commented 1 year ago

Something else that I've noticed when using the library. There is an attribute FieldLength that defines the actual serialization in bytes of a given field/property. On the other hand there is FieldCount where we can limit the number of elements in an array (so if we enter more entries into an array, only the specified number will be serialized).

It looks like those two attributes cannot be combined. See the following definition:

[FieldOrder(4)] 
[FieldLength(2)] 
[FieldCount(8)] 
public int[] PowerLevel { get; set; }

My goal here was to define an array of PowerLevels with 8 elements where an individual Powerlevel would be serialized as two bytes (but represented as an int in the definition). When using this combination, I get the following exception from the library.

System.InvalidOperationException
Unable to write beyond end of stream limit.
   at BinarySerialization.BoundedStream.WriteCheck(FieldLength length)
   at BinarySerialization.BoundedStream.WriteImpl(Byte[] buffer, FieldLength length)
   at BinarySerialization.BoundedStream.Write(Byte[] buffer, FieldLength length)
   at BinarySerialization.AsyncBinaryWriter.Write(Byte[] data, FieldLength fieldLength)
   at BinarySerialization.Graph.ValueGraph.ValueValueNode.Serialize(AsyncBinaryWriter writer, Object value, SerializedType serializedType, FieldLength length)
   at BinarySerialization.Graph.ValueGraph.PrimitiveArrayValueNode.PrimitiveCollectionSerializeOverride(BoundedStream stream, Object boundValue, ValueValueNode childSerializer, SerializedType childSerializedType, FieldLength itemLength, Nullable`1 itemCount)
   at BinarySerialization.Graph.ValueGraph.PrimitiveCollectionValueNode.SerializeOverride(BoundedStream stream, EventShuttle eventShuttle)
   at BinarySerialization.Graph.ValueGraph.ValueNode.SerializeInternal(BoundedStream stream, Func`1 maxLengthDelegate, EventShuttle eventShuttle, Boolean measuring)
   at BinarySerialization.Graph.ValueGraph.ValueNode.Serialize(BoundedStream stream, EventShuttle eventShuttle, Boolean align, Boolean measuring)

If I define it like this, it does work but it forces me to define as a ushort so that it will be represented as two bytes.

    [FieldOrder(4)]
    //[FieldLength(2)]
    [FieldCount(8)]
    public ushort[] PowerLevel { get; set; }

I think I stumbled onto a scenario that probably nobody else uses. Could someone shed some light on this?

bevanweiss commented 1 year ago

I think your understanding of the "FieldCount" is a little off...

On the other hand there is FieldCount where we can limit the number of elements in an array

The FieldCount is the same as FieldLength, but for 'field units' instead of bytes. So having both only makes sense in one situation: When your 'field unit' is in bytes (i.e. it is a List/byte[]), and when you use the same value for both FieldLength and FieldCount (i.e. you have 1 'unit' per byte..) This is exactly the situation given in the documentation, where it states in this instance they are interchangable: https://github.com/jefffhaynes/BinarySerializer#fieldcountattribute

Consider your example:

[FieldLength(2)] [FieldCount(8)]

You are saying: "I have a field, and it is 2 bytes long, and consists of 8 integers (32 bytes)."

If you want to have a limit on the number of items serialized/deserialized, then you probably want to go about this a different way (I can't think of how deserialization would work in this fashion though... all the situations I can image would use a termination field, or a predefined count/length). Serialization should be easy to just pre-process and trim down the size of the instance (i.e. if it's a List then just replace it with a copy which is clamped to the item count you desire)

bevanweiss commented 1 year ago

I've re-read what you're after. You could try the following:

    [FieldOrder(4)] 
    [SerializeAs(SerializedType=SerializedType.UInt2)]
    [FieldCount(8)]
    public Int32 PowerLevel { get; set; }

Although I'm unsure why you'd want to do this still. If you have this defined as an Int32 then all values above +65535 and below 0 won't be serializable. But according to your definition would be valid.

It's generally better to have them align, unless you're going to have additional compliance checking code. I also note that you've swapped between signed and unsigned values. Did you mean to do this?

jefffhaynes commented 1 year ago

I need to look back at this but it's possible this is currently not well supported. I don't think attaching SerializeAs to an array will do anything useful (maybe it should?).

However, I would recommend going with ushort here. The intent of these structures is to represent your serialized format; not necessary to provide the ideal inward facing representation of the data. For that, I would suggest making it clear how you want to transform the data (e.g. straight casting, checking, etc.) and converting to the internal representation you want to use (e.g. int, float, etc.).

WilbertOnGithub commented 1 year ago

Yeah, you have a valid remark there Jeff. Will look into your suggestion.

Thanks for the help. Know that your library is currently being used in our project where we do a lot of serialization of messages to send commands over a serial port to firmware. The performance (and ease of use) of the library is fine work.