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

Subscription: Updates on each setValue #652

Open
nayakned opened this issue Sep 12, 2023 · 4 comments
Open

Subscription: Updates on each setValue #652

nayakned opened this issue Sep 12, 2023 · 4 comments
Labels
Databroker Issues related to databroker (core) enhancement New feature or request

Comments

@nayakned
Copy link
Contributor

For quite a few usecases (e.g. heartbeat signals) it is essential that the update messages to a subscription are sent each time the corresponding datapoint is set (irrespective of whether the value changes or not). Currently this is by default the case with target values (which is also not good for applications which need to triggered only on change of values).

Suggestion: Add an optional flag during subscription request to figure out if the subscribing application needs the information when set(Target)Value is called or only when the value changes.

@nayakned nayakned changed the title Subscription: Updates on each set value Subscription: Updates on each setValue Sep 12, 2023
@SebastianSchildt SebastianSchildt added enhancement New feature or request Databroker Issues related to databroker (core) labels Sep 12, 2023
@argerus
Copy link
Contributor

argerus commented Sep 13, 2023

I agree with the problem description.

So I see two distinct observations here.

Values

Suggestion: Add an optional flag during subscription request to figure out if the subscribing application needs the information when set(Target)Value is called or only when the value changes.

This can make sense. But, a related question is whether this is something that should be decided by the client, or if this is an inherent property of the signal/datapoint. If it is an inherent property of the signal, subscribing to it should always behave in accordance with that.

The VHAL implementation in Android has decided that this is an inherent property of the signal, and this is also how the implementation works internally in databroker. If a signal/datapoint is ChangeType::OnChange it will send notifications only when the signal/datapoint changes. If it is ChangeType::Continous, it will send notifications periodically* (regardless of whether the value changes).

Firstly, this information is not part of VSS though. But since VSS supports adding additional metadata to signals, the only thing missing is adding support for reading this extended metadata in databroker. Right now, it defaults to ChangeType::OnChange as you've noticed, (maybe that's the wrong default).

The second part is that a client should be able to supply a preferred update frequency when subscribing to ChangeType::Continuous type of signals, as to not introduce more load to the client or system than necessary.

This "solution" might seem more complicated than just letting the client decide. But another way to look at it, is that this is metadata (just as specifying the data type) that is needed in order to implement correctly behaving providers of these signals. And in a sense, also correctly behaving clients.

Target values

update messages to a subscription are sent each time [...] (irrespective of whether the value changes or not) [...] is by default the case with target values (which is also not good for applications which need to triggered only on change of values).

Setting a target value is distinct from setting a value. It is more akin to calling a function implemented by an actuator provider. If the provider is not available at the time of the call, it will not be notified. Calling the same "function" again, with the same value, when the provider is available should notify the provider even if the target value doesn't change. Because of that, I think it makes sense to send a notification every time (regardless of what the previous target value was). It might make sense to avoid this notification if the target value doesn't differ from the current value, though.

With that said, the whole mechanism around notifying an actuator provider might be reworked in order to enable the provider to respond to these requests (and propagate the response to the caller), see #560.

(*) Currently, every time the value is set.

@argerus
Copy link
Contributor

argerus commented Sep 13, 2023

It might still be useful to be able to subscribe to signals of type OnChange::Continous based on when their value actually changes. So maybe a solution in line with what is suggested

Suggestion: Add an optional flag during subscription request to figure out if the subscribing application needs the information when set(Target)Value is called or only when the value changes.

would be:

  • Change the default type to OnChange::Continous (interim solution).
  • Add an option (filter) to the subscription method that specifies that a notification should only be sent when the value actually changes.

@SebastianSchildt
Copy link
Contributor

My 2cts

Maybe "OnChange::Continuous" is a better "default, as it migth increase load, but you would never "loose" anything, and it may be less "unexptected

If the plumbing would already be there, how hard would it be, for databroker, upon parsing the VSS JSON checking for a field x-signal-type that is either "CONTINOUS" or "ON_CHANGE" and set it accordingly (or default back to Continous if nothing is there).

This ability would help a lot in deployment I think, i,e,

  • The OP has an overlay with a "heartbeat signal", i.e. he needs to create is VSS tree for databroker anyway, and might just add x-signal-type: Continuous to his overlay, and no matter how the default is, everything will work as expected

  • We could do the same in our DBC Feeder example: The config is an overlay anyway, so e.g. for DoorStates we could just say x-signal-type: "ON-CHANGE"`, So just loading that VSS config to databroker would make things be "efficient" again

@BjoernAtBosch
Copy link

BjoernAtBosch commented Apr 25, 2024

I agree with John's elaboration about the difference between (current) values and target values.
To achieve what Naresh proposed and in parallel reducing load by avoiding unnecessary notifications, there could be two different parameters on provider side:

  • Maximum update frequency (could alternatively also be "minimum update interval"): This is the maximum frequency a provider will (or would be able to) update the signal (if it the value changes)
  • Minimum update frequency (or maximum interval between updates): This is the minimum frequency a signal is "updated"/"send out" even if there is no change. This would serve as "heart beat", i.e. if this update is missing the broker (or its clients) could assume, the signal is currently not provided any longer.

Clients could also be allowed to specify something like a "maximum" update frequency used to limit there update load by reducing the number of notifications they will receive. But for that it should be investigated, what will be done with the value updates in-between: Drop, calculate mean value (how?), collect and send as bunch, ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Databroker Issues related to databroker (core) enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants