Removed C dependency from binary exporter (#395)

* Removed C dependency from binary exporter

Signed-off-by: Sebastian Schildt <[email protected]>

.github/workflows/buildcheck.yml

@@ -43,10 +43,6 @@ jobs:
       - name: flake8
         run: |
           poetry run flake8 src tests contrib
-      - name: Build binary library (Needed for pytest)
-        run: |
-          cd binary
-          make
       - name: Run tests
         run: |
           poetry run pytest --cov=vss_tools --cov-report=term-missing --cov-fail-under=90

binary/README.md

@@ -3,32 +3,15 @@ The binary toolset consists of a tool that translates the VSS YAML specification
 and two libraries that provides methods that are likely to be needed by a server that manages the VSS tree, one written in C, and one in Go.<br>
 
 ## Binary Parser Usage
-The translation tool can be invoked via the make file available on the VSS repo (https://github.com/COVESA/vehicle_signal_specification):
 
 ```bash
-make binary
-```
-or, by invoking all tools:
-
-```bash
-make all
-```
-
-To **run the binary tool without using the make file**, the binary tool library must first be built in the
-binary directory, then the binary exporter of `vspec exporter` is executed in the root directory:
-
-```bash
-cd binary
-gcc -shared -o binarytool.so -fPIC binarytool.c
-cd <vss-root>
-vspec export binary -u spec/units.yaml --vspec spec/VehicleSignalSpecification.vspec -o vss.binary -b binary/binarytool.so
+vspec export binary -u spec/units.yaml --vspec spec/VehicleSignalSpecification.vspec -o vss.binary
 ```
 
 where `vss.binary` is the tree file in binary format.
 
-Current version is found at https://github.com/COVESA/vehicle_signal_specification/blob/master/VERSION.
-
 ### Validation
+
 Access Control of the signals can be supported by including the extended attribute validate in each of the nodes.
 This attribute is used by the VISSv2 specification.
 More information can be found in:
@@ -37,12 +20,11 @@ In case the validate attribute is added to the nodes, it must be specified when
 using the extended attributes flag `-e`:
 
 ```bash
-cd ..  # continue from the previous example
-vspec export binary -e validate -u ./spec/units.yaml --vspec ./spec/VehicleSignalSpecification.vspec -o vss.binary -b binary/binarytool.so
+vspec export binary -e validate -u ./spec/units.yaml --vspec ./spec/VehicleSignalSpecification.vspec -o vss.binary
 ```
 
-
 ## Tool Functionalities
+
 The two libraries provides the same set of methods, such as:
 
 - to read the file into memory,
@@ -57,11 +39,13 @@ Each library is also complemented with a testparser that uses the library to tra
 A textbased UI presents different parser commands, that is then executed and the results are presented.
 
 ### C parser
+
 To build the testparser from the c_parser directory:
 
 ```bash
 cc testparser.c cparserlib.c -o ctestparser
 ```
+
 When starting it, the path to the binary file must be provided. If started from the c_parser directory,
 and assuming a binary tree file has been created in the VSS parent directory:
 
@@ -70,11 +54,13 @@ and assuming a binary tree file has been created in the VSS parent directory:
 ```
 
 ### Go parser
+
 To build the testparser from the go_parser directory:
 
 ```bash
 go build -o gotestparser testparser.go
 ```
+
 When starting it, the path to the binary file must be provided. If started from the go_parser directory,
 and assuming a binary tree file has been created in the VSS parent directory:
 
@@ -118,9 +104,9 @@ The nodes are written into the file in the order given by a recursive method as
 
 ```python
 def traverseAndWriteNode(thisNode):
-	writeNode(thisNode)
-	for i = 0 ; i < thisNode.Children ; i++:
-		traverseAndWriteNode(thisNode.Child[i])
+   writeNode(thisNode)
+   for i = 0 ; i < thisNode.Children ; i++:
+      traverseAndWriteNode(thisNode.Child[i])
 ```
 
 When reading the file the same recursive pattern must be used to generate the correct VSS tree, as is the case for all the described tools.

src/vss_tools/vspec/vssexporters/vss2binary.py

@@ -7,9 +7,27 @@
 # SPDX-License-Identifier: MPL-2.0
 
 # Convert vspec tree to binary format
+#
+# This is the pure Python version of the binary exporter. The
+# format has been taken from the C version at
+# https://github.com/COVESA/vss-tools/blob/4.2/binary/binarytool.c
+# This is a basic "length"/"data" format, where the length is in front
+# of each field, and the field order is fixed.
+#
+# The length fields are mostly 8 bit (i.e. long strings/paths are not
+# supported) with the exception of description and allowed (16 bit).
+# The order of fields (where each field is composed of
+# fieldlength + fielddata) is:
+#
+# name (vsspath), type, uuid, description, datatype, min, max, unit,
+# allowed, default, validate
+#
+# if a field is not present (e.g. min, max, unit, allowed, default, validate),
+# the length is 0.
 
-import ctypes
+import struct
 from pathlib import Path
+from typing import BinaryIO
 
 import rich_click as click
 
@@ -39,37 +57,56 @@ def intToHexChar(hexInt):
         return chr(hexInt - 10 + ord("A"))
 
 
-def export_node(cdll: ctypes.CDLL, node: VSSNode, generate_uuid, out_file: str):
-    uuid = "" if node.uuid is None else node.uuid
+# Create a struct containing the length of the string as uint8
+#  and the string itself
+def create_l8v_string(s: str) -> bytes:
+    pack = struct.pack(f"{len(s)+1}p", s.encode())
+    # log.debug(f"create_l8v_string: {s} as {pack.hex()}")
+    return pack
+
+
+# Create a struct containing the length of the string as uint16
+#  and the string itself
+def create_l16v_string(s: str) -> bytes:
+    pack = struct.pack(f"=H{len(s)}s", len(s), s.encode())
+    # log.debug(f"create_l16v_string: {s} as {pack.hex()}")
+    return pack
+
+
+def export_node(node: VSSNode, generate_uuid, f: BinaryIO):
     data = node.get_vss_data().as_dict()
-    cdll.createBinaryCnode(
-        out_file.encode(),
-        node.name.encode(),
-        data.get("type", "").encode(),
-        uuid.encode(),
-        data.get("description", "").encode(),
-        data.get("datatype", "").encode(),
-        str(data.get("min", "")).encode(),
-        str(data.get("max", "")).encode(),
-        data.get("unit", "").encode(),
-        b"" if data.get("allowed") is None else allowedString(data.get("allowed", "")).encode(),
-        str(data.get("default", "")).encode(),
-        str(data.get("validate", "")).encode(),
-        len(node.children),
-    )
+
+    f.write(create_l8v_string(node.name))
+    f.write(create_l8v_string(data.get("type", "")))
+    if node.uuid is None:
+        log.debug(("No UUID for node %s", node.name))
+        f.write(struct.pack("B", 0))
+    else:
+        f.write(create_l8v_string(node.uuid))
+
+    f.write(create_l16v_string(data.get("description", "")))
+    f.write(create_l8v_string(data.get("datatype", "")))
+    f.write(create_l8v_string(str(data.get("min", ""))))
+    f.write(create_l8v_string(str(data.get("max", ""))))
+    f.write(create_l8v_string(data.get("unit", "")))
+
+    if data.get("allowed") is None:
+        f.write(struct.pack("H", 0))
+    else:
+        f.write(create_l16v_string(allowedString(data.get("allowed", ""))))
+
+    f.write(create_l8v_string(str(data.get("default", ""))))
+    f.write(create_l8v_string(str(data.get("validate", ""))))
+
+    f.write(struct.pack("B", len(node.children)))
+
     for child in node.children:
-        export_node(cdll, child, generate_uuid, out_file)
+        export_node(child, generate_uuid, f)
 
 
 @click.command()
 @clo.vspec_opt
 @clo.output_required_opt
-@click.option(
-    "--bintool-dll",
-    "-b",
-    required=True,
-    type=click.Path(exists=True, dir_okay=False, readable=True),
-)
 @clo.include_dirs_opt
 @clo.extended_attributes_opt
 @clo.strict_opt
@@ -89,27 +126,11 @@ def cli(
     overlays: tuple[Path],
     quantities: tuple[Path],
     units: tuple[Path],
-    bintool_dll: Path,
 ):
     """
     Export to Binary.
     """
-    cdll = ctypes.CDLL(str(bintool_dll))
-    cdll.createBinaryCnode.argtypes = (
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_char_p,
-        ctypes.c_int,
-    )
+
     tree, _ = get_trees(
         vspec=vspec,
         include_dirs=include_dirs,
@@ -122,5 +143,6 @@ def cli(
         overlays=overlays,
     )
     log.info("Generating binary output...")
-    export_node(cdll, tree, uuid, str(output))
-    log.info(f"Binary output generated in {output}")
+    with open(str(output), "wb") as f:
+        export_node(tree, uuid, f)
+    log.info("Binary output generated in %s", output)

tests/binary/test_binary.py

@@ -32,10 +32,7 @@ def test_binary(tmp_path):
     test_binary = tmp_path / "test.binary"
     ctestparser = tmp_path / "ctestparser"
     gotestparser = tmp_path / "gotestparser"
-    bintool_lib = tmp_path / "binarytool.so"
-    cmd = f"gcc -shared -o {bintool_lib} -fPIC {BIN_DIR / 'binarytool.c'}"
-    subprocess.run(cmd.split(), check=True)
-    cmd = f"vspec export binary -b {bintool_lib} -u {TEST_UNITS}"
+    cmd = f"vspec export binary  -u {TEST_UNITS}"
     cmd += f" -q {TEST_QUANT} -s {HERE / 'test.vspec'} -o {test_binary}"
     subprocess.run(cmd.split(), check=True)
     cmd = f"cc {BIN_DIR / 'c_parser/testparser.c'} {BIN_DIR / 'c_parser/cparserlib.c'} -o {ctestparser}"

tests/vspec/test_node_removal/test_node_removal.py

@@ -63,8 +63,6 @@ def test_deleted_node(exporter: str, out_file: str, overlay: Optional[str], tmp_
     if overlay:
         ov = str(HERE / overlay)
     cmd = exporter.split() + get_cla(spec, str(output), ov).split()
-    if "binary" in exporter:
-        cmd.extend(["-b", HERE / "../../../binary/binarytool.so"])
     process = subprocess.run(cmd, capture_output=True, text=True)
     assert process.returncode == 0
     result = output.read_text()
@@ -140,8 +138,6 @@ def test_deleted_branch(exporter: str, out_file: str, overlay: Optional[str], tm
     if overlay:
         ov = str(HERE / overlay)
     cmd = exporter.split() + get_cla(spec, str(output), ov).split()
-    if "binary" in exporter:
-        cmd.extend(["-b", HERE / "../../../binary/binarytool.so"])
     process = subprocess.run(cmd, capture_output=True, text=True)
     assert process.returncode == 0
     result_file = output.read_text()
@@ -212,8 +208,6 @@ def test_deleted_instance(
 
     ov = HERE / overlay
     cmd = exporter.split() + get_cla(spec, str(output), str(ov)).split()
-    if "binary" in exporter:
-        cmd.extend(["-b", HERE / "../../../binary/binarytool.so"])
 
     process = subprocess.run(cmd, capture_output=True, text=True)
     print(process.stdout)