-
Notifications
You must be signed in to change notification settings - Fork 76
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
end-of-PDU handling and DOP resolution cleanups #300
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this allows to continue in non-strict mode, albeit with undefined results. Signed-off-by: Andreas Lauser <[email protected]> Signed-off-by: Gerrit Ecke <[email protected]>
we now hopefully implement all specified DOPs, so we can be more strict if a referenced DOP cannot be resolved. Signed-off-by: Andreas Lauser <[email protected]> Signed-off-by: Gerrit Ecke <[email protected]>
so far, `DynamicLengthField` and `EndOfPduField` only allowed lists. Signed-off-by: Andreas Lauser <[email protected]> Signed-off-by: Gerrit Ecke <[email protected]>
an element of a field is at the end of a PDU if the field is at the end of the PDU and the item is the last element of the field. Note that even though some pathetic cases where this makes a difference can be constructed, I have doubts if this matters in practice. Signed-off-by: Andreas Lauser <[email protected]> Signed-off-by: Gerrit Ecke <[email protected]>
Note that this does not make a difference because all items of fields are structures, which need to set the origin position to the beginning of the structure anyway, but IMO it is conceptually clearer to explicitly set the origin position in fields as well. Signed-off-by: Andreas Lauser <[email protected]> Signed-off-by: Gerrit Ecke <[email protected]>
…ription` it turns out that these classes are yet another multiplexer mechanism. Signed-off-by: Andreas Lauser <[email protected]> Signed-off-by: Gerrit Ecke <[email protected]>
kayoub5
reviewed
May 7, 2024
The reason we expect `Sequence` instead of `Iterable` is because we use the `len()` method which is not available for `Iterable`. thanks to [at]kayoub5 for the catch! Signed-off-by: Andreas Lauser <[email protected]> Signed-off-by: Gerrit Ecke <[email protected]>
kayoub5
approved these changes
May 7, 2024
@andlaus I recommend renaming the pr, to make the changelog clear |
andlaus
changed the title
Dynamic endmarker fields drivebys
end-of-PDU handling and DOP resolution cleanups
May 7, 2024
ok, done. the new title now reflects the two most important changes... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains a bunch of fixes for smaller unrelated issues which I've stumbled over while working on implementing dynamic endmarker fields.
Andreas Lauser <[email protected]>, on behalf of MBition GmbH.
Provider Information