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.

Sign up for GitHub

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

Binary exporter support for structs. #420

Closed
wants to merge 10 commits into from
Closed

Binary exporter support for structs. #420

Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
847ed46
Binary parser bug fix.
Dec 19, 2023
5fda216
Merge branch 'COVESA:master' into master
UlfBj Aug 8, 2024
edbfc8f
Merge branch 'COVESA:master' into master
UlfBj Oct 23, 2024
71e3e8b
Merge branch 'COVESA:master' into master
UlfBj Oct 23, 2024
f3ac54e
Binary exporter support for structs.
UlfBj Oct 23, 2024
4cba86c
Merge branch 'master' into master
UlfBj Oct 24, 2024
4c64632
Update binary.py
UlfBj Oct 24, 2024
4a8c9a3
Build chck error fix.
UlfBj Oct 24, 2024
caf4823
Merge conflict resolution.
UlfBj Oct 24, 2024
fc2f3b3
binary.py cleanup.
UlfBj Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/vss_tools/exporters/binary.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ def export_node(node: VSSNode, generate_uuid, f: BinaryIO):
@clo.overlays_opt
@clo.quantities_opt
@clo.units_opt
@clo.types_opt
@clo.types_output_opt
@clo.extend_all_attributes_opt
def cli(
vspec: Path,
output: Path,
Expand All @@ -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,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not using that, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

extend_all_attributes is probably not needed. The other two are referenced at other places in the code so I assume they are needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah so please get rid of the unused ones

):
"""
Export to Binary.
"""

tree, _ = get_trees(
tree, datatype_tree = get_trees(
vspec=vspec,
include_dirs=include_dirs,
aborts=aborts,
Expand All @@ -140,9 +146,17 @@ def cli(
uuid=uuid,
quantities=quantities,
units=units,
types=types,
overlays=overlays,
)

log.info("Generating binary output...")
if datatype_tree:
if types_output:
Copy link
Collaborator

Choose a reason for hiding this comment

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

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 ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test shows that if --types-output is not used in the make command the second if statement becomes false.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

with open(str(types_output), "wb") as f:
export_node(datatype_tree, uuid, f)
log.info("Binary datatype tree output generated in %s", types_output)

with open(str(output), "wb") as f:
export_node(tree, uuid, f)
log.info("Binary output generated in %s", output)
log.info("Binary main tree output generated in %s", output)