Introduce common classes for v1 and v2 protocols #19
Conversation
wba2hi
commented
Mar 11, 2025
•
wba2hi
commented
- Removes the DataBrokerSubscriber
- Introduces a common DataBrokerConnector / DataBrokerConnection
- DataBrokerConnection exposes kuksaValV1 and kuksaValV2 protocols as properties
- Updates kuksa.val.v1 and kuksa.val.v2 protos
- Add parameter bufferSize for subscribe method (kuksa.val.v2)
- Add streamedUpdate method (kuksa.val.v1)
BREAKING CHANGE: kuksa.val.v1 and kuksa.val.v2 specific classes were slimlined - DataBrokerConnector: Package has been changed - DataBrokerConnection: Package has been changed - DataBrokerConnection: Api specific calls were moved to DatabrokerConnection#kuksaValV1 resp. DatabrokerConnection#kuksaValV2 - DataBrokerConnectorV2: Removed - DataBrokerConnectionV2: Removed
... due to simultaneous access to shared resources. Due to a Gradle or Library update it seems that the debug and release tests are executed using multiple threads, which might lead to one thread starting the corresponding Databroker Container, while the other closes it. This might lead to timeouts and therefore failing tests. Fixed it, by creating a container for each test class. Previously only one container was created for each testset (testRelease / testDebug)
Solution is to sophisticated for the kuksa-java-sdk. However the managemend of the subscription could be put inside velocitas vehicle-app-java-sdk at some later point in time. BREAKING CHANGE: DataBrokerConnection#unsubscribe was removed the unsubscribe method now returns a CancelableContext which must be stored by the user, if canceling the subscription is needed.
wba2hi
force-pushed
the
introduce-common-classes-for-v1-and-v2-protocols
branch
from
6754726 to
4fd7c0f
Compare
wba2hi
force-pushed
the
introduce-common-classes-for-v1-and-v2-protocols
branch
2 times, most recently
from
e0f5b77 to
7940bba
Compare
wba2hi
force-pushed
the
introduce-common-classes-for-v1-and-v2-protocols
branch
from
7940bba to
c145f33
Compare
|
I tested the fixes applied here w.r.t. the failing tests using boschglobal: https://github.com/boschglobal/kuksa-java-sdk/actions/runs/13831796420 - while the build was failing continuously before, it was no longer failing for 5 consecutive runs. The fix was to align the timeout values for a) Databroker connect (previously set to 5) b) Databroker Image Download and c) Databroker Container start and give a bit "more air to breath". Only a) should have an impact on test run times, because there are a few tests which test connecting against an invalid Databroker. |
| .setPath(vssPath) | ||
| .addFields(field) | ||
| .addAllFields(fields) |
There was a problem hiding this comment.
why do we need a fields parameter when addAllFields() suggest that you just add all fields that are available?
There was a problem hiding this comment.
I think this is a misunderstanding. #addAllFields will not "add all fields which are available", it will "add all fields which (to the Proto Model) which are provided per parameter" or described differently
#addFields adds a single field, while #addAllFields adds a list of fields.
...-sdk/src/main/kotlin/org/eclipse/kuksa/connectivity/databroker/v1/DataBrokerTransporterV1.kt
Outdated
Show resolved
Hide resolved
| */ | ||
| fun subscribeById( | ||
| signalIds: List<Int>, | ||
| bufferSize: Int = 0, |
There was a problem hiding this comment.
Unfortunately Java does not know a real UInt as of yet. Kotlin in fact does have a UInt class, however when called from non-kotlin code they will simply be represented as an Int with some special constructors. We had some issues with this representation. It's also represented as an Int in the corresponding Proto Model, so let's leave it like this. However I will test what happens if a negative bufferSize is specified and depending on the outcome maybe add specific handling.
There was a problem hiding this comment.
Testing shows, that when a negative BufferSize is send the following error is send from databroker:
INVALID_ARGUMENT: Subscription buffer_size max allowed value is 1000
io.grpc.StatusException: INVALID_ARGUMENT: Subscription buffer_size max allowed value is 1000
Looks like we could improve the error message on Databroker-side (instead of only speaking about max value). However, the Databroker correctly deals with the error message, so I won't add additional handling, only an additional test.
There was a problem hiding this comment.
During testing I found out, that the Exception was not thrown when calling kuksaValV2#subscribe but only when consuming the flow
val subscribeRequest = SubscribeRequestV2(listOf("Vehicle.Speed"))
val flow = connection.kuksaValV2.subscribe(subscribeRequest)
flow.collect { response -> // this call will throw the Exception
}
To have it fail early in case of bufferSize issue, I added separate handling. However this does not cover the other Exceptions (e.g. PATH not found, Unauthenticated etc,...). They will still be thrown only when consuming and not as a DataBrokerException, but as a io.grpc.StatusException
...pse/kuksa/connectivity/databroker/v1/authentication/DataBrokerConnectorAuthenticationTest.kt
Show resolved
Hide resolved
