Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding a simple persistence provider #11

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

SebastianSchildt
Copy link
Contributor

This is a simple persistence provider.

It allows to persist and restore chosen VSS path between databroker restarts.

Signed-off-by: Sebastian Schildt <sebastian.schildt@de.bosch.com>
Copy link
Contributor

@lukasmittag lukasmittag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First comments, need to take a deeper look on code level again

@@ -0,0 +1,71 @@
# KUKSA Persistence Provider

All data in KUKSA is ephemereal. However, in a car there is often data that does not change over the lifetime of a vehicle, and data where you want changes to be persisted over ignition cycles.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
All data in KUKSA is ephemereal. However, in a car there is often data that does not change over the lifetime of a vehicle, and data where you want changes to be persisted over ignition cycles.
All data in KUKSA is ephemeral. However, in a car there is often data that does not change over the lifetime of a vehicle, and data where you want changes to be persisted over ignition cycles.


All data in KUKSA is ephemereal. However, in a car there is often data that does not change over the lifetime of a vehicle, and data where you want changes to be persisted over ignition cycles.

This provider can achieve this. It can restore certain values upon startup, either sensor (current) values, or actuations.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you put soe thought into how this would work for actuations with the new concept that will be introduced in the kuksa.val.v2?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that good I think - Isn't the thoughts in kuksa.val.v2 that there should only be a single instance interested in the target/actuation-value? I.e. there cannot be multiple entities interested in the same actuation value?


This provider can achieve this. It can restore certain values upon startup, either sensor (current) values, or actuations.

An example for one-time restoration of current values are attributes that are maybe not set in a default VSS model deployed to ALL cars of a specific variant, but nevertheless are constant of a specific car, such as the VIN or the Vehicle Color.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An example for one-time restoration of current values are attributes that are maybe not set in a default VSS model deployed to ALL cars of a specific variant, but nevertheless are constant of a specific car, such as the VIN or the Vehicle Color.
An example for one-time restoration of current values are attributes that are maybe not set in a default VSS model deployed to ALL cars of a specific variant, but nevertheless are constant of a specific car, such as the VIN or the Vehicle.Color.

either use Vehicle.Color or vehicle color


An example for one-time restoration of current values are attributes that are maybe not set in a default VSS model deployed to ALL cars of a specific variant, but nevertheless are constant of a specific car, such as the VIN or the Vehicle Color.

This provider can also watch (subscribe) certain current or actuation values. This is useful when interacting with components that do not provide their own persistence management. Assume a climate control UI that can react on unser input and interact with the HVAC system, but is otherwise stateless. By watching and restoring the desired target temperature, the user's preference is saved and restored, without the HVAC UI needing any specific code to handle this.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This provider can also watch (subscribe) certain current or actuation values. This is useful when interacting with components that do not provide their own persistence management. Assume a climate control UI that can react on unser input and interact with the HVAC system, but is otherwise stateless. By watching and restoring the desired target temperature, the user's preference is saved and restored, without the HVAC UI needing any specific code to handle this.
This provider can also monitor (subscribe) certain current or actuation values. This is useful when interacting with components that do not provide their own persistence management. Assume a climate control UI that can react on unser input and interact with the HVAC system, but is otherwise stateless. By watching and restoring the desired target temperature, the user's preference is saved and restored, without the HVAC UI needing any specific code to handle this.

]
},

"restore-and-watch": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay if this here is called watch ignore my command above

};

if datapoint_entries.is_none() {
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log message?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this only for error situations, i.e. we have already got one of the log messages above? Replace None with return Noneabove?

* SPDX-License-Identifier: Apache-2.0
********************************************************************************/

pub mod filestorage;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why split storage and filestorage into two files? Just move this to filestorage imo

@erikbosch
Copy link
Contributor

@lukasmittag - I was in contact with Sebastian and merged a PR from Matthias, so your previous comments are out of sync. Do you want to "reapply" them or should I integrate them? I anyway need to squash the commits as Matthias do not have signed the ECA.

@erikbosch
Copy link
Contributor

I created a replacement PR at #13 where the commits have been squashed. It seems most comments have been addressed. I have not tested the PR, but this is incubation but we should preferably say something in main README

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants