space_packet_parser icon indicating copy to clipboard operation
space_packet_parser copied to clipboard

Fix incorrect parsing of `StringDataEncoding` element

Open medley56 opened this issue 1 year ago • 2 comments

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:

  1. We currently allow StringDataEncoding/SizeInBits/Fixed to contain DiscreteLookup and DynamicValue. This is incorrect as Fixed may only contain the FixedValue element. (p. 72).
  2. 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).
  3. 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.

medley56 avatar Mar 19 '24 21:03 medley56

@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.

medley56 avatar Mar 19 '24 21:03 medley56

Copy that—thanks for the heads up.

cgobat avatar Mar 19 '24 23:03 cgobat