fix(sdk): address Vision service review feedback

- Respect server-returned mime_type in get_object_point_clouds: server
  now populates response.mime_type from the result's cloud.mime_type
  (not echoing the request), and client reads response.mime_type()
  rather than the request mime. Matches the proto contract and the Go
  RDK client behavior. Adds a regression test where request mime
  differs from server-returned mime.
- Initialize Vision::detection::confidence, classification::confidence,
  and all three properties bools via NSDMIs so default-constructed
  values are well-defined.
- Introduce Vision::image as a Vision-specific input type for
  get_detections / get_classifications. Camera::raw_image (alias) is
  kept only for capture_all_result.image where the wire type is the
  full Camera Image proto. Avoids silently dropping source_name on
  inputs whose wire format doesn't carry one — mirrors how Go's RDK
  takes image.Image (decoupled from the camera service).
- Rename point_cloud_object::point_cloud field to ::cloud so it no
  longer shadows the Vision::point_cloud type alias.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: HipsterBrown

src/viam/sdk/services/private/vision.cpp

@@ -83,16 +83,16 @@ Vision::classification from_proto(const vpb::Classification& p) {
 }
 
 void to_proto(const Vision::point_cloud_object& o, ::viam::common::v1::PointCloudObject* out) {
-    out->set_point_cloud(std::string(reinterpret_cast<const char*>(o.point_cloud.pc.data()),
-                                     o.point_cloud.pc.size()));
+    out->set_point_cloud(
+        std::string(reinterpret_cast<const char*>(o.cloud.pc.data()), o.cloud.pc.size()));
     *(out->mutable_geometries()->mutable_geometries()) = impl::to_repeated_field(o.geometries);
 }
 
 Vision::point_cloud_object from_proto(const ::viam::common::v1::PointCloudObject& p) {
     Vision::point_cloud_object out;
     const auto& bytes = p.point_cloud();
-    out.point_cloud.pc.assign(reinterpret_cast<const unsigned char*>(bytes.data()),
-                              reinterpret_cast<const unsigned char*>(bytes.data()) + bytes.size());
+    out.cloud.pc.assign(reinterpret_cast<const unsigned char*>(bytes.data()),
+                        reinterpret_cast<const unsigned char*>(bytes.data()) + bytes.size());
     out.geometries = impl::from_repeated_field(p.geometries().geometries());
     return out;
 }

src/viam/sdk/services/private/vision_client.cpp

@@ -45,13 +45,13 @@ std::vector<Vision::detection> VisionClient::get_detections_from_camera(
         });
 }
 
-std::vector<Vision::detection> VisionClient::get_detections(const Vision::raw_image& image,
+std::vector<Vision::detection> VisionClient::get_detections(const Vision::image& img,
                                                             const ProtoStruct& extra) {
     return make_client_helper(this, *stub_, &service_type::StubInterface::GetDetections)
         .with(extra,
               [&](auto& req) {
-                  req.set_image(std::string(image.bytes.begin(), image.bytes.end()));
-                  req.set_mime_type(image.mime_type);
+                  req.set_image(std::string(img.bytes.begin(), img.bytes.end()));
+                  req.set_mime_type(img.mime_type);
               })
         .invoke([](auto& response) {
             std::vector<Vision::detection> out;
@@ -82,13 +82,14 @@ std::vector<Vision::classification> VisionClient::get_classifications_from_camer
         });
 }
 
-std::vector<Vision::classification> VisionClient::get_classifications(
-    const Vision::raw_image& image, int count, const ProtoStruct& extra) {
+std::vector<Vision::classification> VisionClient::get_classifications(const Vision::image& img,
+                                                                      int count,
+                                                                      const ProtoStruct& extra) {
     return make_client_helper(this, *stub_, &service_type::StubInterface::GetClassifications)
         .with(extra,
               [&](auto& req) {
-                  req.set_image(std::string(image.bytes.begin(), image.bytes.end()));
-                  req.set_mime_type(image.mime_type);
+                  req.set_image(std::string(img.bytes.begin(), img.bytes.end()));
+                  req.set_mime_type(img.mime_type);
                   req.set_n(count);
               })
         .invoke([](auto& response) {
@@ -109,12 +110,15 @@ std::vector<Vision::point_cloud_object> VisionClient::get_object_point_clouds(
                   req.set_camera_name(camera_name);
                   req.set_mime_type(mime_type);
               })
-        .invoke([&mime_type](auto& response) {
+        .invoke([](auto& response) {
             std::vector<Vision::point_cloud_object> out;
             out.reserve(response.objects_size());
+            // The wire format carries one mime_type for all objects; copy it
+            // onto each returned object.  May differ from the requested mime —
+            // see GetObjectPointCloudsResponse.mime_type in vision.proto.
             for (const auto& proto_obj : response.objects()) {
                 auto pco = impl::vision::from_proto(proto_obj);
-                pco.point_cloud.mime_type = mime_type;  // populate from request
+                pco.cloud.mime_type = response.mime_type();
                 out.push_back(std::move(pco));
             }
             return out;

src/viam/sdk/services/private/vision_client.hpp

@@ -36,11 +36,11 @@ class VisionClient : public Vision {
 
     std::vector<Vision::detection> get_detections_from_camera(const std::string& camera_name,
                                                               const ProtoStruct& extra) override;
-    std::vector<Vision::detection> get_detections(const Vision::raw_image& image,
+    std::vector<Vision::detection> get_detections(const Vision::image& img,
                                                   const ProtoStruct& extra) override;
     std::vector<Vision::classification> get_classifications_from_camera(
         const std::string& camera_name, int count, const ProtoStruct& extra) override;
-    std::vector<Vision::classification> get_classifications(const Vision::raw_image& image,
+    std::vector<Vision::classification> get_classifications(const Vision::image& img,
                                                             int count,
                                                             const ProtoStruct& extra) override;
     std::vector<Vision::point_cloud_object> get_object_point_clouds(

src/viam/sdk/services/private/vision_server.cpp

@@ -46,10 +46,10 @@ ::grpc::Status VisionServer::GetDetections(
     ::viam::service::vision::v1::GetDetectionsResponse* response) noexcept {
     return make_service_helper<Vision>(
         "VisionServer::GetDetections", this, context, request)([&](auto& helper, auto& vs) {
-        Vision::raw_image image;
-        image.bytes.assign(request->image().begin(), request->image().end());
-        image.mime_type = request->mime_type();
-        const auto results = vs->get_detections(image, helper.getExtra());
+        Vision::image img;
+        img.bytes.assign(request->image().begin(), request->image().end());
+        img.mime_type = request->mime_type();
+        const auto results = vs->get_detections(img, helper.getExtra());
         for (const auto& d : results) {
             *response->add_detections() = impl::vision::to_proto(d);
         }
@@ -77,10 +77,10 @@ ::grpc::Status VisionServer::GetClassifications(
     ::viam::service::vision::v1::GetClassificationsResponse* response) noexcept {
     return make_service_helper<Vision>(
         "VisionServer::GetClassifications", this, context, request)([&](auto& helper, auto& vs) {
-        Vision::raw_image image;
-        image.bytes.assign(request->image().begin(), request->image().end());
-        image.mime_type = request->mime_type();
-        const auto results = vs->get_classifications(image, request->n(), helper.getExtra());
+        Vision::image img;
+        img.bytes.assign(request->image().begin(), request->image().end());
+        img.mime_type = request->mime_type();
+        const auto results = vs->get_classifications(img, request->n(), helper.getExtra());
         for (const auto& c : results) {
             *response->add_classifications() = impl::vision::to_proto(c);
         }
@@ -98,7 +98,12 @@ ::grpc::Status VisionServer::GetObjectPointClouds(
         for (const auto& obj : results) {
             impl::vision::to_proto(obj, response->add_objects());
         }
-        response->set_mime_type(request->mime_type());
+        // The wire format carries a single mime_type for all objects; take it
+        // from the first result (the server may legitimately convert formats
+        // and return something different from request->mime_type()).
+        if (!results.empty()) {
+            response->set_mime_type(results.front().cloud.mime_type);
+        }
     });
 }
 

src/viam/sdk/services/vision.cpp

@@ -29,25 +29,27 @@ API API::traits<Vision>::api() {
 
 Vision::Vision(std::string name) : Service(std::move(name)) {}
 
-bool operator==(const Vision::detection& a, const Vision::detection& b) {
-    return a.x_min == b.x_min && a.y_min == b.y_min && a.x_max == b.x_max && a.y_max == b.y_max &&
-           a.class_name == b.class_name && a.confidence == b.confidence &&
-           a.x_min_normalized == b.x_min_normalized && a.y_min_normalized == b.y_min_normalized &&
-           a.x_max_normalized == b.x_max_normalized && a.y_max_normalized == b.y_max_normalized;
+bool operator==(const Vision::detection& lhs, const Vision::detection& rhs) {
+    return lhs.x_min == rhs.x_min && lhs.y_min == rhs.y_min && lhs.x_max == rhs.x_max &&
+           lhs.y_max == rhs.y_max && lhs.class_name == rhs.class_name &&
+           lhs.confidence == rhs.confidence && lhs.x_min_normalized == rhs.x_min_normalized &&
+           lhs.y_min_normalized == rhs.y_min_normalized &&
+           lhs.x_max_normalized == rhs.x_max_normalized &&
+           lhs.y_max_normalized == rhs.y_max_normalized;
 }
 
-bool operator==(const Vision::classification& a, const Vision::classification& b) {
-    return a.class_name == b.class_name && a.confidence == b.confidence;
+bool operator==(const Vision::classification& lhs, const Vision::classification& rhs) {
+    return lhs.class_name == rhs.class_name && lhs.confidence == rhs.confidence;
 }
 
-bool operator==(const Vision::point_cloud_object& a, const Vision::point_cloud_object& b) {
-    return a.point_cloud == b.point_cloud && a.geometries == b.geometries;
+bool operator==(const Vision::point_cloud_object& lhs, const Vision::point_cloud_object& rhs) {
+    return lhs.cloud == rhs.cloud && lhs.geometries == rhs.geometries;
 }
 
-bool operator==(const Vision::properties& a, const Vision::properties& b) {
-    return a.classifications_supported == b.classifications_supported &&
-           a.detections_supported == b.detections_supported &&
-           a.object_point_clouds_supported == b.object_point_clouds_supported;
+bool operator==(const Vision::properties& lhs, const Vision::properties& rhs) {
+    return lhs.classifications_supported == rhs.classifications_supported &&
+           lhs.detections_supported == rhs.detections_supported &&
+           lhs.object_point_clouds_supported == rhs.object_point_clouds_supported;
 }
 
 }  // namespace sdk

src/viam/sdk/services/vision.hpp

@@ -28,8 +28,6 @@ class Vision : public Service {
     /// @struct detection
     /// @brief Result of a single object detection.
     /// Pixel bbox fields are optional int64; normalized bbox fields are optional double.
-    /// @note The field name `point_cloud` in `point_cloud_object` shadows the type alias
-    /// `point_cloud` — this is intentional and matches the proto field layout.
     struct detection {
         boost::optional<std::int64_t> x_min;
         boost::optional<std::int64_t> y_min;
@@ -40,34 +38,47 @@ class Vision : public Service {
         boost::optional<double> x_max_normalized;
         boost::optional<double> y_max_normalized;
         std::string class_name;
-        double confidence;
+        double confidence = 0.0;
     };
 
     /// @struct classification
     /// @brief Result of a single image classification.
     struct classification {
         std::string class_name;
-        double confidence;
+        double confidence = 0.0;
     };
 
     using point_cloud = Camera::point_cloud;
     using raw_image = Camera::raw_image;
 
+    /// @struct image
+    /// @brief Image input to `get_detections` / `get_classifications`.
+    ///
+    /// Distinct from `raw_image` (a.k.a. `Camera::raw_image`) because the Vision request
+    /// messages on the wire only carry bytes + mime_type — they do not carry a `source_name`.
+    /// See the proto definitions for `GetDetectionsRequest` and `GetClassificationsRequest`.
+    /// The `raw_image` alias remains for `capture_all_result.image`, whose wire type is the
+    /// full Camera Image proto and does carry a source name.
+    struct image {
+        std::string mime_type;
+        std::vector<unsigned char> bytes;
+    };
+
     /// @struct point_cloud_object
     /// @brief A point cloud and its associated geometry.
-    /// @note The field name `point_cloud` shadows the type alias `point_cloud` — intentional,
-    /// matching the proto field name.
+    /// @note The proto field is named `point_cloud`; we use `cloud` here to avoid shadowing
+    /// the `point_cloud` type alias (GCC rejects the shadowing under -fpermissive).
     struct point_cloud_object {
-        point_cloud point_cloud;
+        point_cloud cloud;
         std::vector<GeometryConfig> geometries;
     };
 
     /// @struct properties
     /// @brief Describes the vision service's supported capabilities.
     struct properties {
-        bool classifications_supported;
-        bool detections_supported;
-        bool object_point_clouds_supported;
+        bool classifications_supported = false;
+        bool detections_supported = false;
+        bool object_point_clouds_supported = false;
     };
 
     /// @struct capture_options
@@ -99,15 +110,14 @@ class Vision : public Service {
     virtual std::vector<detection> get_detections_from_camera(const std::string& camera_name,
                                                               const ProtoStruct& extra) = 0;
 
-    /// @brief Get detections from a raw image.
-    inline std::vector<detection> get_detections(const raw_image& image) {
-        return get_detections(image, {});
+    /// @brief Get detections from an image.
+    inline std::vector<detection> get_detections(const image& img) {
+        return get_detections(img, {});
     }
 
-    /// @brief Get detections from a raw image.
+    /// @brief Get detections from an image.
     /// @param extra Any additional arguments to the method.
-    virtual std::vector<detection> get_detections(const raw_image& image,
-                                                  const ProtoStruct& extra) = 0;
+    virtual std::vector<detection> get_detections(const image& img, const ProtoStruct& extra) = 0;
 
     /// @brief Get classifications from a named camera's next image.
     /// @param count The number of classifications to return.
@@ -122,16 +132,16 @@ class Vision : public Service {
     virtual std::vector<classification> get_classifications_from_camera(
         const std::string& camera_name, int count, const ProtoStruct& extra) = 0;
 
-    /// @brief Get classifications from a raw image.
+    /// @brief Get classifications from an image.
     /// @param count The number of classifications to return.
-    inline std::vector<classification> get_classifications(const raw_image& image, int count) {
-        return get_classifications(image, count, {});
+    inline std::vector<classification> get_classifications(const image& img, int count) {
+        return get_classifications(img, count, {});
     }
 
-    /// @brief Get classifications from a raw image.
+    /// @brief Get classifications from an image.
     /// @param count The number of classifications to return.
     /// @param extra Any additional arguments to the method.
-    virtual std::vector<classification> get_classifications(const raw_image& image,
+    virtual std::vector<classification> get_classifications(const image& img,
                                                             int count,
                                                             const ProtoStruct& extra) = 0;
 

src/viam/sdk/tests/mocks/mock_vision.cpp

@@ -21,13 +21,13 @@ std::vector<Vision::detection> MockVision::get_detections_from_camera(
     return canned_detections;
 }
 
-std::vector<Vision::detection> MockVision::get_detections(const raw_image& image,
+std::vector<Vision::detection> MockVision::get_detections(const Vision::image& img,
                                                           const ProtoStruct& extra) {
     if (throw_on_next_call) {
         throw_on_next_call = false;
         throw sdk::Exception("mock failure");
     }
-    last_image = image;
+    last_image = img;
     last_extra = extra;
     return canned_detections;
 }
@@ -44,14 +44,14 @@ std::vector<Vision::classification> MockVision::get_classifications_from_camera(
     return canned_classifications;
 }
 
-std::vector<Vision::classification> MockVision::get_classifications(const raw_image& image,
+std::vector<Vision::classification> MockVision::get_classifications(const Vision::image& img,
                                                                     int count,
                                                                     const ProtoStruct& extra) {
     if (throw_on_next_call) {
         throw_on_next_call = false;
         throw sdk::Exception("mock failure");
     }
-    last_image = image;
+    last_image = img;
     last_count = count;
     last_extra = extra;
     return canned_classifications;

src/viam/sdk/tests/mocks/mock_vision.hpp

@@ -25,7 +25,7 @@ class MockVision : public sdk::Vision {
     std::string last_camera_name;
     int last_count = 0;
     std::string last_mime_type;
-    sdk::Vision::raw_image last_image;
+    sdk::Vision::image last_image;
     sdk::Vision::capture_options last_capture_options;
     sdk::ProtoStruct last_extra;
     sdk::ProtoStruct last_command;
@@ -35,12 +35,12 @@ class MockVision : public sdk::Vision {
 
     std::vector<sdk::Vision::detection> get_detections_from_camera(
         const std::string& camera_name, const sdk::ProtoStruct& extra) override;
-    std::vector<sdk::Vision::detection> get_detections(const sdk::Vision::raw_image& image,
+    std::vector<sdk::Vision::detection> get_detections(const sdk::Vision::image& img,
                                                        const sdk::ProtoStruct& extra) override;
     std::vector<sdk::Vision::classification> get_classifications_from_camera(
         const std::string& camera_name, int count, const sdk::ProtoStruct& extra) override;
     std::vector<sdk::Vision::classification> get_classifications(
-        const sdk::Vision::raw_image& image, int count, const sdk::ProtoStruct& extra) override;
+        const sdk::Vision::image& img, int count, const sdk::ProtoStruct& extra) override;
     std::vector<sdk::Vision::point_cloud_object> get_object_point_clouds(
         const std::string& camera_name,
         const std::string& mime_type,