eclipse-velocitas / vehicle-app-python-sdk

vehicle-app-python-sdk
Apache License 2.0
7 stars 18 forks source link

Fix errors in examples #141

Open erikbosch opened 1 month ago

erikbosch commented 1 month ago

I performed the following test:

What I fixed:

What I did not fix:

erikbosch commented 1 month ago

IMO if we now do some fixes on the examples we should align everything, that means we should use VSS 4.0 everywhere and also adapt the datapoints accordingly.

Furthermore we should align the structure to have a main and vapp file as in the seat-adjuster example.

If we don't do it correct IMO this PR is not helping a lot, since we still need additional work...

How is it as of today? Is Databroker always started with default VSS catalog when starting runtime, i.e. not considering what is written in appManifest? If so it makes sense to adapt examples so they all use default VSS version. I did this work mostly to validate that there are no regressions coming from #139 which does not seem to be the case, if we are to verify/update all aspects of each example more time would be needed.

It also appears to me that many examples are quite similar, so do we need them all? If we would pend more time on them it would also be great if they could be tested as part of CI when updating SDK. That does not happen today, right?

MP91 commented 1 month ago

IMO if we now do some fixes on the examples we should align everything, that means we should use VSS 4.0 everywhere and also adapt the datapoints accordingly. Furthermore we should align the structure to have a main and vapp file as in the seat-adjuster example. If we don't do it correct IMO this PR is not helping a lot, since we still need additional work...

How is it as of today? Is Databroker always started with default VSS catalog when starting runtime, i.e. not considering what is written in appManifest? If so it makes sense to adapt examples so they all use default VSS version. I did this work mostly to validate that there are no regressions coming from #139 which does not seem to be the case, if we are to verify/update all aspects of each example more time would be needed.

It also appears to me that many examples are quite similar, so do we need them all? If we would pend more time on them it would also be great if they could be tested as part of CI when updating SDK. That does not happen today, right?

According to databroker: velocitas init should download the specified file and add the config for the databroker. so databroker should always be started with the vss file which is defined in the app manifest.

Agreed that reworking the examples can take a lot of time, that's why it did not happen so far :D

Is there maybe an easy solution to at least be able to use the examples in the template without errors? your initial message at least mentions some more errors besides the ones fixed?

erikbosch commented 1 month ago

Putting this one on hold for now - before merging we should better run through the examples thoroughly and update documentation. Possibly adapt examples so they can be imported by the template.