space_packet_parser
space_packet_parser copied to clipboard
Fix incorrect parsing of `StringDataEncoding` element
Oddly, the StringDataEncoding/SizeInBits element does not behave the same as the BinaryDataEncoding/SizeInBits element. This led to some mistakes in the way StringDataEncoding elements are parsed and the way string data is returned.
In particular, known issues:
- We currently allow StringDataEncoding/SizeInBits/Fixed to contain DiscreteLookup and DynamicValue. This is incorrect as Fixed may only contain the FixedValue element. (p. 72).
- We do not currently support the StringDataEncoding/Variable element. The Variable element functions similarly to SizeInBits with its TerminationChar and LeadingSize functionality but also allows DiscreteLookup and DynamicValue as well. (p. 73).
- The way we parse strings is actually incorrect. The spec says that for strings encoded with SizeInBits (as opposed to Variable), the Fixed/FixedValue size is the maximum size of the string. The LeadingSize and TerminationChar elements actually provide a mechanism for deriving a substring from the full string encoding rather than actually specifying a dynamic size. This is actually convenient in many ways and should be handled by returning the full Fixed/FixedValue string as the raw value and then the substring (using LeadingSize or TerminationChar) as a derived value.
@cgobat In reading through the green book I realized there are some fundamental issues with the way we use StringDataEncoding. I think these issues are mostly separate from the work you've done with #26 but I wanted to let you know.
Copy that—thanks for the heads up.