Refactor array handling and add tests

eclipse · Jul 12, 2023 · 42118a7 · 42118a7
1 parent 9f3ddb9
commit 42118a7
Show file tree

Hide file tree

Showing 8 changed files with 563 additions and 70 deletions.
diff --git a/.github/workflows/kuksa-go.yml b/.github/workflows/kuksa-go.yml
@@ -0,0 +1,46 @@
+# /********************************************************************************
+# * Copyright (c) 2022 Contributors to the Eclipse Foundation
+# *
+# * See the NOTICE file(s) distributed with this work for additional
+# * information regarding copyright ownership.
+# *
+# * This program and the accompanying materials are made available under the
+# * terms of the Apache License 2.0 which is available at
+# * http://www.apache.org/licenses/LICENSE-2.0
+# *
+# * SPDX-License-Identifier: Apache-2.0
+# ********************************************************************************/
+
+name: kuksa_go_client
+
+on:
+  push:
+    branches: [ master ]
+  pull_request:
+    branches: [ master ]
+    paths:
+    - ".github/workflows/kuksa-go.yml"
+    - "kuksa_go_client/**"
+  workflow_dispatch:
+
+jobs:
+
+  kuksa-go-client-test:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout kuksa.val
+        uses: actions/checkout@v3
+      - name: Run go tests
+        run: |
+          cd kuksa_go_client
+          # We cannot use sudo apt install protobuf-compiler
+          # as default in Ubuntu 22.04 (3.12) consider optional as experimental feature
+          go run protocInstall/protocInstall.go
+          export PATH=$PATH:$HOME/protoc/bin
+          sudo chmod +x $HOME/protoc/bin/protoc
+          go install google.golang.org/protobuf/cmd/protoc-gen-go@latest
+          go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
+          export PATH=$PATH:$HOME/go/bin
+          go generate .
+          go test .
+
diff --git a/kuksa-client/README.md b/kuksa-client/README.md
@@ -182,6 +182,79 @@ This is an example showing how some of the commands can be used:
 
 ![try kuksa-client out](https://raw.githubusercontent.com/eclipse/kuksa.val/master/doc/pictures/testclient_basic.gif "test client usage")
 
+### Syntax for specifying data in the command line interface
+
+Values used as argument to for example `setValue` shall match the type given. Quotes (single and double) are 
+generally not needed, except in a few special cases. A few valid examples on setting float is shown below:
+
+```
+setValue Vehicle.Speed 43
+setValue Vehicle.Speed "45"
+setValue Vehicle.Speed '45.2'
+```
+
+For strings escaped quotes are needed if you want quotes to be sent to Server/Databroker, like if you want to store
+`Almost "red"` as value. Alternatively you can use outer single quotes and inner double quotes.
+
+The two examples below are equal:
+
+```
+setValue Vehicle.Cabin.Light.InteractiveLightBar.Effect 'Almost \"red\"'
+setValue Vehicle.Cabin.Light.InteractiveLightBar.Effect 'Almost "red"'
+```
+
+Alternatively you can use inner single quotes, but then the value will be represented by double quotes (`Almost "blue"`)
+when stored anyhow.
+
+```
+setValue Vehicle.Cabin.Light.InteractiveLightBar.Effect "Almost 'blue'"
+setValue Vehicle.Cabin.Light.InteractiveLightBar.Effect "Almost \'blue\'"
+```
+
+If not using outer quotes the inner quotes will be lost, the examples below are equal.
+Leading/trialing spaces are ignored.
+
+```
+setValue Vehicle.Cabin.Light.InteractiveLightBar.Effect Almost 'green'
+setValue Vehicle.Cabin.Light.InteractiveLightBar.Effect Almost green
+setValue Vehicle.Cabin.Light.InteractiveLightBar.Effect 'Almost green'
+setValue Vehicle.Cabin.Light.InteractiveLightBar.Effect "Almost green"
+setValue Vehicle.Cabin.Light.InteractiveLightBar.Effect 'Almost green         '
+```
+
+
+It is possible to set array values. Setting a string array with simple identifiers is not a problem. Also not if they
+contain blanks
+
+```
+// Array with two elements
+setValue Vehicle.OBD.DTCList [abc,def]
+// Array with two elements, "hello there" and "def"
+setValue Vehicle.OBD.DTCList [hello there,def]
+```
+
+Setting values that includes comma or quotation marks is more tricky, as the shell argument parser handling affects
+how they are interpreted. The recommended approach is to have outer quotes of one type and user inner quotes of the
+other type.
+
+Example 1: First item should be `hello, there`
+
+```
+setValue Vehicle.OBD.DTCList '[ "hello, there",def]'
+```
+
+Example 2: First item should be `hello, "there"`
+
+```
+setValue Vehicle.OBD.DTCList '["hello, \"there\"",def]'
+```
+
+Example 3: First item should be `hello, 'there'`
+
+```
+setValue Vehicle.OBD.DTCList "['hello, \'there\'',def]"
+```
+
 ### Updating VSS Structure
 
 Using the test client, it is also possible to update and extend the VSS data structure.

diff --git a/kuksa-client/kuksa_client/__main__.py b/kuksa-client/kuksa_client/__main__.py
@@ -301,6 +301,20 @@ def do_authorize(self, args):
     def do_setValue(self, args):
         """Set the value of a path"""
         if self.checkConnection():
+            # If there is a blank before a single/double quote on the kuksa-client cli then
+            # the argparser shell will remove it, there is nothing we can do to it
+            # This gives off behavior for examples like:
+            # setValue Vehicle.OBD.DTCList [ "dtc1, dtc2", ddd]
+            # which will be treated as input of 3 elements
+            # The recommended approach is to have quotes (of a different type) around the whole value
+            # if your strings includes quotes, commas or other items
+            # setValue Vehicle.OBD.DTCList '[ "dtc1, dtc2", ddd]'
+            # or
+            # setValue Vehicle.OBD.DTCList "[ 'dtc1, dtc2', ddd]"
+            # If you really need to include a quote in the values use backslash and use the quote type
+            # you want as inner value:
+            # setValue Vehicle.OBD.DTCList "[ 'dtc1, \'dtc2', ddd]"
+            # Will result in two elements in the array; "dtc1, 'dtc2" and "ddd"
             value = str(' '.join(args.Value))
             resp = self.commThread.setValue(
                 args.Path, value, args.attribute)

diff --git a/kuksa-client/kuksa_client/grpc/__init__.py b/kuksa-client/kuksa_client/grpc/__init__.py
@@ -315,6 +315,59 @@ def from_message(cls, message: types_pb2.Datapoint):
             ) if message.HasField('timestamp') else None,
         )
 
+
+    def cast_array_values(cast, array):
+        """
+        Parses array input and cast individual values to wanted type.
+        Note that input value to this function is not the same as given if you use kuksa-client command line
+        as parts (e.g. surrounding quotes) are removed by shell, and then do_setValue also do some magic.
+        """
+        array = array.strip('[]')
+
+        # Split the input string into separate values
+        # First alternative, not quotes including escaped single or double quote, ends at comma, whitespace or EOL
+        # Second group is double quoted string, may contain quoted strings and single quotes inside, ends at non-escaped
+        # double quote
+        # Third is similar but for single quote
+        # Using raw strings with surrounding single/double quotes to minimize need for escapes
+        pattern = r'(?:\\"|\\' + \
+                  r"'|[^'" + r'",])+|"(?:\\"|[^"])*"|' + \
+                  r"'(?:\\'|[^'])*'"
+        values = re.findall(pattern, array)
+        for item in values:
+            # We may in some cases match blanks, that is intended as we want to be able to write arguments like
+            # My Way
+            # ... without quotes
+            if item.strip() == '':
+                #skip
+                pass
+            else:
+                yield cast(item)
+
+    def cast_bool(value) -> bool:
+        if value in ('False', 'false', 'F', 'f'):
+            value = 0
+        return bool(value)
+
+    def cast_str(value) -> str:
+        """
+        Strip based on the following rules.
+        - Leading/Trailing blanks are removed
+        - If there are single or double quote at both start and end they are removed
+        - Finally any quote escapes are removed
+        """
+        logger.info("cast_str %s", value)
+        new_val = value.strip()
+        if new_val.startswith('"') and new_val.endswith('"'):
+            new_val = new_val[1:-1]
+        if new_val.startswith("'") and new_val.endswith("'"):
+            new_val = new_val[1:-1]
+        # Replace escaped quotes with normal quotes
+        new_val = new_val.replace('\\\"', '\"')
+        new_val = new_val.replace("\\\'", "\'")
+        logger.info("new_val %s", new_val)
+        return new_val
+
     def to_message(self, value_type: DataType) -> types_pb2.Datapoint:
         message = types_pb2.Datapoint()
 
@@ -323,27 +376,6 @@ def set_array_attr(obj, attr, values):
             array.Clear()
             array.values.extend(values)
 
-        def cast_array_values(cast, array):
-            array = array.strip('[]')
-            pattern = r'(?:\\.|[^",])*"(?:\\.|[^"])*"|[^",]+'
-            values = re.findall(pattern, array)
-            for item in values:
-                if item == '':
-                    #skip
-                    pass
-                else:
-                    if cast == str:
-                        item = item.replace('\"', '').replace('\\', '"').strip()
-                    yield cast(item)
-
-        def cast_bool(value):
-            if value in ('False', 'false', 'F', 'f'):
-                value = 0
-            return bool(value)
-
-        def cast_str(value):
-            return str(value).replace('\"', '').replace('\\', '"').strip()
-
 
         field, set_field, cast_field = {
             DataType.INT8: ('int32', setattr, int),
@@ -356,20 +388,32 @@ def cast_str(value):
             DataType.INT64: ('int64', setattr, int),
             DataType.FLOAT: ('float', setattr, float),
             DataType.DOUBLE: ('double', setattr, float),
-            DataType.BOOLEAN: ('bool', setattr, cast_bool),
-            DataType.STRING: ('string', setattr, cast_str),
-            DataType.INT8_ARRAY: ('int32_array', set_array_attr, lambda array: cast_array_values(int, array)),
-            DataType.INT16_ARRAY: ('int32_array', set_array_attr, lambda array: cast_array_values(int, array)),
-            DataType.INT32_ARRAY: ('int32_array', set_array_attr, lambda array: cast_array_values(int, array)),
-            DataType.UINT8_ARRAY: ('uint32_array', set_array_attr, lambda array: cast_array_values(int, array)),
-            DataType.UINT16_ARRAY: ('uint32_array', set_array_attr, lambda array: cast_array_values(int, array)),
-            DataType.UINT32_ARRAY: ('uint32_array', set_array_attr, lambda array: cast_array_values(int, array)),
-            DataType.UINT64_ARRAY: ('uint64_array', set_array_attr, lambda array: cast_array_values(int, array)),
-            DataType.INT64_ARRAY: ('int64_array', set_array_attr, lambda array: cast_array_values(int, array)),
-            DataType.FLOAT_ARRAY: ('float_array', set_array_attr, lambda array: cast_array_values(float, array)),
-            DataType.DOUBLE_ARRAY: ('double_array', set_array_attr, lambda array: cast_array_values(float, array)),
-            DataType.BOOLEAN_ARRAY: ('bool_array', set_array_attr, lambda array: cast_array_values(cast_bool, array)),
-            DataType.STRING_ARRAY: ('string_array', set_array_attr, lambda array: cast_array_values(str, array)),
+            DataType.BOOLEAN: ('bool', setattr, Datapoint.cast_bool),
+            DataType.STRING: ('string', setattr, Datapoint.cast_str),
+            DataType.INT8_ARRAY: ('int32_array', set_array_attr, 
+                                  lambda array: Datapoint.cast_array_values(int, array)),
+            DataType.INT16_ARRAY: ('int32_array', set_array_attr, 
+                                   lambda array: Datapoint.cast_array_values(int, array)),
+            DataType.INT32_ARRAY: ('int32_array', set_array_attr, 
+                                   lambda array: Datapoint.cast_array_values(int, array)),
+            DataType.UINT8_ARRAY: ('uint32_array', set_array_attr, 
+                                   lambda array: Datapoint.cast_array_values(int, array)),
+            DataType.UINT16_ARRAY: ('uint32_array', set_array_attr, 
+                                    lambda array: Datapoint.cast_array_values(int, array)),
+            DataType.UINT32_ARRAY: ('uint32_array', set_array_attr, 
+                                    lambda array: Datapoint.cast_array_values(int, array)),
+            DataType.UINT64_ARRAY: ('uint64_array', set_array_attr, 
+                                    lambda array: Datapoint.cast_array_values(int, array)),
+            DataType.INT64_ARRAY: ('int64_array', set_array_attr, 
+                                   lambda array: Datapoint.cast_array_values(int, array)),
+            DataType.FLOAT_ARRAY: ('float_array', set_array_attr, 
+                                   lambda array: Datapoint.cast_array_values(float, array)),
+            DataType.DOUBLE_ARRAY: ('double_array', set_array_attr, 
+                                    lambda array: Datapoint.cast_array_values(float, array)),
+            DataType.BOOLEAN_ARRAY: ('bool_array', set_array_attr, 
+                                     lambda array: Datapoint.cast_array_values(Datapoint.cast_bool, array)),
+            DataType.STRING_ARRAY: ('string_array', set_array_attr, 
+                                    lambda array: Datapoint.cast_array_values(Datapoint.cast_str, array)),
         }.get(value_type, (None, None, None))
         if self.value is not None:
             if all((field, set_field, cast_field)):