Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect parsing of StringDataEncoding element #30

Open
medley56 opened this issue Mar 19, 2024 · 2 comments
Open

Fix incorrect parsing of StringDataEncoding element #30

medley56 opened this issue Mar 19, 2024 · 2 comments
Assignees
Labels
breaking This involves a breaking change bug Something isn't working

Comments

@medley56
Copy link
Owner

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 medley56 changed the title Fix incorrect parsing of StringDataEncoding/SizeInBits element Fix incorrect parsing of StringDataEncoding element Mar 19, 2024
@medley56
Copy link
Owner Author

@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 medley56 added the bug Something isn't working label Mar 19, 2024
@cgobat
Copy link
Contributor

cgobat commented Mar 19, 2024

Copy that—thanks for the heads up.

@medley56 medley56 added the breaking This involves a breaking change label Mar 20, 2024
@medley56 medley56 added this to the Version 5.0.0 Release milestone Mar 20, 2024
@medley56 medley56 changed the title Fix incorrect parsing of StringDataEncoding element Fix incorrect parsing of StringDataEncoding element Mar 20, 2024
@medley56 medley56 self-assigned this Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This involves a breaking change bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants