Align Terminology of VSS model classes / public API #94

Chrylo · 2024-03-06T15:22:26Z

close #70

Renaming:
VssDefinition (Annotation) -> VssModelGenerator
VssNode (Any node with vss childs / parents) -> VssNode
VssSpecification (Any node + vss information) -> Removed / merged with VssNode
VssProperty (Leaf) -> VssLeaf

VssSpecificationListener -> VssNodeListener
PropertyListener -> VssPathListener
SpecificationPropertyListener -> VssNodePathListener
Property -> Removed / Replaced with:

interface DataBrokerRequest {
    vssPath: String
    fields: Collection<Field>
}
FetchRequest : DatabrokerRequest
SubscribeRequest : DatabrokerRequest
UpdateRequest : DatabrokerRequest {
    dataPoint: DataPoint
}
VssNodeDataBrokerRequest : DataBrokerRequest<T : Node> {
    vssNode: T
}
VssNodeFetchRequest : VssNodeDataBrokerRequest
VssNodeUpdateRequest : VssNodeDataBrokerRequest 
VssNodeSubscribeRequest : VssNodeDataBrokerRequest

Checked:

Tests
Diagrams (puml)
Main code
Guides
Kdoc

Tested:

Get / Set / Subscribe via Vss paths + generated Models (Tested model Vehicle.Speed)

It is easy to miss naming like "property" etc. in the documentation / guides and so on. So please have a good look also.

BREAKING CHANGE: The API for fetch / subscribe / update did change.
The input for these methods are now wrapped via "DataBrokerRequest"
classes e.g. "FetchRequest", "UpdateRequest". For generated VSS models
the classes "VssNodeFetchRequest" etc. should be used.

The Property model was removed in favor of DataBrokerRequest

VssProperty -> VssNode close eclipse-kuksa#70

The VssNode interface is already a good name for the defined VssSpecification properties.

The VssSpecification (renamed to VssNode) now also has additional properties defined. The property type was not checked before when generating the vss node attributes like the UUID. This resulted into a duplicated generation of the "child" + "parent" properties.

The Definition wording is kinda redundant with what VSS means (Specification). So the annotation name was changed to something it is actually responsible for: Triggering the generation of Models.

# Conflicts: # vss-core/src/main/java/org/eclipse/kuksa/vsscore/annotation/VssDefinition.kt # vss-core/src/main/java/org/eclipse/kuksa/vsscore/model/VssSpecification.kt # vss-core/src/main/kotlin/org/eclipse/kuksa/vsscore/annotation/VssDefinition.kt # vss-core/src/main/kotlin/org/eclipse/kuksa/vsscore/annotation/VssModelGenerator.kt # vss-core/src/main/kotlin/org/eclipse/kuksa/vsscore/model/VssNode.kt # vss-core/src/main/kotlin/org/eclipse/kuksa/vsscore/model/VssSpecification.kt # vss-core/src/test/java/org/eclipse/kuksa/vsscore/model/VssSpecificationTest.kt # vss-core/src/test/kotlin/org/eclipse/kuksa/vsscore/model/VssNodeTest.kt # vss-core/src/test/kotlin/org/eclipse/kuksa/vsscore/model/VssSpecificationTest.kt

The "Property" name was not a good reflection of the combination of the variables vssPath + Field (grpc). This model is not replaced with explicit *Request classes. BREAKING CHANGE: The API for fetch / subscribe / update did change. The input for these methods are now wrapped via "DataBrokerRequest" classes e.g. "FetchRequest", "UpdateRequest". For generated VSS models the classes "VssNodeFetchRequest" etc. should be used. - The Property model was removed in favor of DataBrokerRequest

wba2hi

Short functional test was Ok

kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/connectivity/databroker/DataBrokerConnection.kt

...sdk/src/main/kotlin/org/eclipse/kuksa/connectivity/databroker/request/VssNodeFetchRequest.kt

...src/main/kotlin/org/eclipse/kuksa/connectivity/databroker/request/VssNodeSubscribeRequest.kt

...dk/src/main/kotlin/org/eclipse/kuksa/connectivity/databroker/request/VssNodeUpdateRequest.kt

README.md

kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/connectivity/databroker/listener/VssPathListener.kt

vss-processor-plugin/src/main/java/org/eclipse/kuksa/vssprocessor/plugin/VssProcessorPlugin.kt

wba2hi · 2024-03-07T12:31:56Z

@SebastianSchildt and @eriksven

Maybe you can also have a short look at this - not from a technical perspective but from the perspective of a user who knows VSS and wants to use our SDK :D You don't need to take a closer look at the source code only however the samples in the samples project resp. in the quickstart.md can give an understanding on how it is now looking.

SebastianSchildt · 2024-03-08T05:10:47Z

The VSS related changes seems sensible to me. I remember, I stumbled a bit when in existing code I saw something that seemed to listen to "onSpecificationChanged" or something. That I assume looks "nicer" now.

As I am not fluent at all with the API I cannot really give an in-depth comment, but just from "reading" the examples in Quickstart, I like the new style better. From "VSS" perspective, no ideas how Kotlin evangelist feel about it.

I would assume, when this is merged, we do a new Maven release, and the also adapt the companion?

wba2hi

Minor smallish findings. Functionality-wise still looks good.

docs/QUICKSTART.md

...rc/main/kotlin/org/eclipse/kuksa/connectivity/databroker/subscription/VssNodePathListener.kt

kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/extension/vss/VssLeafBooleanExtension.kt

To stick more to the VSS terminology the VssLeaf was renamed to VssSignal. In addition a VssBranch interface was introduced to clearly separate them from VssSignals.

eriksven · 2024-03-13T10:14:49Z

docs/QUICKSTART.md

@@ -68,8 +68,8 @@ void connectInsecure(String host, int port) {
 ```kotlin
 fun fetch() {
    lifecycleScope.launch {
-        val property = Property("Vehicle.Speed", listOf(Field.FIELD_VALUE))
-        val response = dataBrokerConnection?.fetch(property) ?: return@launch
+        val request = FetchRequest("Vehicle.Speed", Field.FIELD_VALUE)


Could we call this GetRequest as in the gRPC kuksa.val.v1 interface exposed by the databroker? (https://github.com/eclipse/kuksa.val/blob/master/proto/kuksa/val/v1/val.proto).

In the same way, I would prefer get() instead of fetch().

Hey @eriksven! No strong opinion here. This was also something I thought about but decided against. We definitely want to align with the VSS terminology but not necessarily with the Kuksa gRPC one because the definition there may not fit with every other environment (Android in this case) like you already mentioned in your comment. :)

It basically just comes down to the consistent semantic we decided in the Android API here.

GET (and SET) implies a really simple logic -> no overhead when retrieving or setting the information

FETCH implies a more complex process like some other object will take care of the Request for you and asynchronously returns the data at some point.

That the "Request" + "Subscribe" naming fits with the gRPC one is basically just a coincidence.^^

I hope I made the reasoning a bit understandable.

eriksven · 2024-03-13T10:15:59Z

docs/QUICKSTART.md

@@ -78,20 +78,20 @@ fun fetch() {

 fun update() {
    lifecycleScope.launch {
-        val property = Property("Vehicle.Speed", listOf(Field.FIELD_VALUE))
+        val request = UpdateRequest("Vehicle.Speed", Field.FIELD_VALUE)


Maybe use SetRequest and Set() to align with Kuksa.val.v1 API.

eriksven · 2024-03-13T10:26:36Z

Thanks for the PR and the changes which bring this closer to the terminology used within VSS. I also like to use Signal and Branch instead of Leaf. I added two comments to get closer to terminology of the gRPC interface exposed by the KUKSA databroker which has get() and set(). But I am unsure whether this makes sense in Android/Kotlin terminology.

Sorry, for the delayed reaction on my side.

wba2hi

LGTM

wba2hi

Some Tests in VssNodeTest and VssNodeCopyTest seem to fail reliably leading to the failed build.

The filter was wrongly added to the method.

Chrylo · 2024-03-14T14:05:51Z

Some Tests in VssNodeTest and VssNodeCopyTest seem to fail reliably leading to the failed build.

fixed

wba2hi

LGTM

Chrylo added 14 commits February 28, 2024 16:17

refactor: Align Terminology of VSS model classes / public API

5748884

VssProperty -> VssNode close eclipse-kuksa#70

chore: Combine VssNode + VssSpecification

5dbcb42

The VssNode interface is already a good name for the defined VssSpecification properties.

chore: Rename VssSpecification -> VssNode

4420cee

chore: Rename VssSpecification -> VssNode

6d55337

chore: Rename VssDefinition -> VssModelGenerator

ed2f24a

The Definition wording is kinda redundant with what VSS means (Specification). So the annotation name was changed to something it is actually responsible for: Triggering the generation of Models.

chore: Sort DataBroker classes into packages

11fd909

chore: Correct missed naming

8335f51

chore: Update AGP + Compose versions

c359191

chore: Rename PropertyListener -> VssPathListener

6ad4459

test: Adapt test naming

db3ed59

chore: Rename VssNode extensions

3ecbac8

wba2hi requested changes Mar 7, 2024

View reviewed changes

Chrylo added 4 commits March 7, 2024 15:10

chore: Converts varargs -> Collection for performance

defb3a1

chore: Rename missed Specification -> VssNode

25d5447

Merge remote-tracking branch 'eclipse/main' into task-70

a1d0025

test: Adapt tests to vararg change

c0a06a2

wba2hi requested changes Mar 8, 2024

View reviewed changes

Chrylo added 2 commits March 8, 2024 10:48

chore: Adapt QUICKSTART to vararg API

b897e0f

chore: Introduce VssSignal + VssBranch

505b13a

To stick more to the VSS terminology the VssLeaf was renamed to VssSignal. In addition a VssBranch interface was introduced to clearly separate them from VssSignals.

eriksven reviewed Mar 13, 2024

View reviewed changes

wba2hi approved these changes Mar 14, 2024

View reviewed changes

wba2hi requested changes Mar 14, 2024

View reviewed changes

chore: Fix missing child in heritage

4975b33

The filter was wrongly added to the method.

Chrylo and others added 2 commits March 14, 2024 13:38

chore: Simplify filter logic for VssNode.findSignal()

3579307

Merge branch 'main' into task-70

474602b

wba2hi approved these changes Mar 14, 2024

View reviewed changes

wba2hi merged commit af19daf into eclipse-kuksa:main Mar 14, 2024
5 checks passed

wba2hi deleted the task-70 branch March 14, 2024 14:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Align Terminology of VSS model classes / public API #94

Align Terminology of VSS model classes / public API #94

Chrylo commented Mar 6, 2024

wba2hi left a comment

wba2hi commented Mar 7, 2024

SebastianSchildt commented Mar 8, 2024

wba2hi left a comment

eriksven Mar 13, 2024 •

edited

Loading

Chrylo Mar 13, 2024

eriksven Mar 13, 2024

eriksven commented Mar 13, 2024

wba2hi left a comment

wba2hi left a comment

Chrylo commented Mar 14, 2024

wba2hi left a comment

Align Terminology of VSS model classes / public API #94

Align Terminology of VSS model classes / public API #94

Conversation

Chrylo commented Mar 6, 2024

wba2hi left a comment

Choose a reason for hiding this comment

wba2hi commented Mar 7, 2024

SebastianSchildt commented Mar 8, 2024

wba2hi left a comment

Choose a reason for hiding this comment

eriksven Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

Chrylo Mar 13, 2024

Choose a reason for hiding this comment

eriksven Mar 13, 2024

Choose a reason for hiding this comment

eriksven commented Mar 13, 2024

wba2hi left a comment

Choose a reason for hiding this comment

wba2hi left a comment

Choose a reason for hiding this comment

Chrylo commented Mar 14, 2024

wba2hi left a comment

Choose a reason for hiding this comment

eriksven Mar 13, 2024 •

edited

Loading