search

microsoft / azure_spatial_anchors_ros

ROS wrapper for the Azure Spatial Anchors Linux SDK, allowing robots (and other devices with vision-based sensors) to co-localize with other robots, AR-enabled phones, and Hololens devices.
https://azure.microsoft.com/en-us/services/spatial-anchors/
MIT License
86 stars 20 forks source link

Strings in message and service and definitions #23

Closed RobertBlakeAnderson closed 2 years ago

RobertBlakeAnderson commented 2 years ago

Currently in definitions such as FindAnchor.srv, the client is required to concatenate a string of semicolon-separated IDs in order to reference multiple anchors.

I propose it would be better practice for the server to take a vector of IDs. If the backend requires the combined string, it should be created within the ROS service callback.

jeffdelmerico commented 2 years ago

Thanks for the suggestion. The request to the cloud service does need to be in the form of a concatenated string of uuids, but I think your proposal to expose this as a vector instead would be a bit easier to use.

Tagging @EricVoll in case he can add that feature in the near future. But please feel free to implement it yourself and create a PR if this functionality is relevant for your use case.

RobertBlakeAnderson commented 2 years ago

I'm willing to make a PR. Initially I'll add the vector member and deprecation warnings on the current string member, so that we don't break existing clients. I'll make a note to make another PR in a few months to actually remove the string.

jeffdelmerico commented 2 years ago

Great! That sounds like a good plan.

EricVoll commented 2 years ago

Thanks @RobertBlakeAnderson for helping out!