Merge pull request #104 from boschglobal/fix-93

Add dataType property to VssSignal
eclipse-kuksa · Mar 19, 2024 · ee3e6c2 · ee3e6c2
2 parents fbc54b7 + 767a785
commit ee3e6c2
Show file tree

Hide file tree

Showing 17 changed files with 432 additions and 198 deletions.
diff --git a/app/src/main/java/org/eclipse/kuksa/testapp/databroker/JavaDataBrokerEngine.java b/app/src/main/java/org/eclipse/kuksa/testapp/databroker/JavaDataBrokerEngine.java
@@ -26,8 +26,8 @@
 import org.eclipse.kuksa.connectivity.databroker.DataBrokerConnection;
 import org.eclipse.kuksa.connectivity.databroker.DataBrokerConnector;
 import org.eclipse.kuksa.connectivity.databroker.listener.DisconnectListener;
-import org.eclipse.kuksa.connectivity.databroker.listener.VssPathListener;
 import org.eclipse.kuksa.connectivity.databroker.listener.VssNodeListener;
+import org.eclipse.kuksa.connectivity.databroker.listener.VssPathListener;
 import org.eclipse.kuksa.connectivity.databroker.request.FetchRequest;
 import org.eclipse.kuksa.connectivity.databroker.request.SubscribeRequest;
 import org.eclipse.kuksa.connectivity.databroker.request.UpdateRequest;

diff --git a/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/AnyCopyExtension.kt b/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/AnyCopyExtension.kt
@@ -19,9 +19,9 @@
 
 package org.eclipse.kuksa.extension
 
-import kotlin.reflect.KParameter
 import kotlin.reflect.full.instanceParameter
 import kotlin.reflect.full.memberFunctions
+import kotlin.reflect.full.valueParameters
 
 /**
  * Uses reflection to create a copy with any constructor parameter which matches the given [paramToValue] map.
@@ -37,16 +37,20 @@ internal fun <T : Any> T.copy(paramToValue: Map<String, Any?> = emptyMap()): T {
     val copyFunction = instanceClass::memberFunctions.get().first { it.name == "copy" }
     val instanceParameter = copyFunction.instanceParameter ?: return this
 
-    val valueArgs = copyFunction.parameters
-        .filter { parameter ->
-            parameter.kind == KParameter.Kind.VALUE
-        }.mapNotNull { parameter ->
+    val valueArgs = copyFunction.valueParameters
+        .mapNotNull { parameter ->
             paramToValue[parameter.name]?.let { value -> parameter to value }
         }
 
     val parameterToInstance = mapOf(instanceParameter to this)
     val parameterToValue = parameterToInstance + valueArgs
-    val copy = copyFunction.callBy(parameterToValue) ?: this
+
+    val copy: Any
+    try {
+        copy = copyFunction.callBy(parameterToValue) ?: this
+    } catch (e: IllegalArgumentException) {
+        throw IllegalArgumentException("${this::class.simpleName} copy parameters do not match: $paramToValue", e)
+    }
 
     return copy as T
 }
diff --git a/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/DataPointExtension.kt b/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/DataPointExtension.kt
@@ -35,28 +35,38 @@ val Types.Metadata.valueType: ValueCase
     get() = dataType.dataPointValueCase
 
 /**
- * Converts the [VssSignal.value] into a [Datapoint] object.
+ * Converts the [VssSignal.value] into a [Datapoint] object. The [VssSignal.dataType] is used to derive the correct
+ * [ValueCase].
  *
  * @throws IllegalArgumentException if the [VssSignal] could not be converted to a [Datapoint].
  */
+@OptIn(ExperimentalUnsignedTypes::class)
 val <T : Any> VssSignal<T>.datapoint: Datapoint
     get() {
-        val stringValue = value.toString()
-        return when (value::class) {
-            String::class -> ValueCase.STRING.createDatapoint(stringValue)
-            Boolean::class -> ValueCase.BOOL.createDatapoint(stringValue)
-            Float::class -> ValueCase.FLOAT.createDatapoint(stringValue)
-            Double::class -> ValueCase.DOUBLE.createDatapoint(stringValue)
-            Int::class -> ValueCase.INT32.createDatapoint(stringValue)
-            Long::class -> ValueCase.INT64.createDatapoint(stringValue)
-            UInt::class -> ValueCase.UINT32.createDatapoint(stringValue)
-            Array<String>::class -> ValueCase.DOUBLE.createDatapoint(stringValue)
-            IntArray::class -> ValueCase.INT32_ARRAY.createDatapoint(stringValue)
-            BooleanArray::class -> ValueCase.BOOL_ARRAY.createDatapoint(stringValue)
-            LongArray::class -> ValueCase.INT64_ARRAY.createDatapoint(stringValue)
-
-            else -> throw IllegalArgumentException("Could not create datapoint for the value class: ${value::class}!")
+        val valueCase = when (dataType) {
+            String::class -> ValueCase.STRING
+            Boolean::class -> ValueCase.BOOL
+            Int::class -> ValueCase.INT32
+            Float::class -> ValueCase.FLOAT
+            Double::class -> ValueCase.DOUBLE
+            Long::class -> ValueCase.INT64
+            UInt::class -> ValueCase.UINT32
+            ULong::class -> ValueCase.UINT64
+            Array<String>::class -> ValueCase.DOUBLE
+            BooleanArray::class -> ValueCase.BOOL_ARRAY
+            IntArray::class -> ValueCase.INT32_ARRAY
+            FloatArray::class -> ValueCase.FLOAT_ARRAY
+            DoubleArray::class -> ValueCase.DOUBLE_ARRAY
+            LongArray::class -> ValueCase.INT64_ARRAY
+            UIntArray::class -> ValueCase.UINT32_ARRAY
+            ULongArray::class -> ValueCase.UINT64_ARRAY
+
+            else -> throw IllegalArgumentException("Could not create datapoint for value class: ${dataType::class}!")
         }
+
+        val stringValue = value.toString()
+
+        return valueCase.createDatapoint(stringValue)
     }
 
 /**

diff --git a/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/DataTypeConversionExtension.kt b/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/DataTypeConversionExtension.kt
@@ -41,30 +41,35 @@ val Types.DataType.dataPointValueCase: ValueCase
             -> ValueCase.INT32
 
             Types.DataType.DATA_TYPE_INT64 -> ValueCase.INT64
+
             Types.DataType.DATA_TYPE_UINT8,
             Types.DataType.DATA_TYPE_UINT16,
             Types.DataType.DATA_TYPE_UINT32,
-            Types.DataType.DATA_TYPE_UINT64,
-            -> ValueCase.UINT64
+            -> ValueCase.UINT32
+
+            Types.DataType.DATA_TYPE_UINT64 -> ValueCase.UINT64
 
-            Types.DataType.DATA_TYPE_BOOLEAN_ARRAY -> ValueCase.BOOL_ARRAY
             Types.DataType.DATA_TYPE_INT8_ARRAY,
             Types.DataType.DATA_TYPE_INT16_ARRAY,
             Types.DataType.DATA_TYPE_INT32_ARRAY,
             -> ValueCase.INT32_ARRAY
 
             Types.DataType.DATA_TYPE_INT64_ARRAY -> ValueCase.INT64_ARRAY
+
             Types.DataType.DATA_TYPE_UINT8_ARRAY,
             Types.DataType.DATA_TYPE_UINT16_ARRAY,
             Types.DataType.DATA_TYPE_UINT32_ARRAY,
             -> ValueCase.UINT32_ARRAY
 
             Types.DataType.DATA_TYPE_UINT64_ARRAY -> ValueCase.UINT64_ARRAY
-            Types.DataType.DATA_TYPE_FLOAT_ARRAY -> ValueCase.FLOAT_ARRAY
-            Types.DataType.DATA_TYPE_DOUBLE_ARRAY -> ValueCase.DOUBLE_ARRAY
-            Types.DataType.DATA_TYPE_STRING_ARRAY -> ValueCase.STRING_ARRAY
+
             Types.DataType.DATA_TYPE_STRING -> ValueCase.STRING
             Types.DataType.DATA_TYPE_FLOAT -> ValueCase.FLOAT
             Types.DataType.DATA_TYPE_DOUBLE -> ValueCase.DOUBLE
+
+            Types.DataType.DATA_TYPE_BOOLEAN_ARRAY -> ValueCase.BOOL_ARRAY
+            Types.DataType.DATA_TYPE_FLOAT_ARRAY -> ValueCase.FLOAT_ARRAY
+            Types.DataType.DATA_TYPE_DOUBLE_ARRAY -> ValueCase.DOUBLE_ARRAY
+            Types.DataType.DATA_TYPE_STRING_ARRAY -> ValueCase.STRING_ARRAY
         }
     }
diff --git a/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/vss/VssNodeCopyExtension.kt b/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/vss/VssNodeCopyExtension.kt
@@ -27,7 +27,7 @@ import org.eclipse.kuksa.vsscore.model.VssSignal
 import org.eclipse.kuksa.vsscore.model.findHeritageLine
 import org.eclipse.kuksa.vsscore.model.heritage
 import org.eclipse.kuksa.vsscore.model.variableName
-import kotlin.reflect.full.memberProperties
+import kotlin.reflect.full.declaredMemberProperties
 
 /**
  * Creates a copy of the [VssNode] where the whole [VssNode.findHeritageLine] is replaced
@@ -136,11 +136,12 @@ fun <T : Any> VssSignal<T>.copy(datapoint: Datapoint): VssSignal<T> {
  * Calls the generated copy method of the data class for the [VssSignal] and returns a new copy with the new [value].
  *
  * @throws [IllegalArgumentException] if the copied types do not match.
- * @throws [NoSuchElementException] if no copy method was found for the class.
+ * @throws [NoSuchElementException] if no copy method nor [valuePropertyName] was found for the class.
  */
-fun <T : Any> VssSignal<T>.copy(value: T): VssSignal<T> {
-    val memberProperties = VssSignal::class.memberProperties
-    val firstPropertyName = memberProperties.first().name
+@JvmOverloads
+fun <T : Any> VssSignal<T>.copy(value: T, valuePropertyName: String = "value"): VssSignal<T> {
+    val memberProperties = VssSignal::class.declaredMemberProperties
+    val firstPropertyName = memberProperties.first { it.name == valuePropertyName }.name
     val valueMap = mapOf(firstPropertyName to value)
 
     return this@copy.copy(valueMap)
@@ -179,7 +180,7 @@ fun <T : VssNode> T.copy(
 // region Operators
 
 /**
- * Convenience operator for [deepCopy] with a [VssNode]. It will return the [VssNode] with the updated
+ * Convenience operator for [deepCopy] with a [VssNode]. It will return the parent [VssNode] with the updated child
  * [VssNode].
  *
  * @throws [IllegalArgumentException] if the copied types do not match.

diff --git a/...sdk/src/test/kotlin/org/eclipse/kuksa/connectivity/databroker/DataBrokerConnectionTest.kt b/...sdk/src/test/kotlin/org/eclipse/kuksa/connectivity/databroker/DataBrokerConnectionTest.kt
@@ -23,6 +23,7 @@ import io.grpc.ManagedChannel
 import io.kotest.assertions.nondeterministic.eventually
 import io.kotest.core.spec.style.BehaviorSpec
 import io.kotest.matchers.shouldBe
+import io.kotest.matchers.shouldNotBe
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.verify
@@ -34,6 +35,7 @@ import org.eclipse.kuksa.connectivity.databroker.request.SubscribeRequest
 import org.eclipse.kuksa.connectivity.databroker.request.UpdateRequest
 import org.eclipse.kuksa.connectivity.databroker.request.VssNodeFetchRequest
 import org.eclipse.kuksa.connectivity.databroker.request.VssNodeSubscribeRequest
+import org.eclipse.kuksa.connectivity.databroker.request.VssNodeUpdateRequest
 import org.eclipse.kuksa.extensions.updateRandomFloatValue
 import org.eclipse.kuksa.mocking.FriendlyVssNodeListener
 import org.eclipse.kuksa.mocking.FriendlyVssPathListener
@@ -118,7 +120,7 @@ class DataBrokerConnectionTest : BehaviorSpec({
                 val response = dataBrokerConnection.update(updateRequest)
 
                 then("No error should appear") {
-                    Assertions.assertFalse(response.hasError())
+                    response.hasError() shouldBe false
                 }
 
                 and("When fetching it afterwards") {
@@ -165,22 +167,35 @@ class DataBrokerConnectionTest : BehaviorSpec({
         and("A VssNode") {
             val vssDriver = VssDriver()
 
-            `when`("Fetching the node") {
+            and("A default HeartRate") {
+                val newHeartRateValue = 60
+                val datapoint = Types.Datapoint.newBuilder().setUint32(newHeartRateValue).build()
+                val defaultUpdateRequest = UpdateRequest(vssDriver.heartRate.vssPath, datapoint)
 
-                and("The initial value is different from the default for a child") {
-                    val newHeartRateValue = 60
-                    val datapoint = Types.Datapoint.newBuilder().setUint32(newHeartRateValue).build()
-                    val updateRequest = UpdateRequest(vssDriver.heartRate.vssPath, datapoint)
+                dataBrokerConnection.update(defaultUpdateRequest)
 
-                    dataBrokerConnection.update(updateRequest)
+                `when`("Fetching the node") {
 
-                    val fetchRequest = VssNodeFetchRequest(vssDriver)
-                    val updatedDriver = dataBrokerConnection.fetch(fetchRequest)
+                    and("The initial value is different from the default for a child") {
+                        val fetchRequest = VssNodeFetchRequest(vssDriver)
+                        val updatedDriver = dataBrokerConnection.fetch(fetchRequest)
 
-                    then("Every child node has been updated with the correct value") {
-                        val heartRate = updatedDriver.heartRate
+                        then("Every child node has been updated with the correct value") {
+                            val heartRate = updatedDriver.heartRate
+
+                            heartRate.value shouldBe newHeartRateValue
+                        }
+                    }
+                }
 
-                        heartRate.value shouldBe newHeartRateValue
+                `when`("Updating the node with an invalid value") {
+                    val invalidHeartRate = VssDriver.VssHeartRate(-5) // UInt on DataBroker side
+                    val vssNodeUpdateRequest = VssNodeUpdateRequest(invalidHeartRate)
+                    val response = dataBrokerConnection.update(vssNodeUpdateRequest)
+
+                    then("the update response should contain an error") {
+                        val errorResponse = response.firstOrNull { it.errorsCount >= 1 }
+                        errorResponse shouldNotBe null
                     }
                 }
             }
@@ -213,10 +228,10 @@ class DataBrokerConnectionTest : BehaviorSpec({
                     }
                 }
 
-                and("Any subscribed node was changed") {
+                and("Any subscribed uInt node was changed") {
                     val newHeartRateValue = 50
-                    val datapoint = Types.Datapoint.newBuilder().setUint32(newHeartRateValue).build()
-                    val updateRequest = UpdateRequest(vssDriver.heartRate.vssPath, datapoint)
+                    val newVssHeartRate = VssDriver.VssHeartRate(newHeartRateValue)
+                    val updateRequest = VssNodeUpdateRequest(newVssHeartRate)
 
                     dataBrokerConnection.update(updateRequest)
 

diff --git a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssNode/VssDriver.kt b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssNode/VssDriver.kt
@@ -64,6 +64,9 @@ data class VssDriver @JvmOverloads constructor(
     data class VssHeartRate @JvmOverloads constructor(
         override val `value`: Int = 0,
     ) : VssSignal<Int> {
+        override val dataType: KClass<*>
+            get() = UInt::class
+
         override val comment: String
             get() = ""
 

diff --git a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssNode/VssNodeCopyTest.kt b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssNode/VssNodeCopyTest.kt
@@ -77,6 +77,21 @@ class VssNodeCopyTest : BehaviorSpec({
             }
         }
 
+        and("a changed invalid DataPoint") {
+            val datapoint = Types.Datapoint.newBuilder().setBool(false).build()
+
+            `when`("a copy is done") {
+                val exception = shouldThrow<IllegalArgumentException> {
+                    driverHeartRate.copy(datapoint)
+                }
+
+                then("it should throw an IllegalArgumentException") {
+                    val signalName = driverHeartRate::class.simpleName
+                    exception.message shouldStartWith "$signalName copy parameters do not match"
+                }
+            }
+        }
+
         and("a changed DataPoint") {
             val newValue = 50
             val datapoint = Types.Datapoint.newBuilder().setInt32(newValue).build()

diff --git a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssNode/VssVehicle.kt b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/vssNode/VssVehicle.kt
@@ -69,6 +69,9 @@ data class VssPassenger(
         override val comment: String = "",
         override val value: Int = 80,
     ) : VssSignal<Int> {
+        override val dataType: KClass<*>
+            get() = UInt::class
+
         override val parentClass: KClass<*>
             get() = VssPassenger::class
     }

diff --git a/samples/src/main/java/com/example/sample/JavaActivity.java b/samples/src/main/java/com/example/sample/JavaActivity.java
@@ -26,8 +26,8 @@
 import org.eclipse.kuksa.connectivity.databroker.DataBrokerConnection;
 import org.eclipse.kuksa.connectivity.databroker.DataBrokerConnector;
 import org.eclipse.kuksa.connectivity.databroker.listener.DisconnectListener;
-import org.eclipse.kuksa.connectivity.databroker.listener.VssPathListener;
 import org.eclipse.kuksa.connectivity.databroker.listener.VssNodeListener;
+import org.eclipse.kuksa.connectivity.databroker.listener.VssPathListener;
 import org.eclipse.kuksa.connectivity.databroker.request.FetchRequest;
 import org.eclipse.kuksa.connectivity.databroker.request.SubscribeRequest;
 import org.eclipse.kuksa.connectivity.databroker.request.UpdateRequest;

diff --git a/vss-core/src/main/kotlin/org/eclipse/kuksa/vsscore/model/VssBranch.kt b/vss-core/src/main/kotlin/org/eclipse/kuksa/vsscore/model/VssBranch.kt
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2023 Contributors to the Eclipse Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.eclipse.kuksa.vsscore.model
+
+/**
+ * Defines a [VssNode] which is not a [VssSignal] and only acts as a branch with one or more children. The [type] is
+ * always "branch".
+ */
+interface VssBranch : VssNode {
+    override val type: String
+        get() = "branch"
+}
diff --git a/vss-core/src/main/kotlin/org/eclipse/kuksa/vsscore/model/VssNode.kt b/vss-core/src/main/kotlin/org/eclipse/kuksa/vsscore/model/VssNode.kt
@@ -69,26 +69,6 @@ interface VssNode {
         get() = null
 }
 
-/**
- * Defines a [VssNode] which is not a [VssSignal] and only acts as a branch with one or more children. The [type] is
- * always "branch".
- */
-interface VssBranch : VssNode {
-    override val type: String
-        get() = "branch"
-}
-
-/**
- * Some [VssNode] may have an additional [value] property. These are children [VssSignal] which do not have other
- * children.
- */
-interface VssSignal<T : Any> : VssNode {
-    /**
-     * A primitive type value.
-     */
-    val value: T
-}
-
 /**
  * Splits the [VssNode.vssPath] into its parts.
  */
@@ -219,21 +199,3 @@ fun VssNode.findHeritageLine(
 
     return heritageLine
 }
-
-/**
- * Finds the given [signal] inside the current [VssNode].
- */
-inline fun <reified T : VssSignal<V>, V : Any> VssNode.findSignal(signal: T): VssNode {
-    return heritage
-        .first { it.uuid == signal.uuid }
-}
-
-/**
- * Finds all [VssSignal] which matches the given [KClass.simpleName]. This is useful when multiple nested objects
- * with the same Name exists but are pretty much the same besides the [VssNode.vssPath] etc.
- */
-inline fun <reified T : VssSignal<V>, V : Any> VssNode.findSignal(type: KClass<T>): Map<String, VssNode> {
-    return heritage
-        .filter { it::class.simpleName == type.simpleName }
-        .associateBy { it.vssPath }
-}