w3c / automotive-viss2

MIT License
9 stars 15 forks source link

(WIP) Adding Apache IoTDB as a new State Storage backend #110

Closed slawr closed 4 months ago

slawr commented 5 months ago

Hi, can you please review this WIP PR for possible future merging.

At the moment there are no changes to the WAII documentation. What do I need to add there? During development I was unclear what specification (format, zone etc) WAII was using for time for get/set. Can you confirm? I see I also need to rebase on the current master head, which I will do once reviewed.

Summary

Add support for the Apache IoTDB database as a new State Storage backend by extending the WAII Service Manager to connect to a running IoTDB instance.

Detail

Development environment:

Runtime assumptions:

Sanity testing

Sanity testing was carried out within a Docker deployment of the COVESA Central Data Service Playground:

  1. Create timeseries within database containing VSS nodes and populate some date.
    
    IoTDB> show timeseries
    +-------------------------------------------------------------+-----+---------------+--------+--------+-----------+----+----------+--------+------------------+--------+
    |                                                   Timeseries|Alias|       Database|DataType|Encoding|Compression|Tags|Attributes|Deadband|DeadbandParameters|ViewType|
    +-------------------------------------------------------------+-----+---------------+--------+--------+-----------+----+----------+--------+------------------+--------+
    |          root.test2.dev1.`Vehicle.CurrentLocation.Longitude`| null|root.test2.dev1|   FLOAT|   PLAIN|     SNAPPY|null|      null|    null|              null|    BASE|
    |           root.test2.dev1.`Vehicle.CurrentLocation.Latitude`| null|root.test2.dev1|   FLOAT|   PLAIN|     SNAPPY|null|      null|    null|              null|    BASE|
    |root.test2.dev1.`Vehicle.Cabin.Infotainment.HMI.DistanceUnit`| null|root.test2.dev1|    TEXT|   PLAIN|     SNAPPY|null|      null|    null|              null|    BASE|
    +-------------------------------------------------------------+-----+---------------+--------+--------+-----------+----+----------+--------+------------------+--------+

2. Using WAII HTML client connect to WAII and 'get' VSS node Vehicle.CurrentLocation.Longitude
3. Using WAII HTML client connect to WAII and 'set' VSS node Vehicle.Cabin.Infotainment.HMI.DistanceUnit
4. Check HTML client reports success and values set/returned match the data in the database.
![playground-iotdb-set](https://github.com/w3c/automotive-viss2/assets/4562522/878b6b54-d608-4fa4-94b9-de2445c89cb3)
UlfBj commented 5 months ago

Regarding the

Runtime assumptions:

  1. IoTDB server lifetime (e.g. startup and shutdown) is handled externally to WAII
  2. Management (e.g. creation/deletion) of the IoTDB timeseries containing VSS nodes is handled externally to WAII.
  3. Connection config is currently a build time setting in the Service Manager header. Runtime configuration will be added in a later commit.

1 is fine as long as it i documented how to handle it. Documentation should be found in the tutorial I think. 2 I think that there should be a "tool" available to do this. An example on how to support this could be the https://github.com/COVESA/ccs-components/blob/master/statestorage/sqlImpl/statestorage_mgr.go that sets up an SQLite table using the vsspathlist.json that is created by the WAII server at startup. 3 Selection of one of the WAII supported data store alternatives should be done using the command line parameter.

UlfBj commented 5 months ago

Regarding documentation there is the WAII Data Storage chapter that currently has sections on SQLite and Redis alternatives. I think these should be separate chapters, containing more information than now. IotTDB should have a separate chapter here in my view.

slawr commented 5 months ago

3 Selection of one of the WAII supported data store alternatives should be done using the command line parameter.

Hi, I extended the current implementation to do that. When I said runtime config I meant IoTDB details such as the connection config (IP, port etc) and the DB tree prefix with which to find the VSS data. Basically this info

For developer use these (as with redis connection) are typically quite static, e.g. localhost is sufficient IP, but longer term I would like to make these runtime both for flexibility and importantly to be able to have a pre-built WAII Docker image. With an image initial setup becomes easier, e.g. docker compose is just pulling images and configuring them.

petervolvowinz commented 5 months ago

I think it looks ok.

slawr commented 5 months ago

Regarding the timestamps I spent some time looking over the code again this afternoon.

If I look to the protocol specs I often end up getting confused with how W3C organises the seperate pages and versions. For example: Here the json schema timestamp is defined as integer https://w3c.github.io/automotive/vehicle_data/vehicle_information_service.html#terminology Whilst here it is ISO8601 https://www.w3.org/TR/viss2-core/#timestamp

I assumed the former is an earlier 'VIS' spec but then the document date is 2024.

Ultimately the State Storage integration is to the Service Manager getVehicleData() and setVehicleData() methods so it's the requirements for those methods that are ultimately key.

slawr commented 5 months ago

There are a few lines of logging commented out in my code that I left in for debug purposes. Does WAII have a debug log mechanism (something you can turn on/off) I could replace those with?

slawr commented 5 months ago

1 is fine as long as it i documented how to handle it. Documentation should be found in the tutorial I think. 2 I think that there should be a "tool" available to do this. An example on how to support this could be the https://github.com/COVESA/ccs-components/blob/master/statestorage/sqlImpl/statestorage_mgr.go that sets up an SQLite table using the vsspathlist.json that is created by the WAII server at startup.

Re 1) Agreed on the need for some level of IoTDB documentation. Minimum being the runtime needs for the user. I need to do this anyway for the Central Data Service Playground. OK for WAII I'll concentrate on extending the tutorial documentation.

Re 2) I have not come to a conclusion on how best to do that yet. As a full service DB there are a lot of methods for manipulating it and also flexibility in how the database is organised. There is a balance to be struck between "easy start" and flexibility if someone is investigating something complicated. For sure at a minimum there should be documentation for "easy start" do these 3 steps and WAII will get/set some data.

Pragmatic approach might be for me to add the basic documentation to the tutorial in this PR. We can then consider merging. Improvements possible later.

petervolvowinz commented 5 months ago

@slawr Yes, you are correct it is the getVehicleData and setVehicleData that is the key. I made a direct interface skipping the state storage using a simple two way hash structure to interface the RL broker. That is a direct gRPC connection to the broker from the server. I then added another interface using the stage storage approach designed by Ulf interfacing the broker. The direct approach is actually a more correct way to interface the broker there are issues with this for some of the additional features such as caching though.