Binary exporter support for structs. #420
Conversation
Signed-off-by: Ulf Bjorkengren <[email protected]>
Signed-off-by: Ulf Bjorkengren <[email protected]>
src/vss_tools/exporters/binary.py
Outdated
| @@ -126,12 +129,15 @@ def cli( | |||
| overlays: tuple[Path], | |||
| quantities: tuple[Path], | |||
| units: tuple[Path], | |||
| types: tuple[Path], | |||
| types_output: Path, | |||
| extend_all_attributes: bool, | |||
There was a problem hiding this comment.
Not using that, right?
There was a problem hiding this comment.
extend_all_attributes is probably not needed. The other two are referenced at other places in the code so I assume they are needed.
There was a problem hiding this comment.
Yeah so please get rid of the unused ones
|
Rebase needed to solve conflicts. Let me know if you experience any problems |
|
@erikbosch , It seems uuid should no longer be used? Can it just be removed as a parameter in the export_node() call? |
It is already removed from that on master |
| log.info("Generating binary output...") | ||
| if datatype_tree: | ||
| if types_output: |
There was a problem hiding this comment.
What happens if types_output is not defined?
Usually other exporters export the types then bundled into the main file.
I think either it needs to throw an error in that case for the user or defining a default types output file or ...
There was a problem hiding this comment.
Test shows that if --types-output is not used in the make command the second if statement becomes false.
There was a problem hiding this comment.
does not really answer my question because if datatype_tree is still true then but we are not doing anything with that info in that case
There was a problem hiding this comment.
Ulf - I suggest you add an else statement, something like
else:
log.warning("To generate datatype tree output you must specify wanted output file")
or
else:
log.warning("Ignoring type tree generation as no file name specified")
... that gives a hint that they maybe have forgotten to specify output file.
Signed-off-by: Ulf Bjorkengren <[email protected]>
Signed-off-by: Ulf Bjorkengren <[email protected]>
src/vss_tools/exporters/binary.py
Outdated
| with open(str(output), "wb") as f: | ||
| export_node(tree, f) | ||
| log.info("Binary output generated in %s", output) | ||
| log.info("Binary main tree output generated in %s", output) | ||
|
|
There was a problem hiding this comment.
pre-commit complains on this empty line (i.e. two end of line at end of file, should only be one.)
There was a problem hiding this comment.
@UlfBj - if you just remove a line break here then two of the CI-failures should dissappear
|
MoM:
|
|
If you add the else statement suggested and remove the extra line break at end of file I think this PR should be good to go. One idea - should you possibly say something about how to use the new feature in |
|
MoM:
|
Signed-off-by: Ulf Bjorkengren <[email protected]>
|
Updted according to comments but did not fix it. |
Based on COVESA#420 Signed-off-by: Ulf Bjorkengren <[email protected]>
Based on COVESA#420 Signed-off-by: Ulf Bjorkengren <[email protected]> Signed-off-by: Erik Jaegervall <[email protected]>
Based on #420 Signed-off-by: Ulf Bjorkengren <[email protected]> Signed-off-by: Erik Jaegervall <[email protected]>
|
Merged through #428 |
No description provided.