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

GRPC proto cleanup #336

Merged

Conversation

argerus
Copy link
Contributor

@argerus argerus commented Aug 26, 2022

  1. Move metadata fields to .metadata in Datapoint.

    This makes it more clear what VIEW_METADATA refers to and it makes it possible to have FIELD_METADATA.

  2. Add fields in addition to view in Get & Subscribe

    This makes it possible to subscribe to single fields. It's not really needed for Get but it seems consistent to add it to both.

    Because of this I also removed for example VIEW_UNIT since it's now equally easy to add FIELD_METADATA_UNIT to fields. In general I think view is a good solution to having a default view and a convenient way to request multiple fields. But perhaps not for specifying individual fields.

  3. Include which fields have changed in subscription replies.

    In order for a client to tell which field has changed (e.g. if it subscribes to both value and actuator_target).

  4. Various cleanup

    • Fix typos, prefix all enums, update documentation, removed references to Status
    • Add [field: FIELD_NAME] to all fields to make it super obvious which is which.

Not included

  • I still think there should be some way to specify that a value is not available. Compare with Android VHAL, that has VehiclePropertyStatus for every VehiclePropertyValue which can be AVAILABLE NOT AVAILABLE or ERROR. And databroker which has the same concept embedded in the oneof value. The point is that a missing value isn't even necessarily an error and it should still be possible to at least Get and Subscribe to such a Datapoint.

 * Typos, documentation updates, prefix enums, removed references to Status
 * Add [field: FIELD_NAME] to all fields to make it (more) obvious.
 * Move fields which are metadata to Metadata (.metadata in Datapoint).
 * Add fields to Get & Subscribe as well
 * Include which fields have changed in subscription replies.
Copy link
Contributor

@SebastianSchildt SebastianSchildt left a comment

Choose a reason for hiding this comment

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

Argh, this was supposed to be already in. Must have messed sth up with git...

@SebastianSchildt SebastianSchildt merged commit df17e2e into eclipse-archived:master Aug 26, 2022
@SebastianSchildt SebastianSchildt deleted the feature/update_proto branch August 26, 2022 14:45
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants