APP-8003 Add world state store service

[DTCurrie]

Introducing a new service type, the WorldStateStoreService.

Visualization modules will utilize this service to retrieve information about the machine's world object store for rendering on the client side.

List of changes:

1. Add Points and Line geometry types and add them to the oneof geometry_type on the Geometry message
2. Introduce the WorldStateStoreService with three RPCs:

• ListUUIDs returns all transform UUIDs
• GetTransform returns a transform by UUID
• DoCommand sends/receives arbitrary commands

3. Update the Transform message (these include breaking changes):

// Transform contains a pose and two reference frames. `parent` is the starting reference frame, and `name` is the observer reference frame.
// `pose` represents the pose of an object in the `parent` frame as observed within the `name` frame.
message Transform {
  // The name of the transform
  string name = 1;
  reserved "reference_frame";

  // Deprecated: use `parent` and `pose` instead
  reserved 2;
  reserved "pose_in_observer_frame";

  repeated Geometry geometries = 3;
  reserved "physical_object";

  // The name of the observer reference frame
  string parent = 4;

  // The pose of the named frame as observed from the parent frame
  Pose pose = 5;

  // The UUID of the transform
  bytes uuid = 6;

  // Can hold information like color, opacity, points colors, collision_allowed, etc...
  optional google.protobuf.Struct metadata = 7;
}

These changes are primarily aimed at aligning the usage of the Transform more closely with the expectations of engineers working with motion and visualization tools.

A quick look at RDK and the SDKs seems to indicate that none of our code interacts with Transforms, and just passes them through in various APIs. Our thinking is that, despite these being breaking changes, they should have a relatively low impact and set us up with a better foundation on which to expand our motion and visualization tools.

Scope doc: https://docs.google.com/document/d/1ionvyBa7x3HZU_rwDPBvrAUrQjBrP6sJ10Z_xbBTue8/edit?tab=t.0#heading=h.tcicyojyqi6c

[DTCurrie]

Removing some folks from review while I tweak things to avoid noise.

[raybjork]

Was there a scope for this? Would be helpful to understand the thinking behind it

[DTCurrie]

Was there a scope for this? Would be helpful to understand the thinking behind it

Yes sorry, meant to add a link in the description. Here you go: https://docs.google.com/document/d/1ionvyBa7x3HZU_rwDPBvrAUrQjBrP6sJ10Z_xbBTue8/edit?tab=t.0#heading=h.tcicyojyqi6c

The API design has been slightly tweaked based on testing @micheal-parks did, but for the most part things are the same.

[raybjork]

This looks good to me from a code perspective based on what was decided on in the scope doc. I wonder though if it would be possible to put this under the app proto instead of the general service proto. I expect this API to change frequently and it seems like there is a lot more leniency towards making breaking changes to the App API as compared to resources

[DTCurrie]

This looks good to me from a code perspective based on what was decided on in the scope doc. I wonder though if it would be possible to put this under the app proto instead of the general service proto. I expect this API to change frequently and it seems like there is a lot more leniency towards making breaking changes to the App API as compared to resources

Fair point, I brought this up to steve and he suggested it could live here because it is a new service resource type, but we can mark it as experimental for now, and allow some level of leniency until we have published some Viam-supported visualization modules and are confident with the API.

@micheal-parks has also been testing visualizations with the assumption that this is how the API will work, so we have some real-world proof of concepts to work from. I ended up writing the code to implement the fake model for testing in the RDK. I can work with micheal to test these APIs using his current code and ensure they work as expected, then report back on the results.

I'd be happy to get some more opinions on this before moving forward, though, if you have anyone in mind.

[DTCurrie]

Note from @erh to use existing world object state

[DTCurrie]

Updated per latest conversation:

1. New message type: GeometryInFrame, which is just one geometry instead of a repeated list
2. Add uuid and metadata to PoseInFrame and GeometryInFrame
3. Add label to PoseInFrame
4. Add Line and Points to Geometry['geometryType']. They have one field: array = Float32Array. This is essential for faster over-the-wire speeds, especially for point clouds
5. The world state store service is a list of GeometryInFrame / PoseInFrame

[raybjork]

Ok after reading deeper I understand more. I think initially I was just surprised by everything that was changing because that was not my read from the email for how the changes would need to be written.

I think all we need to do here is add the metadata and UUID onto the GeometryInFrame object since PoseInFrame is a GeometryInFrame with a nil geometry_type (we could alternately do it on the geometry itself, I'm not sure if this would be better). Then the GetWorldStateObjectResponse can just become only the GeometryInFrame

[raybjork]

Hi, @biotinker brought up a great point which is that we've wanted the Transform.Geometry field to be a repeated for a while instead of just an optional. While we are here and breaking this can we also make this change? If @DTCurrie and @micheal-parks are on board I will check in with Eliot that this change makes sense

[DTCurrie]

Hi, @biotinker brought up a great point which is that we've wanted the Transform.Geometry field to be a repeated for a while instead of just an optional. While we are here and breaking this can we also make this change? If @DTCurrie and @micheal-parks are on board I will check in with Eliot that this change makes sense

I am ok with that. As I was looking into implementing @biotinker's suggestions, I noticed that we would encounter some issues with managing composite geometries compared to our protos here. Should we remove the Line and Points messages and instead leave them as multi/composite geometries?

[biotinker]

Should we remove the Line and Points messages and instead leave them as multi/composite geometries?

While I'm not sold on needing a special line type and think it would be nice to implement multi geometries with lines as 0-radius capsules, I acknowledge this would be a larger lift and not strictly necessary.

Points we do need to make a change for. Pointcloud is notably absent from the Geometry message.

Currently PCDs are sent over the wire elsewhere as bytes. If we transmit points as []r3.Vector, that omits potentially collision-relevant data from the pointcloud, specifically certainty values/cutoffs.

[DTCurrie]

Added this based on a conversation with @stevebriskin and @micheal-parks . The intention of this RPC is to allow visualization tools to open a stream and listen for change events from this service. We define a change event as one of the following:

// TransformChangeType is the type of change that has occurred for a transform.
enum TransformChangeType {
  TRANSFORM_CHANGE_TYPE_UNSPECIFIED = 0;
  TRANSFORM_CHANGE_TYPE_ADDED = 1;
  TRANSFORM_CHANGE_TYPE_REMOVED = 2;
  TRANSFORM_CHANGE_TYPE_UPDATED = 3;
}

When an implementor of this service is going to Send a change message, they must define what type of change event is occurring in the response:

message StreamTransformChangesResponse {
  TransformChangeType change_type = 1;
  common.v1.Transform transform = 2;

  // The field mask of the Transform that has changed, if any. For added transforms, this will be empty. For updated
  // transforms, this will be the fields that have changed. For removed transforms, this will be the Transform's UUID
  // path.  
  google.protobuf.FieldMask updated_fields = 3;
}

Note we use a FieldMask, which will allow the implementor only to include fields that were changed during a TRANSFORM_CHANGE_TYPE_UPDATED event, or just the change_type and transform.uuid fields for a TRANSFORM_CHANGE_TYPE_REMOVED event.

The field mask will enable the implementor to reduce the amount of data sent over the wire in a stream response to only the relevant data, allowing the client to understand how to manage each event it receives quickly. This should be particularly helpful when dealing with Transforms that have large amounts of geometry data (like point clouds) or have large quantities of geometries defined.

[DTCurrie]

Do we still want to introduce the Line type if we are allowing Transform to have multiple Geometrys?

[raybjork]

I'm not sure what this has to do with Transform having multiple geometries

[DTCurrie]

Based on this comment:

While I'm not sold on needing a special line type and think it would be nice to implement multi geometries with lines as 0-radius capsules, I acknowledge this would be a larger lift and not strictly necessary.

[micheal-parks]

should line be bytes too to match Pointcloud?