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 sequence additions (#105) #106

Merged
merged 3 commits into from
Nov 2, 2023
Merged

Fix sequence additions (#105) #106

merged 3 commits into from
Nov 2, 2023

Conversation

gabhijit
Copy link
Collaborator

No description provided.

examples/tests/18-e2sm.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@gth828r gth828r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the tests and verified that the new one passes for me. I made a comment about some code comments that could be removed, but otherwise I think this is OK. I don't have enough background on the generator, parser, or resolver to provide a full code review in a timely fashion.

Sequences Additions (those after `...` Extension Marker) were treated as
components from the root components. This resulted in structures with
components being treated as root components and caused errors in codecs
(due to incorrect number of components determined).

Fixed in the parser and resolver. Also defined a new `enum` type to deal
with the fact that the sequence additions can be addition groups or
components (stand alone).
Initially we were simply parsing E2SM specificiation, Now also added
codec example with codec round-trip.
When we were encoding a value of '0' for a constrained whole number, the
required length determinent was not getting the proper value. Fixed this
to get the issue.
@gabhijit gabhijit force-pushed the fix-sequence-additions branch from 09a1801 to 280baf8 Compare November 2, 2023 04:02
@gabhijit gabhijit merged commit 7fdd574 into master Nov 2, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants