From 479d4ca775d5194b109356b08935755a802ea08f Mon Sep 17 00:00:00 2001 From: Sebastian Schildt Date: Wed, 11 Sep 2024 12:45:44 +0200 Subject: [PATCH] Removed C dependency from binary exporter (#395) * Removed C dependency from binary exporter Signed-off-by: Sebastian Schildt --- .github/workflows/buildcheck.yml | 4 - binary/Makefile | 13 --- binary/README.md | 36 ++---- binary/binarytool.c | 85 -------------- .../vspec/vssexporters/vss2binary.py | 110 +++++++++++------- tests/binary/test_binary.py | 5 +- .../test_node_removal/test_node_removal.py | 6 - 7 files changed, 78 insertions(+), 181 deletions(-) delete mode 100644 binary/Makefile delete mode 100644 binary/binarytool.c diff --git a/.github/workflows/buildcheck.yml b/.github/workflows/buildcheck.yml index eb480684..c83e9c88 100644 --- a/.github/workflows/buildcheck.yml +++ b/.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 diff --git a/binary/Makefile b/binary/Makefile deleted file mode 100644 index 0cf4c084..00000000 --- a/binary/Makefile +++ /dev/null @@ -1,13 +0,0 @@ -# -# Makefile to generate binary library -# - -.PHONY: clean all binary - -all: clean binary - -binary: - gcc -shared -o binarytool.so -fPIC binarytool.c - -clean: - rm -f binarytool.so diff --git a/binary/README.md b/binary/README.md index 924aa3b9..8e0cbfb1 100644 --- a/binary/README.md +++ b/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.
## 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 -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. diff --git a/binary/binarytool.c b/binary/binarytool.c deleted file mode 100644 index 7ae0e9ac..00000000 --- a/binary/binarytool.c +++ /dev/null @@ -1,85 +0,0 @@ -/** -* (C) 2020 Geotab Inc -* (C) 2018 Volvo Cars -* -* All files and artifacts in this repository are licensed under the -* provisions of the license provided by the LICENSE file in this repository. -* -* -* Write VSS tree node in binary format to file. -**/ - -#include -#include -#include -#include -#include -#include -#include - -FILE* treeFp; - -void writeNodeData(char* name, char* type, char* uuid, char* descr, char* datatype, char* min, char* max, char* unit, char* allowed, char* defaultAllowed, char* validate, int children) { -//printf("Name=%s, Type=%s, uuid=%s, validate=%s, children=%d, Descr=%s, datatype=%s, min=%s, max=%s Unit=%s, Allowed=%s\n", name, type, uuid, validate, children, descr, datatype, min, max, unit, allowed); - uint8_t nameLen = (uint8_t)strlen(name); - uint8_t typeLen = (uint8_t)strlen(type); - uint8_t uuidLen = (uint8_t)strlen(uuid); - uint16_t descrLen = (uint16_t)strlen(descr); - uint8_t datatypeLen = (uint8_t)strlen(datatype); - uint8_t minLen = (uint8_t)strlen(min); - uint8_t maxLen = (uint8_t)strlen(max); - uint8_t unitLen = (uint8_t)strlen(unit); - uint16_t allowedLen = (uint16_t)strlen(allowed); - uint8_t defaultAllowedLen = (uint8_t)strlen(defaultAllowed); - uint8_t validateLen = (uint8_t)strlen(validate); - - fwrite(&nameLen, sizeof(uint8_t), 1, treeFp); - fwrite(name, sizeof(char)*nameLen, 1, treeFp); - fwrite(&typeLen, sizeof(uint8_t), 1, treeFp); - fwrite(type, sizeof(char)*typeLen, 1, treeFp); - fwrite(&uuidLen, sizeof(uint8_t), 1, treeFp); - fwrite(uuid, sizeof(char)*uuidLen, 1, treeFp); - fwrite(&descrLen, sizeof(uint16_t), 1, treeFp); - fwrite(descr, sizeof(char)*descrLen, 1, treeFp); - fwrite(&datatypeLen, sizeof(uint8_t), 1, treeFp); - if (datatypeLen > 0) { - fwrite(datatype, sizeof(char)*datatypeLen, 1, treeFp); - } - fwrite(&minLen, sizeof(uint8_t), 1, treeFp); - if (minLen > 0) { - fwrite(min, sizeof(char)*minLen, 1, treeFp); - } - fwrite(&maxLen, sizeof(uint8_t), 1, treeFp); - if (maxLen > 0) { - fwrite(max, sizeof(char)*maxLen, 1, treeFp); - } - fwrite(&unitLen, sizeof(uint8_t), 1, treeFp); - if (unitLen > 0) { - fwrite(unit, sizeof(char)*unitLen, 1, treeFp); - } - fwrite(&allowedLen, sizeof(uint16_t), 1, treeFp); - if (allowedLen > 0) { - fwrite(allowed, sizeof(char)*allowedLen, 1, treeFp); - } - fwrite(&defaultAllowedLen, sizeof(uint8_t), 1, treeFp); - if (defaultAllowedLen > 0) { - fwrite(defaultAllowed, sizeof(char)*defaultAllowedLen, 1, treeFp); - } - fwrite(&validateLen, sizeof(uint8_t), 1, treeFp); - if (validateLen > 0) { - fwrite(validate, sizeof(char)*validateLen, 1, treeFp); - } - fwrite(&children, sizeof(uint8_t), 1, treeFp); -} - -void createBinaryCnode(char*fname, char* name, char* type, char* uuid, char* descr, char* datatype, char* min, char* max, char* unit, char* allowed, char* defaultAllowed, char* validate, int children) { - treeFp = fopen(fname, "a"); - if (treeFp == NULL) { - printf("Could not open file=%s for writing of tree.\n", fname); - return; - } - writeNodeData(name, type, uuid, descr, datatype, min, max, unit, allowed, defaultAllowed, validate, children); - fclose(treeFp); -} - - diff --git a/src/vss_tools/vspec/vssexporters/vss2binary.py b/src/vss_tools/vspec/vssexporters/vss2binary.py index 7d502d57..9b727975 100644 --- a/src/vss_tools/vspec/vssexporters/vss2binary.py +++ b/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) diff --git a/tests/binary/test_binary.py b/tests/binary/test_binary.py index 362284c3..4513c3eb 100644 --- a/tests/binary/test_binary.py +++ b/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}" diff --git a/tests/vspec/test_node_removal/test_node_removal.py b/tests/vspec/test_node_removal/test_node_removal.py index 265e44d0..4c32e919 100644 --- a/tests/vspec/test_node_removal/test_node_removal.py +++ b/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)