diff --git a/BUILDING.md b/BUILDING.md index bf7d26613..c2a604f4f 100644 --- a/BUILDING.md +++ b/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 diff --git a/src/viam/sdk/components/camera.hpp b/src/viam/sdk/components/camera.hpp index 5f0b2cd71..6e7c96f48 100644 --- a/src/viam/sdk/components/camera.hpp +++ b/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 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 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). diff --git a/src/viam/sdk/components/private/camera_client.cpp b/src/viam/sdk/components/private/camera_client.cpp index ec09856fc..7fe00a6f3 100644 --- a/src/viam/sdk/components/private/camera_client.cpp +++ b/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 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 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) { diff --git a/src/viam/sdk/components/private/camera_client.hpp b/src/viam/sdk/components/private/camera_client.hpp index 75bbaa72a..c3964fcb6 100644 --- a/src/viam/sdk/components/private/camera_client.hpp +++ b/src/viam/sdk/components/private/camera_client.hpp @@ -26,7 +26,8 @@ class CameraClient : public Camera { CameraClient(std::string name, std::shared_ptr 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 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 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: diff --git a/src/viam/sdk/components/private/camera_server.cpp b/src/viam/sdk/components/private/camera_server.cpp index 2ef91da21..de599b49c 100644 --- a/src/viam/sdk/components/private/camera_server.cpp +++ b/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( - "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); diff --git a/src/viam/sdk/tests/mocks/camera_mocks.cpp b/src/viam/sdk/tests/mocks/camera_mocks.cpp index 1840f6a0f..73a44ce83 100644 --- a/src/viam/sdk/tests/mocks/camera_mocks.cpp +++ b/src/viam/sdk/tests/mocks/camera_mocks.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -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 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 images; Camera::raw_image image1; image1.mime_type = "image/jpeg"; - image1.source_name = "color_sensor"; + image1.source_name = "color"; std::vector 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 bytes2 = {'d', 'e', 'f'}; image2.bytes = bytes2; images.push_back(image2); diff --git a/src/viam/sdk/tests/mocks/camera_mocks.hpp b/src/viam/sdk/tests/mocks/camera_mocks.hpp index 1383d293e..c7aeb835c 100644 --- a/src/viam/sdk/tests/mocks/camera_mocks.hpp +++ b/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 filter_source_names, + const sdk::ProtoStruct& extra) override; point_cloud get_point_cloud(std::string mime_type, const sdk::ProtoStruct& extra) override; std::vector get_geometries(const sdk::ProtoStruct& extra) override; properties get_properties() override; static std::shared_ptr get_mock_camera(); MockCamera(std::string name) : Camera(std::move(name)) {} + const std::vector& 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 geometries_; + std::vector last_filter_source_names_; + ProtoStruct last_extra_; }; Camera::raw_image fake_raw_image(); diff --git a/src/viam/sdk/tests/test_camera.cpp b/src/viam/sdk/tests/test_camera.cpp index 2f1e1531e..f0fec55f5 100644 --- a/src/viam/sdk/tests/test_camera.cpp +++ b/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 mock = MockCamera::get_mock_camera(); + client_to_mock_pipeline(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 mock = MockCamera::get_mock_camera(); + client_to_mock_pipeline(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()); + BOOST_CHECK_EQUAL(last_extra.at("foo").get_unchecked(), "bar"); + }); +} + BOOST_AUTO_TEST_CASE(test_get_point_cloud) { std::shared_ptr mock = MockCamera::get_mock_camera(); client_to_mock_pipeline(mock, [](Camera& client) {