Skip to content
This repository has been archived by the owner on Dec 18, 2024. It is now read-only.

Add actuator filtering to kuksa-client target methods #687

Closed
rafaeling opened this issue Oct 19, 2023 · 6 comments
Closed

Add actuator filtering to kuksa-client target methods #687

rafaeling opened this issue Oct 19, 2023 · 6 comments

Comments

@rafaeling
Copy link
Contributor

rafaeling commented Oct 19, 2023

After fixing permission handling (#654), the actuator filtering was removed from the broker.
We decided that the broker should not throw an error in this case or contain any filter logic on its side. Instead, the responsibility for this kind of filtering should be from the kuksa-client.

The current output on the kuksa-client when calling getTargetValue, getTargetValue includes a combination of actuators, sensors, and attributes. The necessary fix to filter only actuators should be implemented:

Test Client> getTargetValues Vehicle.PowerOptimizeLevel Vehicle.IsMoving
[
    {
        "path": "Vehicle.PowerOptimizeLevel",
        "actuator_target": {
            "value": 8,
            "timestamp": "2023-10-19T08:02:09.639186+00:00"
        }
    },
    {
        "path": "Vehicle.IsMoving"
    }
]

Output expected or something similar:

Test Client> getTargetValues Vehicle.PowerOptimizeLevel Vehicle.IsMoving
[
    {
        "path": "Vehicle.PowerOptimizeLevel",
        "actuator_target": {
            "value": 8,
            "timestamp": "2023-10-19T08:02:09.639186+00:00"
        }
    }
]
@SebastianSchildt
Copy link
Contributor

I see that on getTarget there may be arguments for both types of behavior (as I think there was a discussion waht about (corner?) cases where on get (on GRPC API Level) requests both value and targetValue for something that is a sensor.

Just for clarification though: when we SET target Values I would still very mich expect an error when this is attempted on an attribute or sensor. Is that still the case?

@lukasmittag
Copy link
Contributor

lukasmittag commented Oct 19, 2023

 {
      "path": "Vehicle.IsMoving"
 }

but this would have been what would be returned before so what's the problem?

@argerus
Copy link
Contributor

argerus commented Oct 19, 2023

Just for clarification though: when we SET target Values I would still very mich expect an error when this is attempted on an attribute or sensor. Is that still the case?

Yes. Setting anything but an actuator will return an error.

@rafaeling
Copy link
Contributor Author

rafaeling commented Oct 19, 2023

 {
      "path": "Vehicle.IsMoving"
 }

but this would have been what would be returned before so what's the problem?

As it was initially implemented here (9b7861d#diff-ae8c606603352f073807e33a1cebe4a184a2eb69fe92453536878c44857498c1R100) and later removed, I believe the purpose of these lines was just then to reduce the number of irrelevant signals (sensors and attributes) sent when only retrieving actuator values.

However, it appears that I was mistaken, as it seems that kuksa-client handles actuator filtering correctly here (https://github.com/eclipse/kuksa.val/blob/master/kuksa-client/kuksa_client/grpc/aio.py#L159). Perhaps some refinement would be good to prevent displaying an empty path when there is no actuator_target value.

@SebastianSchildt
Copy link
Contributor

So I checked with 0.4.0 couple of databroker and with master . The behavior has not changed. So this at least not a bug. (we might discuss to change the behavior, but again at least not criticial it seems)

I would not filter it in the kuksa-client. Currently the client shows what it gets from GRPC API and this is how it should be, i.e. IF we want filter it in the client, that would more of a hint to me to change it in databroker.

So maybe this can be closed?

Or did I miss(understood) something?

@rafaeling
Copy link
Contributor Author

I think can be closed

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

No branches or pull requests

4 participants