From 4f555297a21a9e3f6b5cbab02d533fae2a6ca049 Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Tue, 21 Aug 2018 17:53:15 +0200 Subject: [PATCH 1/3] camera: add setting description to SettingOptions This was previously missing. --- backend/proto | 2 +- .../src/plugins/camera/camera_service_impl.h | 2 ++ backend/test/camera_service_impl_test.cpp | 20 ++++++++++++------- integration_tests/camera_settings.cpp | 8 ++++++++ plugins/camera/camera.cpp | 2 +- plugins/camera/camera_impl.cpp | 1 + .../camera/include/plugins/camera/camera.h | 1 + 7 files changed, 27 insertions(+), 9 deletions(-) diff --git a/backend/proto b/backend/proto index cb77f56709..d108a9d346 160000 --- a/backend/proto +++ b/backend/proto @@ -1 +1 @@ -Subproject commit cb77f567098f7e1c0fdcac7ceda2e53817b41f48 +Subproject commit d108a9d346fa2ea7086d001c406e9cedb087f8ac diff --git a/backend/src/plugins/camera/camera_service_impl.h b/backend/src/plugins/camera/camera_service_impl.h index 5944aaa308..e41eda0dbd 100644 --- a/backend/src/plugins/camera/camera_service_impl.h +++ b/backend/src/plugins/camera/camera_service_impl.h @@ -608,6 +608,7 @@ class CameraServiceImpl final : public rpc::camera::CameraService::Service { rpc::camera::SettingOptions *rpc_setting_options) { rpc_setting_options->set_setting_id(setting_options.setting_id); + rpc_setting_options->set_setting_description(setting_options.setting_description); for (const auto option : setting_options.options) { auto rpc_option = rpc_setting_options->add_options(); @@ -620,6 +621,7 @@ class CameraServiceImpl final : public rpc::camera::CameraService::Service { { dronecode_sdk::Camera::SettingOptions setting_options; setting_options.setting_id = rpc_setting_options.setting_id(); + setting_options.setting_description = rpc_setting_options.setting_description(); std::vector options; for (auto option : rpc_setting_options.options()) { diff --git a/backend/test/camera_service_impl_test.cpp b/backend/test/camera_service_impl_test.cpp index e8dd30471b..cf792f3c9a 100644 --- a/backend/test/camera_service_impl_test.cpp +++ b/backend/test/camera_service_impl_test.cpp @@ -119,6 +119,7 @@ class CameraServiceImplTest : public ::testing::TestWithParam { dronecode_sdk::Camera::SettingOptions createSettingOptions(const std::string setting_id, + const std::string setting_description, const std::vector &options) const; std::future subscribePossibleSettingOptionsAsync( std::vector> &possible_settings_events, @@ -1056,7 +1057,8 @@ TEST_F(CameraServiceImplTest, registersToPossibleSettings) std::vector possible_settings; std::vector options; options.push_back(createOption(ARBITRARY_OPTION_ID)); - possible_settings.push_back(createSettingOptions(ARBITRARY_SETTING_ID, options)); + possible_settings.push_back( + createSettingOptions(ARBITRARY_SETTING_ID, ARBITRARY_SETTING_DESCRIPTION, options)); dronecode_sdk::Camera::subscribe_possible_setting_options_callback_t possible_settings_callback; EXPECT_CALL(_camera, subscribe_possible_setting_options(_)) .WillOnce(SaveResult(&possible_settings_callback, &_callback_saved_promise)); @@ -1072,10 +1074,13 @@ TEST_F(CameraServiceImplTest, registersToPossibleSettings) } dronecode_sdk::Camera::SettingOptions CameraServiceImplTest::createSettingOptions( - const std::string setting_id, const std::vector &options) const + const std::string setting_id, + const std::string setting_description, + const std::vector &options) const { dronecode_sdk::Camera::SettingOptions setting_options; setting_options.setting_id = setting_id; + setting_options.setting_description = setting_description; setting_options.options = options; return setting_options; @@ -1125,7 +1130,8 @@ TEST_F(CameraServiceImplTest, sendsOnePossibleSettingOptions) std::vector possible_setting_options; std::vector options; options.push_back(createOption(ARBITRARY_OPTION_ID)); - possible_setting_options.push_back(createSettingOptions(ARBITRARY_SETTING_ID, options)); + possible_setting_options.push_back( + createSettingOptions(ARBITRARY_SETTING_ID, ARBITRARY_SETTING_DESCRIPTION, options)); possible_setting_options_events.push_back(possible_setting_options); checkSendsPossibleSettingOptions(possible_setting_options_events); @@ -1157,7 +1163,7 @@ void CameraServiceImplTest::checkSendsPossibleSettingOptions( std::vector options; options.push_back(createOption(ARBITRARY_OPTION_ID)); arbitrary_possible_setting_options.push_back( - createSettingOptions(ARBITRARY_SETTING_ID, options)); + createSettingOptions(ARBITRARY_SETTING_ID, ARBITRARY_SETTING_DESCRIPTION, options)); possible_setting_options_callback(arbitrary_possible_setting_options); possible_setting_options_events_future.wait(); @@ -1183,11 +1189,11 @@ TEST_F(CameraServiceImplTest, sendsMultiplePossibleSettingOptionss) options1.push_back(createOption("option1_1")); options1.push_back(createOption("option1_2")); options1.push_back(createOption("option1_3")); - possible_setting_options.push_back(createSettingOptions("setting1", options1)); + possible_setting_options.push_back(createSettingOptions("setting1", "Setting one", options1)); std::vector options2; options2.push_back(createOption("option1")); options2.push_back(createOption("option2")); - possible_setting_options.push_back(createSettingOptions("setting2", options2)); + possible_setting_options.push_back(createSettingOptions("setting2", "Setting two", options2)); std::vector options3; options3.push_back(createOption("option1")); @@ -1195,7 +1201,7 @@ TEST_F(CameraServiceImplTest, sendsMultiplePossibleSettingOptionss) options3.push_back(createOption("option2")); options3.push_back(createOption("option1")); options3.push_back(createOption("option1")); - possible_setting_options.push_back(createSettingOptions("setting3", options3)); + possible_setting_options.push_back(createSettingOptions("setting3", "Setting Three", options3)); possible_setting_options_events.push_back(possible_setting_options); diff --git a/integration_tests/camera_settings.cpp b/integration_tests/camera_settings.cpp index d398c60eb5..d35dd2c268 100644 --- a/integration_tests/camera_settings.cpp +++ b/integration_tests/camera_settings.cpp @@ -285,6 +285,14 @@ receive_possible_setting_options(bool &subscription_called, EXPECT_TRUE(settings_options.size() > 0); for (auto &setting_options : settings_options) { LogDebug() << "Got setting '" << setting_options.setting_id << "' with options:"; + + // Check human readable strings too. + if (setting_options.setting_id == "CAM_SHUTTERSPD") { + EXPECT_STREQ(setting_options.setting_description.c_str(), "Shutter Speed"); + } else if (setting_options.setting_id == "CAM_EXPMODE") { + EXPECT_STREQ(setting_options.setting_description.c_str(), "Exposure Mode"); + } + EXPECT_TRUE(setting_options.options.size() > 0); for (auto &option : setting_options.options) { LogDebug() << " - '" << option.option_description << "'"; diff --git a/plugins/camera/camera.cpp b/plugins/camera/camera.cpp index 28c5909858..603511643c 100644 --- a/plugins/camera/camera.cpp +++ b/plugins/camera/camera.cpp @@ -345,7 +345,7 @@ bool operator==(const Camera::SettingOptions &lhs, const Camera::SettingOptions } } - return lhs.setting_id == rhs.setting_id; + return lhs.setting_id == rhs.setting_id && lhs.setting_description == rhs.setting_description; } std::ostream &operator<<(std::ostream &str, Camera::SettingOptions const &setting_options) diff --git a/plugins/camera/camera_impl.cpp b/plugins/camera/camera_impl.cpp index 0c05c9c52b..6ba6ffcbc0 100644 --- a/plugins/camera/camera_impl.cpp +++ b/plugins/camera/camera_impl.cpp @@ -1228,6 +1228,7 @@ void CameraImpl::notify_possible_setting_options() for (auto &possible_setting : possible_settings) { Camera::SettingOptions setting_options; setting_options.setting_id = possible_setting; + get_setting_str(setting_options.setting_id, setting_options.setting_description); get_possible_options(possible_setting, setting_options.options); possible_setting_options.push_back(setting_options); } diff --git a/plugins/camera/include/plugins/camera/camera.h b/plugins/camera/include/plugins/camera/camera.h index 89d7a09dec..d0916eace9 100644 --- a/plugins/camera/include/plugins/camera/camera.h +++ b/plugins/camera/include/plugins/camera/camera.h @@ -449,6 +449,7 @@ class Camera : public PluginBase { */ struct SettingOptions { std::string setting_id; /**< Name of the setting (machine readable). */ + std::string setting_description; /**< Description of the setting (human readable). */ std::vector