Allow interior mutability for the context, read_until_with_ctx to allow ASTERIX "Extended Length Data Items"#646
Open
goto40 wants to merge 34 commits intosharksforarms:masterfrom
Conversation
Contributor
Author
|
Note: I still get a failure (independently of my change?): |
Contributor
Author
|
Ah. I understand: I changed an error message. That's why the compile error check falls. I will have a look... |
140d973 to
7758a30
Compare
ecad1bc to
e34d40d
Compare
5b57af6 to
148df77
Compare
Contributor
Author
|
... Still working on the coverage |
Owner
|
@goto40 thanks for the PR! I haven't gone through it yet - but I'm not too worried about coverage, it's more important to me that the introduced features have tests |
Contributor
Author
|
Ok. Thank you for the info. Please let me know if I should add more info. |
This file contains hidden or 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
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.
TLDR; --> see "Main Changes"
Motivation: "Extended Length Data Items" consist of Arrays of data element like
{x: u32, y: u32, fx: bool}. Thefxis the control element which indicates the end of the array (fx==false/fx==0).However, user code shall not be aware of the
fxfield - it should only be used for I/O and be always correct: for writing this means to setfx=falseonly for the last element. For reading it means to readuntil fx==false. Importantly, for the programmer/user the data element look like{x: u32, y: u32}andfxis completely temporary (tempandtemp_value).Moreover, there may be an array of such elements. In this array we need access to the
fxinformation (readuntil). If thefxfield is completely temporary, you need to pass the info through the context back to the array (see exampleuntil_with_ctx) and set it in the inner element (seeread_post_processingin the example above). We also need to allow a separatectxfor the read/write case (seectxandwriter_ctx), since the array length is only known in the writer case.I made some (non-breaking) changes to
deku, which enabled me to implement the above technique (allowing an implementation of "Extended Length Data Items" of ASTERIX EUROCONTOL data structures).Main changes:
Ctx) must beCopyin some calls.It is changed to
Clone, this allows to add interior mutability in the context(like
Rc<Cell<bool>>). This is no breaking change, sinceCopyimpliesClone.until_with_ctx: likeuntil, but it also gets the context.writer_ctxfor fields: overridesctxfor the writer case - this allowsa different context value for reader and writer (the reader gets the original
ctx).read_post_processing: code which is inserted after a field isread (including a temp field). You have full context access (and you can modify
the context there).
Additional changes: