[RSDK-11703, RSDK-11602] Update C++ GetImages signature (viamrobotics#481)

Author: hexbabe

BUILDING.md

@@ -441,7 +441,7 @@ that they are pre-populated.
 cmake -DVIAMCPPSDK_USE_DYNAMIC_PROTOS=ON ...
 ninja all   # FAILS!
 ninja generate-dynamic-protos
-nina all    # OK!
+ninja all    # OK!
 ```
 
 ### Build Issue: Proto Mismatch

src/viam/sdk/components/camera.hpp

@@ -160,7 +160,27 @@ class Camera : public Component {
     /// @brief Get the next images from the camera as a vector of raw images with names and
     /// metadata.
     /// @return a vector of raw_images and associated response metadata.
-    virtual image_collection get_images() = 0;
+    inline image_collection get_images() {
+        return get_images({}, {});
+    }
+
+    /// @brief Get the next images from specific sources as a vector of raw images with names and
+    /// metadata.
+    /// @param filter_source_names the names of sources to receive images from. If empty, all
+    /// sources are returned.
+    /// @return a vector of raw_images and associated response metadata.
+    inline image_collection get_images(std::vector<std::string> filter_source_names) {
+        return get_images(std::move(filter_source_names), {});
+    }
+
+    /// @brief Get the next images from the camera as a vector of raw images with names and
+    /// metadata.
+    /// @param filter_source_names the names of sources to receive images from. If empty, all
+    /// sources are returned.
+    /// @param extra any additional arguments to the method.
+    /// @return a vector of raw_images and associated response metadata.
+    virtual image_collection get_images(std::vector<std::string> filter_source_names,
+                                        const ProtoStruct& extra) = 0;
 
     /// @brief Get the next `point_cloud` from the camera.
     /// @param mime_type the desired mime_type of the point_cloud (does not guarantee output type).

src/viam/sdk/components/private/camera_client.cpp

@@ -54,7 +54,13 @@ Camera::image_collection from_proto(const viam::component::camera::v1::GetImages
         std::string img_string = img.image();
         const std::vector<unsigned char> bytes(img_string.begin(), img_string.end());
         raw_image.bytes = bytes;
-        raw_image.mime_type = format_to_MIME_string(img.format());
+        // TODO(RSDK-11733): This is a temporary fix to support handling both the format and mime
+        // type. We will remove this once we remove the format field from the proto.
+        if (!img.mime_type().empty()) {
+            raw_image.mime_type = img.mime_type();
+        } else {
+            raw_image.mime_type = format_to_MIME_string(img.format());
+        }
         raw_image.source_name = img.source_name();
         images.push_back(raw_image);
     }
@@ -122,10 +128,21 @@ Camera::raw_image CameraClient::get_image(std::string mime_type, const ProtoStru
         .invoke([](auto& response) { return from_proto(response); });
 };
 
-Camera::image_collection CameraClient::get_images() {
-    return make_client_helper(this, *stub_, &StubType::GetImages).invoke([](auto& response) {
-        return from_proto(response);
-    });
+Camera::image_collection CameraClient::get_images(std::vector<std::string> filter_source_names,
+                                                  const ProtoStruct& extra) {
+    return make_client_helper(this, *stub_, &StubType::GetImages)
+        .with(extra,
+              [&](auto& request) {
+                  if (!filter_source_names.empty()) {
+                      // in newer gRPC versions we would be able to call `Add` or `Assign` on an
+                      // iterator range rather than element-wise copy
+                      request.mutable_filter_source_names()->Reserve(filter_source_names.size());
+                      for (auto& source_name : filter_source_names) {
+                          request.add_filter_source_names(std::move(source_name));
+                      }
+                  }
+              })
+        .invoke([](auto& response) { return from_proto(response); });
 };
 
 Camera::point_cloud CameraClient::get_point_cloud(std::string mime_type, const ProtoStruct& extra) {

src/viam/sdk/components/private/camera_client.hpp

@@ -26,7 +26,8 @@ class CameraClient : public Camera {
     CameraClient(std::string name, std::shared_ptr<grpc::Channel> channel);
     ProtoStruct do_command(const ProtoStruct& command) override;
     raw_image get_image(std::string mime_type, const ProtoStruct& extra) override;
-    image_collection get_images() override;
+    image_collection get_images(std::vector<std::string> filter_source_names,
+                                const ProtoStruct& extra) override;
     point_cloud get_point_cloud(std::string mime_type, const ProtoStruct& extra) override;
     properties get_properties() override;
     std::vector<GeometryConfig> get_geometries(const ProtoStruct& extra) override;
@@ -42,6 +43,7 @@ class CameraClient : public Camera {
     // we need to include these `using` lines.
     using Camera::get_geometries;
     using Camera::get_image;
+    using Camera::get_images;
     using Camera::get_point_cloud;
 
    protected:

src/viam/sdk/components/private/camera_server.cpp

@@ -83,12 +83,15 @@ ::grpc::Status CameraServer::GetImages(
     const ::viam::component::camera::v1::GetImagesRequest* request,
     ::viam::component::camera::v1::GetImagesResponse* response) noexcept {
     return make_service_helper<Camera>(
-        "CameraServer::GetImages", this, request)([&](auto&, auto& camera) {
-        const Camera::image_collection image_coll = camera->get_images();
+        "CameraServer::GetImages", this, request)([&](auto& helper, auto& camera) {
+        const Camera::image_collection image_coll = camera->get_images(
+            {request->filter_source_names().begin(), request->filter_source_names().end()},
+            helper.getExtra());
         for (const auto& img : image_coll.images) {
             ::viam::component::camera::v1::Image proto_image;
             const std::string img_string = bytes_to_string(img.bytes);
             proto_image.set_source_name(img.source_name);
+            proto_image.set_mime_type(img.mime_type);
             proto_image.set_format(
                 MIME_string_to_format(Camera::normalize_mime_type(img.mime_type)));
             proto_image.set_image(img_string);

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

@@ -1,3 +1,4 @@
+#include <algorithm>
 #include <viam/sdk/tests/mocks/camera_mocks.hpp>
 
 #include <viam/sdk/common/proto_value.hpp>
@@ -16,8 +17,23 @@ ProtoStruct MockCamera::do_command(const ProtoStruct&) {
 Camera::raw_image MockCamera::get_image(std::string, const ProtoStruct&) {
     return image_;
 }
-Camera::image_collection MockCamera::get_images() {
-    return images_;
+Camera::image_collection MockCamera::get_images(std::vector<std::string> filter_source_names,
+                                                const ProtoStruct& extra) {
+    last_filter_source_names_ = std::move(filter_source_names);
+    last_extra_ = extra;
+    if (last_filter_source_names_.empty()) {
+        return images_;
+    }
+    Camera::image_collection filtered = images_;
+    filtered.images.clear();
+    for (const auto& img : images_.images) {
+        if (std::find(last_filter_source_names_.begin(),
+                      last_filter_source_names_.end(),
+                      img.source_name) != last_filter_source_names_.end()) {
+            filtered.images.push_back(img);
+        }
+    }
+    return filtered;
 }
 Camera::point_cloud MockCamera::get_point_cloud(std::string, const ProtoStruct&) {
     return pc_;
@@ -43,13 +59,13 @@ Camera::image_collection fake_raw_images() {
     std::vector<Camera::raw_image> images;
     Camera::raw_image image1;
     image1.mime_type = "image/jpeg";
-    image1.source_name = "color_sensor";
+    image1.source_name = "color";
     std::vector<unsigned char> bytes1 = {'a', 'b', 'c'};
     image1.bytes = bytes1;
     images.push_back(image1);
     Camera::raw_image image2;
     image2.mime_type = "image/vnd.viam.dep";
-    image2.source_name = "depth_sensor";
+    image2.source_name = "depth";
     std::vector<unsigned char> bytes2 = {'d', 'e', 'f'};
     image2.bytes = bytes2;
     images.push_back(image2);

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

@@ -12,13 +12,21 @@ class MockCamera : public Camera {
    public:
     ProtoStruct do_command(const ProtoStruct& command) override;
     raw_image get_image(std::string mime_type, const sdk::ProtoStruct& extra) override;
-    image_collection get_images() override;
+    image_collection get_images(std::vector<std::string> filter_source_names,
+                                const sdk::ProtoStruct& extra) override;
     point_cloud get_point_cloud(std::string mime_type, const sdk::ProtoStruct& extra) override;
     std::vector<GeometryConfig> get_geometries(const sdk::ProtoStruct& extra) override;
     properties get_properties() override;
     static std::shared_ptr<MockCamera> get_mock_camera();
     MockCamera(std::string name) : Camera(std::move(name)) {}
 
+    const std::vector<std::string>& last_filter_source_names() const {
+        return last_filter_source_names_;
+    }
+    const ProtoStruct& last_extra() const {
+        return last_extra_;
+    }
+
    private:
     Camera::intrinsic_parameters intrinsic_parameters_;
     Camera::distortion_parameters distortion_parameters_;
@@ -28,6 +36,8 @@ class MockCamera : public Camera {
     Camera::point_cloud pc_;
     ProtoStruct map_;
     std::vector<GeometryConfig> geometries_;
+    std::vector<std::string> last_filter_source_names_;
+    ProtoStruct last_extra_;
 };
 
 Camera::raw_image fake_raw_image();

src/viam/sdk/tests/test_camera.cpp

@@ -51,6 +51,37 @@ BOOST_AUTO_TEST_CASE(test_get_images) {
     });
 }
 
+BOOST_AUTO_TEST_CASE(test_get_images_filtering) {
+    std::shared_ptr<MockCamera> mock = MockCamera::get_mock_camera();
+    client_to_mock_pipeline<Camera>(mock, [&](Camera& client) {
+        // request only color
+        Camera::image_collection images = client.get_images({"color"});
+        BOOST_CHECK_EQUAL(images.images.size(), 1);
+        BOOST_CHECK_EQUAL(images.images[0].source_name, "color");
+
+        // empty filter returns all
+        Camera::image_collection all_images = client.get_images({});
+        BOOST_CHECK_EQUAL(all_images.images.size(), 2);
+
+        // verify filter propagated to mock
+        auto last_filter = mock->last_filter_source_names();
+        BOOST_CHECK_EQUAL(last_filter.size(), 0);
+    });
+}
+
+BOOST_AUTO_TEST_CASE(test_get_images_with_extra) {
+    std::shared_ptr<MockCamera> mock = MockCamera::get_mock_camera();
+    client_to_mock_pipeline<Camera>(mock, [&](Camera& client) {
+        ProtoStruct extra;
+        extra["foo"] = ProtoValue("bar");
+        auto images = client.get_images({}, extra);
+        (void)images;  // unused variable in test body
+        const auto& last_extra = mock->last_extra();
+        BOOST_CHECK(last_extra.at("foo").is_a<std::string>());
+        BOOST_CHECK_EQUAL(last_extra.at("foo").get_unchecked<std::string>(), "bar");
+    });
+}
+
 BOOST_AUTO_TEST_CASE(test_get_point_cloud) {
     std::shared_ptr<MockCamera> mock = MockCamera::get_mock_camera();
     client_to_mock_pipeline<Camera>(mock, [](Camera& client) {