From 993355757b9b3bc3dba3e5065435db44fed099c6 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 1 Oct 2024 12:39:20 -0400 Subject: [PATCH 1/9] add debug log support --- .../dial_api_key/example_dial_api_key.cpp | 1 - src/viam/sdk/common/client_helper.hpp | 12 +++++ src/viam/sdk/common/private/utils.hpp | 11 +++++ src/viam/sdk/common/utils.cpp | 48 +++++++++++++++++++ src/viam/sdk/common/utils.hpp | 19 ++++++++ src/viam/sdk/components/arm.hpp | 5 ++ .../sdk/components/private/motor_client.cpp | 4 +- 7 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 src/viam/sdk/common/private/utils.hpp diff --git a/src/viam/examples/dial_api_key/example_dial_api_key.cpp b/src/viam/examples/dial_api_key/example_dial_api_key.cpp index 5c0a4f687..d0a5b7dac 100644 --- a/src/viam/examples/dial_api_key/example_dial_api_key.cpp +++ b/src/viam/examples/dial_api_key/example_dial_api_key.cpp @@ -22,7 +22,6 @@ #include #include -using viam::robot::v1::Status; using namespace viam::sdk; namespace po = boost::program_options; diff --git a/src/viam/sdk/common/client_helper.hpp b/src/viam/sdk/common/client_helper.hpp index 962648fd1..691acefb5 100644 --- a/src/viam/sdk/common/client_helper.hpp +++ b/src/viam/sdk/common/client_helper.hpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -53,6 +54,13 @@ class ClientHelper { template ClientHelper& with(const AttributeMap& extra, RequestSetupCallable&& rsc) { + if (extra) { + auto key = extra->find(impl::debug_map_key); + if (key != extra->end()) { + ProtoType value = *(key->second); + debug_key_ = *value.get(); + } + } *request_.mutable_extra() = map_to_struct(extra); return with(std::forward(rsc)); } @@ -67,6 +75,9 @@ class ClientHelper { *request_.mutable_name() = client_->name(); ClientContext ctx; + if (debug_key_ != "") { + ctx.set_debug_key(debug_key_); + } const auto result = (stub_->*pfn_)(ctx, request_, &response_); if (result.ok()) { return std::forward(rhc)( @@ -112,6 +123,7 @@ class ClientHelper { private: ClientType* client_; StubType* stub_; + std::string debug_key_; MethodType pfn_; RequestType request_; ResponseType response_; diff --git a/src/viam/sdk/common/private/utils.hpp b/src/viam/sdk/common/private/utils.hpp new file mode 100644 index 000000000..10f6f9c1c --- /dev/null +++ b/src/viam/sdk/common/private/utils.hpp @@ -0,0 +1,11 @@ +#pragma once + +namespace viam { +namespace sdk { +namespace impl { + +const char debug_map_key[] = "com.viam.debug_key_internal"; + +} // namespace impl +} // namespace sdk +} // namespace viam diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index 90ebd56a6..aa5fc24c4 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -14,6 +14,7 @@ #include +#include #include #include #include @@ -108,6 +109,53 @@ void ClientContext::set_client_ctx_authority_() { wrapped_context_.set_authority("viam-placeholder"); } +std::string random_debug_key() { + static const char alphanum[] = "abcdefghijklmnopqrstuvwxyz"; + static std::default_random_engine generator( + std::chrono::system_clock::now().time_since_epoch().count()); + std::uniform_int_distribution distribution(0, sizeof(alphanum) - 1); + + std::string key; + key.reserve(6); + + for (int i = 0; i < 6; ++i) { + key += alphanum[distribution(generator)]; + }; + + return key; +} + +AttributeMap debug_map() { + auto debug_key = random_debug_key(); + return debug_map(debug_key); +} + +AttributeMap debug_map(std::string debug_key) { + AttributeMap map = + std::make_shared>>(); + map->emplace(impl::debug_map_key, std::make_shared(std::move(debug_key))); + + return map; +} + +AttributeMap add_debug_entry(AttributeMap map, std::string debug_key) { + map->emplace(impl::debug_map_key, std::make_shared(std::move(debug_key))); + return map; +} + +AttributeMap add_debug_entry(AttributeMap&& map, std::string debug_key) { + return add_debug_entry(map, std::move(debug_key)); +} + +AttributeMap add_debug_entry(AttributeMap&& map) { + auto debug_key = random_debug_key(); + return add_debug_entry(map, std::move(debug_key)); +} + +void ClientContext::set_debug_key(const std::string& debug_key) { + wrapped_context_.AddMetadata("dtname", debug_key); +} + void ClientContext::add_viam_client_version_() { wrapped_context_.AddMetadata("viam_client", impl::k_version); } diff --git a/src/viam/sdk/common/utils.hpp b/src/viam/sdk/common/utils.hpp index 893fcacd1..2c7ae9224 100644 --- a/src/viam/sdk/common/utils.hpp +++ b/src/viam/sdk/common/utils.hpp @@ -58,6 +58,7 @@ class ClientContext { ClientContext(); operator grpc::ClientContext*(); operator const grpc::ClientContext*() const; + void set_debug_key(const std::string& debug_key); private: void set_client_ctx_authority_(); @@ -65,6 +66,24 @@ class ClientContext { grpc::ClientContext wrapped_context_; }; +/// @brief Returns a new `AttributeMap` with a random key for server-side debug logging +AttributeMap debug_map(); + +/// @brief Returns a new `AttributeMap` with @param debug_key for server-side debug logging +/// @throws Exception if the debug_key contains invalid (e.g., uppercase) gRPC characters +AttributeMap debug_map(std::string debug_key); + +/// @brief Adds @param debug_key for server-side debug logging to @param map +/// @throws Exception if the debug_key contains invalid (e.g., uppercase )gRPC characters +AttributeMap add_debug_entry(AttributeMap&& map, std::string debug_key); + +/// @brief Adds @param debug_key for server-side debug logging to @param map +/// @throws Exception if the debug_key contains invalid (e.g., uppercase )gRPC characters +AttributeMap add_debug_entry(AttributeMap map, std::string debug_key); + +/// @brief Adds a random key to @param map for server-side debug logging +AttributeMap add_debug_entry(AttributeMap&& map); + /// @brief Set the boost trivial logger's severity depending on args. /// @param argc The number of args. /// @param argv The commandline arguments to parse. diff --git a/src/viam/sdk/components/arm.hpp b/src/viam/sdk/components/arm.hpp index 03faf7183..fda93d068 100644 --- a/src/viam/sdk/components/arm.hpp +++ b/src/viam/sdk/components/arm.hpp @@ -93,6 +93,11 @@ class Arm : public Component, public Stoppable { /// @param extra Any additional arguments to the method. virtual std::vector get_joint_positions(const AttributeMap& extra) = 0; + /// @brief Move each joint on the arm to the corresponding angle specified in @param positions + inline void move_to_joint_positions(const std::vector& positions) { + return move_to_joint_positions(positions, {}); + } + /// @brief Move each joint on the arm to the corresponding angle specified in @param positions /// @param extra Any additional arguments to the method. virtual void move_to_joint_positions(const std::vector& positions, diff --git a/src/viam/sdk/components/private/motor_client.cpp b/src/viam/sdk/components/private/motor_client.cpp index 8a1bb3fb6..508f7268f 100644 --- a/src/viam/sdk/components/private/motor_client.cpp +++ b/src/viam/sdk/components/private/motor_client.cpp @@ -1,8 +1,6 @@ #include -#include #include -#include #include #include @@ -21,7 +19,7 @@ namespace impl { MotorClient::MotorClient(std::string name, std::shared_ptr channel) : Motor(std::move(name)), stub_(viam::component::motor::v1::MotorService::NewStub(channel)), - channel_(std::move(channel)){}; + channel_(std::move(channel)) {}; void MotorClient::set_power(double power_pct, const AttributeMap& extra) { return make_client_helper(this, *stub_, &StubType::SetPower) From 838a3a0675803355509ca18bf466df2643ec6b57 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 1 Oct 2024 12:52:13 -0400 Subject: [PATCH 2/9] clang-format fix --- src/viam/sdk/components/private/motor_client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/components/private/motor_client.cpp b/src/viam/sdk/components/private/motor_client.cpp index 508f7268f..3c9b2a491 100644 --- a/src/viam/sdk/components/private/motor_client.cpp +++ b/src/viam/sdk/components/private/motor_client.cpp @@ -19,7 +19,7 @@ namespace impl { MotorClient::MotorClient(std::string name, std::shared_ptr channel) : Motor(std::move(name)), stub_(viam::component::motor::v1::MotorService::NewStub(channel)), - channel_(std::move(channel)) {}; + channel_(std::move(channel)){}; void MotorClient::set_power(double power_pct, const AttributeMap& extra) { return make_client_helper(this, *stub_, &StubType::SetPower) From 219532bdc3a046e0d457b836f7aaceca10104f0a Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 1 Oct 2024 13:30:46 -0400 Subject: [PATCH 3/9] include random --- src/viam/sdk/common/utils.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index aa5fc24c4..eac4a615e 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -1,5 +1,6 @@ #include +#include #include #include From e49a32af79b7b10c99d041a6a34f774107a326ea Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 7 Oct 2024 08:41:32 -0400 Subject: [PATCH 4/9] pr comments part 1 --- src/viam/sdk/common/utils.cpp | 3 +-- src/viam/sdk/common/utils.hpp | 4 ++-- src/viam/sdk/tests/mocks/mock_motion.cpp | 2 +- src/viam/sdk/tests/mocks/mock_motion.hpp | 3 ++- src/viam/sdk/tests/test_motion.cpp | 4 ++++ 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index eac4a615e..6a8f8c2bf 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -127,8 +127,7 @@ std::string random_debug_key() { } AttributeMap debug_map() { - auto debug_key = random_debug_key(); - return debug_map(debug_key); + return debug_map(random_debug_key()); } AttributeMap debug_map(std::string debug_key) { diff --git a/src/viam/sdk/common/utils.hpp b/src/viam/sdk/common/utils.hpp index 2c7ae9224..f40be4317 100644 --- a/src/viam/sdk/common/utils.hpp +++ b/src/viam/sdk/common/utils.hpp @@ -74,11 +74,11 @@ AttributeMap debug_map(); AttributeMap debug_map(std::string debug_key); /// @brief Adds @param debug_key for server-side debug logging to @param map -/// @throws Exception if the debug_key contains invalid (e.g., uppercase )gRPC characters +/// @throws Exception if the debug_key contains invalid (e.g., uppercase) gRPC characters AttributeMap add_debug_entry(AttributeMap&& map, std::string debug_key); /// @brief Adds @param debug_key for server-side debug logging to @param map -/// @throws Exception if the debug_key contains invalid (e.g., uppercase )gRPC characters +/// @throws Exception if the debug_key contains invalid (e.g., uppercase) gRPC characters AttributeMap add_debug_entry(AttributeMap map, std::string debug_key); /// @brief Adds a random key to @param map for server-side debug logging diff --git a/src/viam/sdk/tests/mocks/mock_motion.cpp b/src/viam/sdk/tests/mocks/mock_motion.cpp index ca66056ac..74d6268a5 100644 --- a/src/viam/sdk/tests/mocks/mock_motion.cpp +++ b/src/viam/sdk/tests/mocks/mock_motion.cpp @@ -65,7 +65,7 @@ std::string MockMotion::move_on_globe( pose_in_frame MockMotion::get_pose(const Name&, const std::string&, const std::vector&, - const AttributeMap&) { + const AttributeMap& extra) { return current_location; } diff --git a/src/viam/sdk/tests/mocks/mock_motion.hpp b/src/viam/sdk/tests/mocks/mock_motion.hpp index df88d47a3..6ba28c7c4 100644 --- a/src/viam/sdk/tests/mocks/mock_motion.hpp +++ b/src/viam/sdk/tests/mocks/mock_motion.hpp @@ -91,6 +91,7 @@ class MockMotion : public sdk::Motion { std::string peek_destination_frame; double peek_heading; bool peek_stop_plan_called = false; + std::string peek_debug_key; std::vector peek_obstacles; std::vector peek_map_obstacles; std::shared_ptr peek_constraints; @@ -99,7 +100,7 @@ class MockMotion : public sdk::Motion { std::shared_ptr peek_world_state; MockMotion(std::string name) - : sdk::Motion(std::move(name)), current_location(init_fake_pose()){}; + : sdk::Motion(std::move(name)), current_location(init_fake_pose()) {}; }; } // namespace motion diff --git a/src/viam/sdk/tests/test_motion.cpp b/src/viam/sdk/tests/test_motion.cpp index 49fddc105..35049f575 100644 --- a/src/viam/sdk/tests/test_motion.cpp +++ b/src/viam/sdk/tests/test_motion.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -171,6 +172,9 @@ BOOST_AUTO_TEST_CASE(test_move_and_get_pose) { std::string destination_frame("destination"); std::vector transforms; AttributeMap extra = fake_map(); + std::string debug_key = "debug-key"; + extra = add_debug_entry(extra, debug_key); + pose_in_frame pose = client.get_pose(fake_component_name(), destination_frame, {}, extra); BOOST_CHECK_EQUAL(pose, init_fake_pose()); From 2607f84766e3f12500c8b5c5b8b2974b7691f758 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 7 Oct 2024 10:43:25 -0400 Subject: [PATCH 5/9] fix clang format issue --- src/viam/sdk/tests/mocks/mock_motion.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/tests/mocks/mock_motion.hpp b/src/viam/sdk/tests/mocks/mock_motion.hpp index 1ac6cb9cb..aef661c96 100644 --- a/src/viam/sdk/tests/mocks/mock_motion.hpp +++ b/src/viam/sdk/tests/mocks/mock_motion.hpp @@ -100,7 +100,7 @@ class MockMotion : public sdk::Motion { std::shared_ptr peek_world_state; MockMotion(std::string name) - : sdk::Motion(std::move(name)), current_location(init_fake_pose()) {}; + : sdk::Motion(std::move(name)), current_location(init_fake_pose()){}; }; } // namespace motion From 60897cafcb9d8041ceedfb2f9766037042602ead Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 7 Oct 2024 12:07:26 -0400 Subject: [PATCH 6/9] more pr comments --- src/viam/sdk/common/utils.cpp | 13 +++++++++++-- src/viam/sdk/common/utils.hpp | 13 +++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index 7928d8f01..711fa5a76 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -135,13 +135,22 @@ ProtoStruct debug_map(std::string debug_key) { return map; } -ProtoStruct add_debug_entry(ProtoStruct&& map, std::string debug_key) { +void add_debug_entry(ProtoStruct& map, std::string debug_key) { map.emplace(impl::debug_map_key, ProtoValue(std::move(debug_key))); +} + +void add_debug_entry(ProtoStruct& map) { + add_debug_entry(map, random_debug_key()); +} + +ProtoStruct with_debug_entry(ProtoStruct&& map, std::string debug_key) { + add_debug_entry(map, std::move(debug_key)); return map; } ProtoStruct add_debug_entry(ProtoStruct&& map) { - return add_debug_entry(std::move(map), random_debug_key()); + add_debug_entry(map); + return map; } void ClientContext::set_debug_key(const std::string& debug_key) { diff --git a/src/viam/sdk/common/utils.hpp b/src/viam/sdk/common/utils.hpp index b60f091cf..20beca5ee 100644 --- a/src/viam/sdk/common/utils.hpp +++ b/src/viam/sdk/common/utils.hpp @@ -70,10 +70,19 @@ ProtoStruct debug_map(std::string debug_key); /// @brief Adds @param debug_key for server-side debug logging to @param map /// @throws Exception if the debug_key contains invalid (e.g., uppercase) gRPC characters -ProtoStruct add_debug_entry(ProtoStruct&& map, std::string debug_key); +void add_debug_entry(ProtoStruct& map, std::string debug_key); /// @brief Adds a random key to @param map for server-side debug logging -ProtoStruct add_debug_entry(ProtoStruct&& map); +void add_debug_entry(ProtoStruct& map); + +/// @brief Adds @param debug_key for server-side debug logging to @param map +/// @throws Exception if the debug_key contains invalid (e.g., uppercase) gRPC characters +/// @returns the new ProtoStruct +ProtoStruct with_debug_entry(ProtoStruct&& map, std::string debug_key); + +/// @brief Adds a random key to @param map for server-side debug logging +/// @returns the new ProtoStruct +ProtoStruct with_debug_entry(ProtoStruct&& map); /// @brief Set the boost trivial logger's severity depending on args. /// @param argc The number of args. From e9eb0dc53008eb51019925ddeb27d2b5ba4555dd Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 7 Oct 2024 12:08:11 -0400 Subject: [PATCH 7/9] fix fn name --- src/viam/sdk/common/utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index 711fa5a76..b3fec14c9 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -148,7 +148,7 @@ ProtoStruct with_debug_entry(ProtoStruct&& map, std::string debug_key) { return map; } -ProtoStruct add_debug_entry(ProtoStruct&& map) { +ProtoStruct with_debug_entry(ProtoStruct&& map) { add_debug_entry(map); return map; } From 56c8e670c9ec6111f0a787bd7627efc5bb96dd43 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 7 Oct 2024 13:08:41 -0400 Subject: [PATCH 8/9] fix test --- src/viam/sdk/tests/test_motion.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/tests/test_motion.cpp b/src/viam/sdk/tests/test_motion.cpp index 9176deaca..30e025c39 100644 --- a/src/viam/sdk/tests/test_motion.cpp +++ b/src/viam/sdk/tests/test_motion.cpp @@ -170,7 +170,7 @@ BOOST_AUTO_TEST_CASE(test_move_and_get_pose) { std::vector transforms; ProtoStruct extra = fake_map(); std::string debug_key = "debug-key"; - extra = add_debug_entry(std::move(extra), debug_key); + add_debug_entry(extra, debug_key); pose_in_frame pose = client.get_pose(fake_component_name(), destination_frame, {}, extra); BOOST_CHECK_EQUAL(mock->peek_debug_key, debug_key); BOOST_CHECK_EQUAL(pose, init_fake_pose()); From 87f475cd7ea0b0611b92ad95e08d8bb92d1302bf Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 7 Oct 2024 13:23:13 -0400 Subject: [PATCH 9/9] fix test2 --- src/viam/sdk/tests/test_motion.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/viam/sdk/tests/test_motion.cpp b/src/viam/sdk/tests/test_motion.cpp index 30e025c39..9e06c6aa4 100644 --- a/src/viam/sdk/tests/test_motion.cpp +++ b/src/viam/sdk/tests/test_motion.cpp @@ -168,10 +168,9 @@ BOOST_AUTO_TEST_CASE(test_move_and_get_pose) { client_to_mock_pipeline(mock, [&](Motion& client) { std::string destination_frame("destination"); std::vector transforms; - ProtoStruct extra = fake_map(); std::string debug_key = "debug-key"; - add_debug_entry(extra, debug_key); - pose_in_frame pose = client.get_pose(fake_component_name(), destination_frame, {}, extra); + pose_in_frame pose = client.get_pose( + fake_component_name(), destination_frame, {}, with_debug_entry(fake_map(), debug_key)); BOOST_CHECK_EQUAL(mock->peek_debug_key, debug_key); BOOST_CHECK_EQUAL(pose, init_fake_pose()); @@ -179,7 +178,7 @@ BOOST_AUTO_TEST_CASE(test_move_and_get_pose) { bool success = client.move(fake_pose(), fake_component_name(), ws, nullptr, fake_map()); BOOST_TEST(success); - pose = client.get_pose(fake_component_name(), destination_frame, transforms, extra); + pose = client.get_pose(fake_component_name(), destination_frame, transforms, fake_map()); BOOST_CHECK_EQUAL(pose, fake_pose()); }); }