From 765fa21f0e028d917ecf79adc27f5b93301dcf07 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 12 Feb 2024 11:51:23 -0500 Subject: [PATCH 1/7] init commit --- .clang-tidy | 2 -- src/viam/sdk/common/linear_algebra.cpp | 2 +- src/viam/sdk/common/linear_algebra.hpp | 2 +- src/viam/sdk/common/proto_type.cpp | 4 ++-- src/viam/sdk/common/proto_type.hpp | 4 ++-- src/viam/sdk/components/board/board.cpp | 8 ++++---- src/viam/sdk/components/board/board.hpp | 8 ++++---- 7 files changed, 14 insertions(+), 16 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index deda7293e..ea09448ce 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -6,7 +6,6 @@ # readability-named-parameter: Useful to fix lints about unused parameters # readability-implicit-bool-conversion: We have decided that !ptr-type is cleaner than ptr-type==nullptr # readability-magic-numbers: This encourages useless variables and extra lint lines -# performance-unnecessary-value-param: TODO(RSDK-3007) remove this and fix all lints. # performance-move-const-arg: TODO(RSDK-3019) # misc-unused-parameters: TODO(RSDK-3008) remove this and fix all lints. # misc-include-cleaner: TODO(RSDK-5479) this is overly finnicky, add IWYU support and fix. @@ -28,7 +27,6 @@ Checks: > -readability-named-parameter, -readability-implicit-bool-conversion, -readability-magic-numbers, - -performance-unnecessary-value-param, -performance-move-const-arg, -misc-unused-parameters, -misc-include-cleaner, diff --git a/src/viam/sdk/common/linear_algebra.cpp b/src/viam/sdk/common/linear_algebra.cpp index da6a64760..1ec094ec2 100644 --- a/src/viam/sdk/common/linear_algebra.cpp +++ b/src/viam/sdk/common/linear_algebra.cpp @@ -53,7 +53,7 @@ viam::common::v1::Vector3 Vector3::to_proto(const Vector3& vec) { return result; }; -Vector3 Vector3::from_proto(viam::common::v1::Vector3 vec) { +Vector3 Vector3::from_proto(const viam::common::v1::Vector3& vec) { return {vec.x(), vec.y(), vec.z()}; } diff --git a/src/viam/sdk/common/linear_algebra.hpp b/src/viam/sdk/common/linear_algebra.hpp index add469446..3afe00cad 100644 --- a/src/viam/sdk/common/linear_algebra.hpp +++ b/src/viam/sdk/common/linear_algebra.hpp @@ -31,7 +31,7 @@ class Vector3 { const std::array& data() const; std::array& data(); static viam::common::v1::Vector3 to_proto(const Vector3& vec); - static Vector3 from_proto(viam::common::v1::Vector3 vec); + static Vector3 from_proto(const viam::common::v1::Vector3& vec); private: std::array data_; diff --git a/src/viam/sdk/common/proto_type.cpp b/src/viam/sdk/common/proto_type.cpp index 01a6fc879..71d605b6e 100644 --- a/src/viam/sdk/common/proto_type.cpp +++ b/src/viam/sdk/common/proto_type.cpp @@ -19,7 +19,7 @@ using google::protobuf::Struct; using google::protobuf::Value; // NOLINTNEXTLINE(misc-no-recursion) -Struct map_to_struct(AttributeMap dict) { +Struct map_to_struct(const AttributeMap& dict) { Struct s; if (!dict) { return s; @@ -36,7 +36,7 @@ Struct map_to_struct(AttributeMap dict) { } // NOLINTNEXTLINE(misc-no-recursion) -AttributeMap struct_to_map(Struct struct_) { +AttributeMap struct_to_map(const Struct& struct_) { std::shared_ptr>> map = std::make_shared>>(); diff --git a/src/viam/sdk/common/proto_type.hpp b/src/viam/sdk/common/proto_type.hpp index 9c3dbe3db..dab1e3436 100644 --- a/src/viam/sdk/common/proto_type.hpp +++ b/src/viam/sdk/common/proto_type.hpp @@ -82,8 +82,8 @@ class ProtoType { proto_type_; }; -AttributeMap struct_to_map(google::protobuf::Struct struct_); -google::protobuf::Struct map_to_struct(AttributeMap dict); +AttributeMap struct_to_map(const google::protobuf::Struct& struct_); +google::protobuf::Struct map_to_struct(const AttributeMap& dict); } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/board/board.cpp b/src/viam/sdk/components/board/board.cpp index e15d72c2e..c906452ba 100644 --- a/src/viam/sdk/components/board/board.cpp +++ b/src/viam/sdk/components/board/board.cpp @@ -19,7 +19,7 @@ API API::traits::api() { return {kRDK, kComponent, "board"}; } -Board::status Board::from_proto(viam::common::v1::BoardStatus proto) { +Board::status Board::from_proto(const viam::common::v1::BoardStatus& proto) { Board::status status; for (const auto& analog : proto.analogs()) { status.analog_reader_values.emplace(analog.first, analog.second.value()); @@ -30,11 +30,11 @@ Board::status Board::from_proto(viam::common::v1::BoardStatus proto) { return status; } -Board::analog_value Board::from_proto(viam::common::v1::AnalogStatus proto) { +Board::analog_value Board::from_proto(const viam::common::v1::AnalogStatus& proto) { return proto.value(); } -Board::digital_value Board::from_proto(viam::common::v1::DigitalInterruptStatus proto) { +Board::digital_value Board::from_proto(const viam::common::v1::DigitalInterruptStatus& proto) { return proto.value(); } @@ -53,7 +53,7 @@ Board::power_mode Board::from_proto(viam::component::board::v1::PowerMode proto) } } -viam::common::v1::BoardStatus Board::to_proto(status status) { +viam::common::v1::BoardStatus Board::to_proto(const status& status) { viam::common::v1::BoardStatus proto; for (const auto& analog : status.analog_reader_values) { proto.mutable_analogs()->insert({analog.first, to_proto(analog.second)}); diff --git a/src/viam/sdk/components/board/board.hpp b/src/viam/sdk/components/board/board.hpp index aeea261d9..04596af0e 100644 --- a/src/viam/sdk/components/board/board.hpp +++ b/src/viam/sdk/components/board/board.hpp @@ -52,19 +52,19 @@ class Board : public Component { API api() const override; /// @brief Creates a `status` struct from its proto representation. - static status from_proto(viam::common::v1::BoardStatus proto); + static status from_proto(const viam::common::v1::BoardStatus& proto); /// @brief Creates a `analog_value` struct from its proto representation. - static analog_value from_proto(viam::common::v1::AnalogStatus proto); + static analog_value from_proto(const viam::common::v1::AnalogStatus& proto); /// @brief Creates a `digital_value` struct from its proto representation. - static digital_value from_proto(viam::common::v1::DigitalInterruptStatus proto); + static digital_value from_proto(const viam::common::v1::DigitalInterruptStatus& proto); /// @brief Creates a `power_mode` enum from its proto representation. static power_mode from_proto(viam::component::board::v1::PowerMode proto); /// @brief Converts a `status` struct to its proto representation. - static viam::common::v1::BoardStatus to_proto(status status); + static viam::common::v1::BoardStatus to_proto(const status& status); /// @brief Converts a `analog_value` struct to its proto representation. static viam::common::v1::AnalogStatus to_proto(analog_value analog_value); From 3e0b80f52301a8c17faadda84a490a7e0f55193a Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 12 Feb 2024 14:56:31 -0500 Subject: [PATCH 2/7] various fixes --- src/viam/sdk/components/camera/camera.cpp | 18 +++++++++++------- src/viam/sdk/components/camera/camera.hpp | 16 +++++++++------- src/viam/sdk/components/encoder/encoder.cpp | 6 ++++-- src/viam/sdk/components/encoder/encoder.hpp | 4 ++-- src/viam/sdk/components/motor/motor.cpp | 7 ++++--- src/viam/sdk/components/motor/motor.hpp | 7 ++++--- .../movement_sensor/movement_sensor.cpp | 8 ++++---- .../movement_sensor/movement_sensor.hpp | 10 ++++++---- .../components/power_sensor/power_sensor.cpp | 4 ++-- .../components/power_sensor/power_sensor.hpp | 4 ++-- src/viam/sdk/components/servo/servo.cpp | 2 +- src/viam/sdk/components/servo/servo.hpp | 2 +- src/viam/sdk/config/resource.cpp | 4 ++-- src/viam/sdk/config/resource.hpp | 4 ++-- src/viam/sdk/module/handler_map.cpp | 4 ++-- src/viam/sdk/module/handler_map.hpp | 4 ++-- src/viam/sdk/module/module.cpp | 2 +- src/viam/sdk/module/service.cpp | 10 +++++----- src/viam/sdk/module/service.hpp | 10 ++++++---- src/viam/sdk/referenceframe/frame.cpp | 2 +- src/viam/sdk/referenceframe/frame.hpp | 2 +- src/viam/sdk/resource/resource.cpp | 2 +- src/viam/sdk/resource/resource.hpp | 2 +- src/viam/sdk/resource/resource_api.cpp | 11 ++++++----- src/viam/sdk/robot/client.cpp | 14 ++++---------- src/viam/sdk/robot/client.hpp | 6 +++--- src/viam/sdk/robot/service.cpp | 19 ++++++++++--------- src/viam/sdk/robot/service.hpp | 8 ++++---- src/viam/sdk/rpc/dial.cpp | 8 ++++---- src/viam/sdk/rpc/dial.hpp | 3 ++- src/viam/sdk/rpc/server.cpp | 6 +++--- src/viam/sdk/rpc/server.hpp | 4 ++-- 32 files changed, 112 insertions(+), 101 deletions(-) diff --git a/src/viam/sdk/components/camera/camera.cpp b/src/viam/sdk/components/camera/camera.cpp index b4bc4ac57..14789f426 100644 --- a/src/viam/sdk/components/camera/camera.cpp +++ b/src/viam/sdk/components/camera/camera.cpp @@ -50,7 +50,8 @@ std::string Camera::format_to_MIME_string(viam::component::camera::v1::Format fo } } -::viam::component::camera::v1::Format Camera::MIME_string_to_format(std::string mime_string) { +::viam::component::camera::v1::Format Camera::MIME_string_to_format( + const std::string& mime_string) { if (mime_string == "image/vnd.viam.rgba") { return viam::component::camera::v1::FORMAT_RAW_RGBA; } @@ -66,7 +67,7 @@ ::viam::component::camera::v1::Format Camera::MIME_string_to_format(std::string return viam::component::camera::v1::FORMAT_UNSPECIFIED; } -Camera::raw_image Camera::from_proto(viam::component::camera::v1::GetImageResponse proto) { +Camera::raw_image Camera::from_proto(const viam::component::camera::v1::GetImageResponse& proto) { Camera::raw_image raw_image; std::string img_string = proto.image(); const std::vector bytes(img_string.begin(), img_string.end()); @@ -76,7 +77,8 @@ Camera::raw_image Camera::from_proto(viam::component::camera::v1::GetImageRespon return raw_image; } -Camera::image_collection Camera::from_proto(viam::component::camera::v1::GetImagesResponse proto) { +Camera::image_collection Camera::from_proto( + const viam::component::camera::v1::GetImagesResponse& proto) { Camera::image_collection image_collection; std::vector images; for (const auto& img : proto.images()) { @@ -93,7 +95,8 @@ Camera::image_collection Camera::from_proto(viam::component::camera::v1::GetImag return image_collection; } -Camera::point_cloud Camera::from_proto(viam::component::camera::v1::GetPointCloudResponse proto) { +Camera::point_cloud Camera::from_proto( + const viam::component::camera::v1::GetPointCloudResponse& proto) { Camera::point_cloud point_cloud; std::string pc_string = proto.point_cloud(); const std::vector bytes(pc_string.begin(), pc_string.end()); @@ -103,7 +106,7 @@ Camera::point_cloud Camera::from_proto(viam::component::camera::v1::GetPointClou } Camera::intrinsic_parameters Camera::from_proto( - viam::component::camera::v1::IntrinsicParameters proto) { + const viam::component::camera::v1::IntrinsicParameters& proto) { Camera::intrinsic_parameters params; // NOLINTNEXTLINE(bugprone-narrowing-conversions) params.width_px = proto.width_px(); @@ -117,14 +120,15 @@ Camera::intrinsic_parameters Camera::from_proto( } Camera::distortion_parameters Camera::from_proto( - viam::component::camera::v1::DistortionParameters proto) { + const viam::component::camera::v1::DistortionParameters& proto) { Camera::distortion_parameters params; params.model = proto.model(); params.parameters = {proto.parameters().begin(), proto.parameters().end()}; return params; } -Camera::properties Camera::from_proto(viam::component::camera::v1::GetPropertiesResponse proto) { +Camera::properties Camera::from_proto( + const viam::component::camera::v1::GetPropertiesResponse& proto) { Camera::distortion_parameters distortion_parameters; Camera::intrinsic_parameters intrinsic_parameters; Camera::properties properties; diff --git a/src/viam/sdk/components/camera/camera.hpp b/src/viam/sdk/components/camera/camera.hpp index 6dd4eb5e3..b5794dcbf 100644 --- a/src/viam/sdk/components/camera/camera.hpp +++ b/src/viam/sdk/components/camera/camera.hpp @@ -90,26 +90,28 @@ class Camera : public Component { static std::string format_to_MIME_string(viam::component::camera::v1::Format format); /// @brief convert a MIME type string with a protobuf format enum. - static viam::component::camera::v1::Format MIME_string_to_format(std::string mime_string); + static viam::component::camera::v1::Format MIME_string_to_format( + const std::string& mime_string); /// @brief Creates a `raw_image` struct from its proto representation. - static raw_image from_proto(viam::component::camera::v1::GetImageResponse proto); + static raw_image from_proto(const viam::component::camera::v1::GetImageResponse& proto); /// @brief Creates a `image_collection` struct from its proto representation. - static image_collection from_proto(viam::component::camera::v1::GetImagesResponse proto); + static image_collection from_proto(const viam::component::camera::v1::GetImagesResponse& proto); /// @brief Creates a `point_cloud` struct from its proto representation. - static point_cloud from_proto(viam::component::camera::v1::GetPointCloudResponse proto); + static point_cloud from_proto(const viam::component::camera::v1::GetPointCloudResponse& proto); /// @brief creates an `intrinsic_parameters` struct from its proto representation. - static intrinsic_parameters from_proto(viam::component::camera::v1::IntrinsicParameters proto); + static intrinsic_parameters from_proto( + const viam::component::camera::v1::IntrinsicParameters& proto); /// @brief creats a `distortion_parameters` struct from its proto representation. static distortion_parameters from_proto( - viam::component::camera::v1::DistortionParameters proto); + const viam::component::camera::v1::DistortionParameters& proto); /// @brief creates a `properties` struct from its proto representation. - static properties from_proto(viam::component::camera::v1::GetPropertiesResponse proto); + static properties from_proto(const viam::component::camera::v1::GetPropertiesResponse& proto); /// @brief converts a `distortion_parameters` struct to its proto representation. static viam::component::camera::v1::DistortionParameters to_proto(distortion_parameters); diff --git a/src/viam/sdk/components/encoder/encoder.cpp b/src/viam/sdk/components/encoder/encoder.cpp index 52a2505c7..e5d92a318 100644 --- a/src/viam/sdk/components/encoder/encoder.cpp +++ b/src/viam/sdk/components/encoder/encoder.cpp @@ -36,7 +36,8 @@ Encoder::position_type Encoder::from_proto(viam::component::encoder::v1::Positio } } -Encoder::position Encoder::from_proto(viam::component::encoder::v1::GetPositionResponse proto) { +Encoder::position Encoder::from_proto( + const viam::component::encoder::v1::GetPositionResponse& proto) { Encoder::position position; position.value = proto.value(); @@ -44,7 +45,8 @@ Encoder::position Encoder::from_proto(viam::component::encoder::v1::GetPositionR return position; } -Encoder::properties Encoder::from_proto(viam::component::encoder::v1::GetPropertiesResponse proto) { +Encoder::properties Encoder::from_proto( + const viam::component::encoder::v1::GetPropertiesResponse& proto) { Encoder::properties properties; properties.ticks_count_supported = proto.ticks_count_supported(); diff --git a/src/viam/sdk/components/encoder/encoder.hpp b/src/viam/sdk/components/encoder/encoder.hpp index 85ed6c450..27a934ac7 100644 --- a/src/viam/sdk/components/encoder/encoder.hpp +++ b/src/viam/sdk/components/encoder/encoder.hpp @@ -50,10 +50,10 @@ class Encoder : public Component { static position_type from_proto(viam::component::encoder::v1::PositionType proto); /// @brief Creates a `position` struct from its proto representation. - static position from_proto(viam::component::encoder::v1::GetPositionResponse proto); + static position from_proto(const viam::component::encoder::v1::GetPositionResponse& proto); /// @brief Creates a `properties` struct from its proto representation. - static properties from_proto(viam::component::encoder::v1::GetPropertiesResponse proto); + static properties from_proto(const viam::component::encoder::v1::GetPropertiesResponse& proto); /// @brief Converts a `position_type` struct to its proto representation. static viam::component::encoder::v1::PositionType to_proto(position_type position_type); diff --git a/src/viam/sdk/components/motor/motor.cpp b/src/viam/sdk/components/motor/motor.cpp index 7e87cc25b..978c0be1e 100644 --- a/src/viam/sdk/components/motor/motor.cpp +++ b/src/viam/sdk/components/motor/motor.cpp @@ -11,7 +11,7 @@ namespace viam { namespace sdk { -Motor::position Motor::from_proto(viam::component::motor::v1::GetPositionResponse proto) { +Motor::position Motor::from_proto(const viam::component::motor::v1::GetPositionResponse& proto) { return proto.position(); } API Motor::api() const { @@ -22,7 +22,7 @@ API API::traits::api() { return {kRDK, kComponent, "motor"}; } -Motor::power_status Motor::from_proto(viam::component::motor::v1::IsPoweredResponse proto) { +Motor::power_status Motor::from_proto(const viam::component::motor::v1::IsPoweredResponse& proto) { Motor::power_status power_status; power_status.is_on = proto.is_on(); @@ -30,7 +30,8 @@ Motor::power_status Motor::from_proto(viam::component::motor::v1::IsPoweredRespo return power_status; } -Motor::properties Motor::from_proto(viam::component::motor::v1::GetPropertiesResponse proto) { +Motor::properties Motor::from_proto( + const viam::component::motor::v1::GetPropertiesResponse& proto) { Motor::properties properties; properties.position_reporting = proto.position_reporting(); return properties; diff --git a/src/viam/sdk/components/motor/motor.hpp b/src/viam/sdk/components/motor/motor.hpp index 173cd69e3..39284b4dd 100644 --- a/src/viam/sdk/components/motor/motor.hpp +++ b/src/viam/sdk/components/motor/motor.hpp @@ -46,14 +46,15 @@ class Motor : public Component, public Stoppable { }; /// @brief Creates a `position` struct from its proto representation. - static position from_proto(viam::component::motor::v1::GetPositionResponse proto); + static position from_proto(const viam::component::motor::v1::GetPositionResponse& proto); /// @brief Creates a `power_status` struct from its proto representation. - static power_status from_proto(viam::component::motor::v1::IsPoweredResponse proto); + static power_status from_proto(const viam::component::motor::v1::IsPoweredResponse& proto); /// @brief Creates a `properties` struct from its proto representation. - static properties from_proto(viam::component::motor::v1::GetPropertiesResponse proto); + static properties from_proto(const viam::component::motor::v1::GetPropertiesResponse& proto); + /// // CR erodkin: why aren't the `to_proto` methods complaining about lack of `const&`? /// @brief Converts a `position` struct to its proto representation. static viam::component::motor::v1::GetPositionResponse to_proto(position position); diff --git a/src/viam/sdk/components/movement_sensor/movement_sensor.cpp b/src/viam/sdk/components/movement_sensor/movement_sensor.cpp index f8e5655a1..a802ca819 100644 --- a/src/viam/sdk/components/movement_sensor/movement_sensor.cpp +++ b/src/viam/sdk/components/movement_sensor/movement_sensor.cpp @@ -20,21 +20,21 @@ API API::traits::api() { } MovementSensor::compassheading MovementSensor::from_proto( - viam::component::movementsensor::v1::GetCompassHeadingResponse proto) { + const viam::component::movementsensor::v1::GetCompassHeadingResponse& proto) { MovementSensor::compassheading compassheading; compassheading.value = proto.value(); return compassheading; } MovementSensor::position MovementSensor::from_proto( - viam::component::movementsensor::v1::GetPositionResponse proto) { + const viam::component::movementsensor::v1::GetPositionResponse& proto) { MovementSensor::position position; position.coordinate = viam::sdk::geo_point::from_proto(proto.coordinate()); position.altitude_m = proto.altitude_m(); return position; } -MovementSensor::orientation MovementSensor::from_proto(viam::common::v1::Orientation proto) { +MovementSensor::orientation MovementSensor::from_proto(const viam::common::v1::Orientation& proto) { MovementSensor::orientation orientation; orientation.o_x = proto.o_x(); orientation.o_y = proto.o_y(); @@ -44,7 +44,7 @@ MovementSensor::orientation MovementSensor::from_proto(viam::common::v1::Orienta } MovementSensor::properties MovementSensor::from_proto( - viam::component::movementsensor::v1::GetPropertiesResponse proto) { + const viam::component::movementsensor::v1::GetPropertiesResponse& proto) { MovementSensor::properties properties; properties.linear_velocity_supported = proto.linear_velocity_supported(); properties.angular_velocity_supported = proto.angular_velocity_supported(); diff --git a/src/viam/sdk/components/movement_sensor/movement_sensor.hpp b/src/viam/sdk/components/movement_sensor/movement_sensor.hpp index bd22c2ab1..d24d82ea1 100644 --- a/src/viam/sdk/components/movement_sensor/movement_sensor.hpp +++ b/src/viam/sdk/components/movement_sensor/movement_sensor.hpp @@ -57,16 +57,18 @@ class MovementSensor : public Component { /// @brief Creates a `compassheading` struct from its proto representation. static compassheading from_proto( - viam::component::movementsensor::v1::GetCompassHeadingResponse proto); + const viam::component::movementsensor::v1::GetCompassHeadingResponse& proto); /// @brief Creates a `position` struct from its proto representation. - static position from_proto(viam::component::movementsensor::v1::GetPositionResponse proto); + static position from_proto( + const viam::component::movementsensor::v1::GetPositionResponse& proto); /// @brief Creates an `orientation` struct from its proto representation. - static orientation from_proto(viam::common::v1::Orientation proto); + static orientation from_proto(const viam::common::v1::Orientation& proto); /// @brief Creates a `properties` struct from its proto representation. - static properties from_proto(viam::component::movementsensor::v1::GetPropertiesResponse proto); + static properties from_proto( + const viam::component::movementsensor::v1::GetPropertiesResponse& proto); /// @brief Converts a `compassheading` struct to its proto representation. static viam::component::movementsensor::v1::GetCompassHeadingResponse to_proto( diff --git a/src/viam/sdk/components/power_sensor/power_sensor.cpp b/src/viam/sdk/components/power_sensor/power_sensor.cpp index 137c9a7e2..bb142a7be 100644 --- a/src/viam/sdk/components/power_sensor/power_sensor.cpp +++ b/src/viam/sdk/components/power_sensor/power_sensor.cpp @@ -19,14 +19,14 @@ API API::traits::api() { return {kRDK, kComponent, "power_sensor"}; } -PowerSensor::voltage PowerSensor::from_proto(GetVoltageResponse proto) { +PowerSensor::voltage PowerSensor::from_proto(const GetVoltageResponse& proto) { PowerSensor::voltage v; v.volts = proto.volts(); v.is_ac = proto.is_ac(); return v; } -PowerSensor::current PowerSensor::from_proto(GetCurrentResponse proto) { +PowerSensor::current PowerSensor::from_proto(const GetCurrentResponse& proto) { PowerSensor::current c; c.amperes = proto.amperes(); c.is_ac = proto.is_ac(); diff --git a/src/viam/sdk/components/power_sensor/power_sensor.hpp b/src/viam/sdk/components/power_sensor/power_sensor.hpp index b2052b7af..f77f531f1 100644 --- a/src/viam/sdk/components/power_sensor/power_sensor.hpp +++ b/src/viam/sdk/components/power_sensor/power_sensor.hpp @@ -44,10 +44,10 @@ class PowerSensor : public Component { API api() const override; /// @brief Creates a `voltage` struct from its proto representation. - static voltage from_proto(GetVoltageResponse proto); + static voltage from_proto(const GetVoltageResponse& proto); /// @brief Creates a `current` struct from its proto representation. - static current from_proto(GetCurrentResponse proto); + static current from_proto(const GetCurrentResponse& proto); /// @brief Converts a `voltage` struct to its proto representation. static GetVoltageResponse to_proto(voltage v); diff --git a/src/viam/sdk/components/servo/servo.cpp b/src/viam/sdk/components/servo/servo.cpp index 2b6b558b9..90ede6baa 100644 --- a/src/viam/sdk/components/servo/servo.cpp +++ b/src/viam/sdk/components/servo/servo.cpp @@ -17,7 +17,7 @@ API API::traits::api() { return {kRDK, kComponent, "servo"}; } -Servo::position Servo::from_proto(viam::component::servo::v1::GetPositionResponse proto) { +Servo::position Servo::from_proto(const viam::component::servo::v1::GetPositionResponse& proto) { return proto.position_deg(); } diff --git a/src/viam/sdk/components/servo/servo.hpp b/src/viam/sdk/components/servo/servo.hpp index d9012b66f..5a4b9d406 100644 --- a/src/viam/sdk/components/servo/servo.hpp +++ b/src/viam/sdk/components/servo/servo.hpp @@ -29,7 +29,7 @@ class Servo : public Component, public Stoppable { API api() const override; /// @brief Creates a `position` struct from its proto representation. - static position from_proto(viam::component::servo::v1::GetPositionResponse proto); + static position from_proto(const viam::component::servo::v1::GetPositionResponse& proto); /// @brief Move the servo to the provided angle /// @param angle_deg The desired angle of the servo in degrees. diff --git a/src/viam/sdk/config/resource.cpp b/src/viam/sdk/config/resource.cpp index 4a002dfcf..42139699b 100644 --- a/src/viam/sdk/config/resource.cpp +++ b/src/viam/sdk/config/resource.cpp @@ -89,7 +89,7 @@ void ResourceConfig::fix_api() { } } -ResourceConfig ResourceConfig::from_proto(viam::app::v1::ComponentConfig proto_cfg) { +ResourceConfig ResourceConfig::from_proto(const viam::app::v1::ComponentConfig& proto_cfg) { ResourceConfig resource(proto_cfg.type()); resource.name_ = proto_cfg.name(); resource.namespace__ = proto_cfg.namespace_(); @@ -138,7 +138,7 @@ viam::app::v1::ComponentConfig ResourceConfig::to_proto() const { return proto_cfg; } -ResourceConfig::ResourceConfig(std::string type) : api_({kRDK, type, ""}), type_(type){}; +ResourceConfig::ResourceConfig(const std::string& type) : api_({kRDK, type, ""}), type_(type){}; } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/config/resource.hpp b/src/viam/sdk/config/resource.hpp index 801e737cc..50ae69845 100644 --- a/src/viam/sdk/config/resource.hpp +++ b/src/viam/sdk/config/resource.hpp @@ -22,9 +22,9 @@ class ResourceLevelServiceConfig { class ResourceConfig { public: - static ResourceConfig from_proto(viam::app::v1::ComponentConfig proto_cfg); + static ResourceConfig from_proto(const viam::app::v1::ComponentConfig& proto_cfg); viam::app::v1::ComponentConfig to_proto() const; - ResourceConfig(std::string type); + ResourceConfig(const std::string& type); Name resource_name(); const API& api() const; const LinkConfig& frame() const; diff --git a/src/viam/sdk/module/handler_map.cpp b/src/viam/sdk/module/handler_map.cpp index 2067b1957..1eb9f644b 100644 --- a/src/viam/sdk/module/handler_map.cpp +++ b/src/viam/sdk/module/handler_map.cpp @@ -37,7 +37,7 @@ viam::module::v1::HandlerMap HandlerMap_::to_proto() const { HandlerMap_::HandlerMap_(){}; // NOLINTNEXTLINE(readability-const-return-type) -const HandlerMap_ HandlerMap_::from_proto(viam::module::v1::HandlerMap proto) { +const HandlerMap_ HandlerMap_::from_proto(const viam::module::v1::HandlerMap& proto) { HandlerMap_ hm; const google::protobuf::RepeatedPtrField& handlers = @@ -65,7 +65,7 @@ const HandlerMap_ HandlerMap_::from_proto(viam::module::v1::HandlerMap proto) { return hm; } -void HandlerMap_::add_model(Model model, RPCSubtype subtype) { +void HandlerMap_::add_model(const Model& model, const RPCSubtype& subtype) { handles_[subtype].push_back(model); } diff --git a/src/viam/sdk/module/handler_map.hpp b/src/viam/sdk/module/handler_map.hpp index 704c64f69..d15bcf5d3 100644 --- a/src/viam/sdk/module/handler_map.hpp +++ b/src/viam/sdk/module/handler_map.hpp @@ -10,10 +10,10 @@ namespace sdk { class HandlerMap_ { public: HandlerMap_(); - void add_model(Model model, RPCSubtype subtype); + void add_model(const Model& model, const RPCSubtype& subtype); viam::module::v1::HandlerMap to_proto() const; - static const HandlerMap_ from_proto(viam::module::v1::HandlerMap proto); + static const HandlerMap_ from_proto(const viam::module::v1::HandlerMap& proto); friend std::ostream& operator<<(std::ostream& os, const HandlerMap_& hm); private: diff --git a/src/viam/sdk/module/module.cpp b/src/viam/sdk/module/module.cpp index 87a3ee908..3c09df767 100644 --- a/src/viam/sdk/module/module.cpp +++ b/src/viam/sdk/module/module.cpp @@ -10,7 +10,7 @@ namespace viam { namespace sdk { -Module::Module(std::string addr) : addr_(addr){}; +Module::Module(std::string addr) : addr_(std::move(addr)){}; void Module::set_ready() { ready_ = true; diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 90217bbb9..800a6665d 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -61,7 +61,7 @@ Dependencies ModuleService::get_dependencies_( return deps; } -std::shared_ptr ModuleService::get_parent_resource_(Name name) { +std::shared_ptr ModuleService::get_parent_resource_(const Name& name) { if (!parent_) { parent_ = RobotClient::at_local_socket(parent_addr_, {0, boost::none}); } @@ -215,7 +215,7 @@ ModuleService::ModuleService(std::string addr) ModuleService::ModuleService(int argc, char** argv, - std::vector> registrations) { + const std::vector>& registrations) { if (argc < 2) { throw std::runtime_error("Need socket path as command line argument"); } @@ -264,8 +264,8 @@ ModuleService::~ModuleService() { } } -void ModuleService::add_model_from_registry_inlock_(API api, - Model model, +void ModuleService::add_model_from_registry_inlock_(const API& api, + const Model& model, const std::lock_guard& lock) { const std::shared_ptr creator = Registry::lookup_resource_server(api); @@ -279,7 +279,7 @@ void ModuleService::add_model_from_registry_inlock_(API api, module_->mutable_handles().add_model(model, rpc_subtype); }; -void ModuleService::add_model_from_registry(API api, Model model) { +void ModuleService::add_model_from_registry(const API& api, const Model& model) { const std::lock_guard lock(lock_); return add_model_from_registry_inlock_(api, model, lock); } diff --git a/src/viam/sdk/module/service.hpp b/src/viam/sdk/module/service.hpp index 077bb4616..86b772214 100644 --- a/src/viam/sdk/module/service.hpp +++ b/src/viam/sdk/module/service.hpp @@ -35,7 +35,7 @@ class ModuleService : viam::module::v1::ModuleService::Service { /// @param registrations Models to register and add to the module. explicit ModuleService(int argc, char** argv, - std::vector> registrations); + const std::vector>& registrations); ~ModuleService(); /// @brief Starts module. serve will return when SIGINT or SIGTERM is received @@ -47,7 +47,7 @@ class ModuleService : viam::module::v1::ModuleService::Service { /// of ModelRegistration, the passed in models will already be registered and added. /// @param api The API to add. /// @param model The model to add. - void add_model_from_registry(API api, Model model); + void add_model_from_registry(const API& api, const Model& model); private: ::grpc::Status AddResource(::grpc::ServerContext* context, @@ -71,10 +71,12 @@ class ModuleService : viam::module::v1::ModuleService::Service { const ::viam::module::v1::ValidateConfigRequest* request, ::viam::module::v1::ValidateConfigResponse* response) override; - void add_model_from_registry_inlock_(API api, Model model, const std::lock_guard&); + void add_model_from_registry_inlock_(const API& api, + const Model& model, + const std::lock_guard&); Dependencies get_dependencies_(google::protobuf::RepeatedPtrField const& proto, std::string const& resource_name); - std::shared_ptr get_parent_resource_(Name name); + std::shared_ptr get_parent_resource_(const Name& name); std::mutex lock_; std::unique_ptr module_; diff --git a/src/viam/sdk/referenceframe/frame.cpp b/src/viam/sdk/referenceframe/frame.cpp index d352df1b2..299942384 100644 --- a/src/viam/sdk/referenceframe/frame.cpp +++ b/src/viam/sdk/referenceframe/frame.cpp @@ -22,7 +22,7 @@ viam::app::v1::Frame LinkConfig::to_proto() const { return frame; }; -LinkConfig LinkConfig::from_proto(viam::app::v1::Frame proto) { +LinkConfig LinkConfig::from_proto(const viam::app::v1::Frame& proto) { LinkConfig lc; lc.parent_ = proto.parent(); diff --git a/src/viam/sdk/referenceframe/frame.hpp b/src/viam/sdk/referenceframe/frame.hpp index 0668e3ba4..4d5f593e5 100644 --- a/src/viam/sdk/referenceframe/frame.hpp +++ b/src/viam/sdk/referenceframe/frame.hpp @@ -13,7 +13,7 @@ namespace sdk { class LinkConfig { public: viam::app::v1::Frame to_proto() const; - static LinkConfig from_proto(viam::app::v1::Frame proto); + static LinkConfig from_proto(const viam::app::v1::Frame& proto); translation get_translation() const; OrientationConfig get_orientation_config() const; GeometryConfig get_geometry_config() const; diff --git a/src/viam/sdk/resource/resource.cpp b/src/viam/sdk/resource/resource.cpp index adbd6019f..61db75452 100644 --- a/src/viam/sdk/resource/resource.cpp +++ b/src/viam/sdk/resource/resource.cpp @@ -19,7 +19,7 @@ std::string Resource::name() const { return name_; } -void Resource::reconfigure(Dependencies deps, ResourceConfig cfg){}; +void Resource::reconfigure(const Dependencies& deps, const ResourceConfig& cfg){}; ResourceName Resource::get_resource_name(std::string name) const { ResourceName r; diff --git a/src/viam/sdk/resource/resource.hpp b/src/viam/sdk/resource/resource.hpp index eacfd2126..0a3456f43 100644 --- a/src/viam/sdk/resource/resource.hpp +++ b/src/viam/sdk/resource/resource.hpp @@ -28,7 +28,7 @@ class Resource { /// @brief Reconfigures a resource. /// @param deps Dependencies of the resource. /// @param cfg The resource's config. - virtual void reconfigure(Dependencies deps, ResourceConfig cfg); + virtual void reconfigure(const Dependencies& deps, const ResourceConfig& cfg); /// @brief Return the resource's name. virtual std::string name() const; diff --git a/src/viam/sdk/resource/resource_api.cpp b/src/viam/sdk/resource/resource_api.cpp index af3eb6d69..53ce5ae66 100644 --- a/src/viam/sdk/resource/resource_api.cpp +++ b/src/viam/sdk/resource/resource_api.cpp @@ -31,7 +31,7 @@ std::string APIType::to_string() const { } APIType::APIType(std::string namespace_, std::string resource_type) - : namespace_(namespace_), resource_type_(resource_type){}; + : namespace_(std::move(namespace_)), resource_type_(std::move(resource_type)){}; std::string API::to_string() const { return APIType::to_string() + ":" + resource_subtype_; @@ -71,10 +71,11 @@ API API::from_string(std::string api) { } API::API(APIType type, std::string resource_subtype) - : APIType(type), resource_subtype_(resource_subtype) {} + : APIType(std::move(type)), resource_subtype_(std::move(resource_subtype)) {} API::API(std::string namespace_, std::string resource_type, std::string resource_subtype) - : APIType(namespace_, resource_type), resource_subtype_(resource_subtype) {} + : APIType(std::move(namespace_), std::move(resource_type)), + resource_subtype_(std::move(resource_subtype)) {} bool API::is_service_type() { return (this->resource_type() == "service"); @@ -163,7 +164,7 @@ Name Name::from_string(std::string name) { } Name::Name(API api, std::string remote, std::string name) - : api_(api), remote_name_(std::move(remote)), name_(std::move(name)) {} + : api_(std::move(api)), remote_name_(std::move(remote)), name_(std::move(name)) {} bool operator==(const API& lhs, const API& rhs) { return lhs.to_string() == rhs.to_string(); @@ -216,7 +217,7 @@ const API& RPCSubtype::api() const { }; ModelFamily::ModelFamily(std::string namespace_, std::string family) - : namespace_(namespace_), family_(family) {} + : namespace_(std::move(namespace_)), family_(std::move(family)) {} Model::Model(ModelFamily model_family, std::string model_name) : model_family_(std::move(model_family)), model_name_(std::move(model_name)) {} diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index 23534f140..06533fa61 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -1,17 +1,11 @@ #include -#include #include #include -#include #include #include -#include -#include #include #include -#include -#include #include #include @@ -78,7 +72,7 @@ void RobotClient::close() { viam_channel_->close(); } -bool is_error_response(grpc::Status response) { +bool is_error_response(const grpc::Status& response) { return !response.ok() && (response.error_message() != kStreamRemoved); } std::vector RobotClient::get_status() { @@ -222,7 +216,7 @@ void RobotClient::refresh_every() { } }; -RobotClient::RobotClient(std::shared_ptr channel) +RobotClient::RobotClient(const std::shared_ptr& channel) : viam_channel_(channel), channel_(channel->channel()), should_close_channel_(false), @@ -234,8 +228,8 @@ std::vector* RobotClient::resource_names() { return resources; } -std::shared_ptr RobotClient::with_channel(std::shared_ptr channel, - Options options) { +std::shared_ptr RobotClient::with_channel(const std::shared_ptr& channel, + const Options& options) { std::shared_ptr robot = std::make_shared(channel); robot->refresh_interval_ = options.refresh_interval(); robot->should_refresh_ = (robot->refresh_interval_ > 0); diff --git a/src/viam/sdk/robot/client.hpp b/src/viam/sdk/robot/client.hpp index 2597f4b01..f92f85592 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -65,9 +65,9 @@ class RobotClient { /// @param options Options for connecting and refreshing. /// Connects directly to a pre-existing channel. A robot created this way must be /// `close()`d manually. - static std::shared_ptr with_channel(std::shared_ptr channel, - Options options); - RobotClient(std::shared_ptr channel); + static std::shared_ptr with_channel(const std::shared_ptr& channel, + const Options& options); + RobotClient(const std::shared_ptr& channel); std::vector* resource_names(); /// @brief Lookup and return a `shared_ptr` to a resource. diff --git a/src/viam/sdk/robot/service.cpp b/src/viam/sdk/robot/service.cpp index fc2dac40e..c824b41fd 100644 --- a/src/viam/sdk/robot/service.cpp +++ b/src/viam/sdk/robot/service.cpp @@ -30,7 +30,7 @@ using google::protobuf::RepeatedPtrField; using viam::common::v1::ResourceName; using viam::robot::v1::Status; -RobotService_::RobotService_(std::shared_ptr manager, Server& server) +RobotService_::RobotService_(const std::shared_ptr& manager, Server& server) : ResourceServer(manager) { server.register_service(this); // register all managed resources with the appropriate resource servers. @@ -39,7 +39,7 @@ RobotService_::RobotService_(std::shared_ptr manager, Server& s } } -std::vector RobotService_::generate_metadata() { +std::vector RobotService_::generate_metadata_() { std::vector metadata; for (const auto& key_and_val : resource_manager()->resources()) { for (const ResourceName& resource : resource_names_for_resource(key_and_val.second)) { @@ -49,7 +49,8 @@ std::vector RobotService_::generate_metadata() { return metadata; } -std::vector RobotService_::generate_status(RepeatedPtrField resource_names) { +std::vector RobotService_::generate_status_( + const RepeatedPtrField& resource_names) { std::vector statuses; for (const auto& cmp : resource_manager()->resources()) { const std::shared_ptr resource = cmp.second; @@ -58,7 +59,7 @@ std::vector RobotService_::generate_status(RepeatedPtrFieldapi().resource_subtype() == resource->api().resource_subtype()) { bool resource_present = false; const ResourceName name = resource->get_resource_name(resource->name()); - for (auto& resource_name : resource_names) { + for (const auto& resource_name : resource_names) { if (ResourceNameEqual::check_equal(name, resource_name)) { resource_present = true; break; @@ -76,7 +77,7 @@ std::vector RobotService_::generate_status(RepeatedPtrField returnable_statuses; for (auto& status : statuses) { bool status_name_is_known = false; - for (auto& resource_name : resource_names) { + for (const auto& resource_name : resource_names) { if (status.name().SerializeAsString() == resource_name.SerializeAsString()) { status_name_is_known = true; break; @@ -97,7 +98,7 @@ ::grpc::Status RobotService_::ResourceNames(::grpc::ServerContext* context, "Called [ResourceNames] without a request"); }; RepeatedPtrField* p = response->mutable_resources(); - for (const ResourceName& name : generate_metadata()) { + for (const ResourceName& name : generate_metadata_()) { *p->Add() = name; } @@ -113,7 +114,7 @@ ::grpc::Status RobotService_::GetStatus(::grpc::ServerContext* context, }; RepeatedPtrField* response_status = response->mutable_status(); - const std::vector statuses = generate_status(request->resource_names()); + const std::vector statuses = generate_status_(request->resource_names()); for (const Status& status : statuses) { *response_status->Add() = status; } @@ -126,7 +127,7 @@ void RobotService_::stream_status( ::grpc::ServerWriter<::viam::robot::v1::StreamStatusResponse>* writer, int interval) { while (true) { - const std::vector statuses = generate_status(request->resource_names()); + const std::vector statuses = generate_status_(request->resource_names()); viam::robot::v1::StreamStatusResponse response; RepeatedPtrField* response_status = response.mutable_status(); for (const Status& status : statuses) { @@ -202,7 +203,7 @@ ::grpc::Status RobotService_::StopAll(::grpc::ServerContext* context, return grpc::Status(status, status_message); } -std::shared_ptr RobotService_::resource_by_name(Name name) { +std::shared_ptr RobotService_::resource_by_name(const Name& name) { std::shared_ptr r; const std::lock_guard lock(lock_); auto resources = resource_manager()->resources(); diff --git a/src/viam/sdk/robot/service.hpp b/src/viam/sdk/robot/service.hpp index 2864b2caa..3b5cf4b1f 100644 --- a/src/viam/sdk/robot/service.hpp +++ b/src/viam/sdk/robot/service.hpp @@ -32,8 +32,8 @@ using viam::robot::v1::Status; /// @ingroup Robot class RobotService_ : public ResourceServer, public viam::robot::v1::RobotService::Service { public: - explicit RobotService_(std::shared_ptr manager, Server& server); - std::shared_ptr resource_by_name(Name name); + explicit RobotService_(const std::shared_ptr& manager, Server& server); + std::shared_ptr resource_by_name(const Name& name); ::grpc::Status ResourceNames(::grpc::ServerContext* context, const ::viam::robot::v1::ResourceNamesRequest* request, ::viam::robot::v1::ResourceNamesResponse* response) override; @@ -50,8 +50,8 @@ class RobotService_ : public ResourceServer, public viam::robot::v1::RobotServic private: std::mutex lock_; - std::vector generate_metadata(); - std::vector generate_status(RepeatedPtrField resource_names); + std::vector generate_metadata_(); + std::vector generate_status_(const RepeatedPtrField& resource_names); void stream_status(const ::viam::robot::v1::StreamStatusRequest* request, ::grpc::ServerWriter<::viam::robot::v1::StreamStatusResponse>* writer, diff --git a/src/viam/sdk/rpc/dial.cpp b/src/viam/sdk/rpc/dial.cpp index 4ac15590e..0b63989c8 100644 --- a/src/viam/sdk/rpc/dial.cpp +++ b/src/viam/sdk/rpc/dial.cpp @@ -47,16 +47,16 @@ const std::string& Credentials::payload() const { } ViamChannel::ViamChannel(std::shared_ptr channel, const char* path, void* runtime) - : channel_(channel), path_(path), closed_(false), rust_runtime_(runtime) {} + : channel_(std::move(channel)), path_(path), closed_(false), rust_runtime_(runtime) {} DialOptions::DialOptions() = default; void DialOptions::set_credentials(boost::optional creds) { - credentials_ = creds; + credentials_ = std::move(creds); } void DialOptions::set_entity(boost::optional entity) { - auth_entity_ = entity; + auth_entity_ = std::move(entity); } void DialOptions::set_timeout(std::chrono::duration timeout) { @@ -84,7 +84,7 @@ bool DialOptions::allows_insecure_downgrade() const { } std::shared_ptr ViamChannel::dial(const char* uri, - boost::optional options) { + const boost::optional& options) { void* ptr = init_rust_runtime(); const DialOptions opts = options.get_value_or(DialOptions()); const std::chrono::duration float_timeout = opts.timeout(); diff --git a/src/viam/sdk/rpc/dial.hpp b/src/viam/sdk/rpc/dial.hpp index d9b8c4518..86fbf582a 100644 --- a/src/viam/sdk/rpc/dial.hpp +++ b/src/viam/sdk/rpc/dial.hpp @@ -14,7 +14,8 @@ class ViamChannel { public: void close(); ViamChannel(std::shared_ptr channel, const char* path, void* runtime); - static std::shared_ptr dial(const char* uri, boost::optional options); + static std::shared_ptr dial(const char* uri, + const boost::optional& options); const std::shared_ptr& channel() const; diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 7988520c4..d5f790eef 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -39,7 +39,7 @@ void Server::register_service(grpc::Service* service) { builder_->RegisterService(service); } -void Server::add_resource(std::shared_ptr resource) { +void Server::add_resource(const std::shared_ptr& resource) { auto api = resource->api(); if (managed_servers_.find(api) == managed_servers_.end()) { std::ostringstream buffer; @@ -60,7 +60,7 @@ void Server::start() { builder_ = nullptr; } -void Server::add_listening_port(std::string address, +void Server::add_listening_port(const std::string& address, std::shared_ptr creds) { if (!builder_) { throw std::runtime_error("Cannot add a listening port after server has started"); @@ -70,7 +70,7 @@ void Server::add_listening_port(std::string address, creds = grpc::InsecureServerCredentials(); } - builder_->AddListeningPort(address, creds); + builder_->AddListeningPort(address, std::move(creds)); } void Server::wait() { diff --git a/src/viam/sdk/rpc/server.hpp b/src/viam/sdk/rpc/server.hpp index 5e1a12f03..b3dfc26a8 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -45,13 +45,13 @@ class Server { /// @brief Adds a specific managed resource to the associated resource server /// @param resource The resource to add /// @throws `std::runtime_error` if a matching `ResourceServer` doesn't exist in the server. - void add_resource(std::shared_ptr resource); + void add_resource(const std::shared_ptr& resource); /// @brief Adds a listening port to the server. /// @param address The address to listen at. /// @param creds The server credentials; defaults to a insecure server credentials. /// @throws `std::runtime_error` if called after the server has been `start`ed. - void add_listening_port(std::string address, + void add_listening_port(const std::string& address, std::shared_ptr creds = nullptr); /// @brief waits on server close, only returning when the server is closed. From 9b20bf2546f5ed8f238079562e23207d66469fb6 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 13 Feb 2024 11:25:47 -0500 Subject: [PATCH 3/7] remaining fixes --- .../examples/modules/complex/base/impl.cpp | 2 +- .../examples/modules/complex/base/impl.hpp | 4 +-- .../examples/modules/complex/gizmo/impl.cpp | 2 +- .../examples/modules/complex/gizmo/impl.hpp | 4 +-- .../modules/complex/summation/impl.cpp | 2 +- .../modules/complex/summation/impl.hpp | 4 +-- src/viam/sdk/robot/client.cpp | 27 ++++++++++--------- src/viam/sdk/robot/client.hpp | 21 ++++++++------- src/viam/sdk/services/motion/server.cpp | 4 +++ src/viam/sdk/spatialmath/orientation.cpp | 2 +- src/viam/sdk/spatialmath/orientation.hpp | 2 +- 11 files changed, 42 insertions(+), 32 deletions(-) diff --git a/src/viam/examples/modules/complex/base/impl.cpp b/src/viam/examples/modules/complex/base/impl.cpp index 0a2517cfe..d31a3ea92 100644 --- a/src/viam/examples/modules/complex/base/impl.cpp +++ b/src/viam/examples/modules/complex/base/impl.cpp @@ -34,7 +34,7 @@ std::string find_motor(ResourceConfig cfg, std::string motor_name) { return *motor_string; } -void MyBase::reconfigure(Dependencies deps, ResourceConfig cfg) { +void MyBase::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) { // Downcast `left` and `right` dependencies to motors. auto left = find_motor(cfg, "left"); auto right = find_motor(cfg, "right"); diff --git a/src/viam/examples/modules/complex/base/impl.hpp b/src/viam/examples/modules/complex/base/impl.hpp index 378b25469..7b390e0d8 100644 --- a/src/viam/examples/modules/complex/base/impl.hpp +++ b/src/viam/examples/modules/complex/base/impl.hpp @@ -13,10 +13,10 @@ using namespace viam::sdk; // specifies a static `validate` method that checks config validity. class MyBase : public Base { public: - MyBase(Dependencies deps, ResourceConfig cfg) : Base(cfg.name()) { + MyBase(const Dependencies& deps, const ResourceConfig& cfg) : Base(cfg.name()) { this->reconfigure(deps, cfg); }; - void reconfigure(Dependencies deps, ResourceConfig cfg) override; + void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) override; static std::vector validate(ResourceConfig cfg); bool is_moving() override; diff --git a/src/viam/examples/modules/complex/gizmo/impl.cpp b/src/viam/examples/modules/complex/gizmo/impl.cpp index da0128f50..01a1e7522 100644 --- a/src/viam/examples/modules/complex/gizmo/impl.cpp +++ b/src/viam/examples/modules/complex/gizmo/impl.cpp @@ -31,7 +31,7 @@ std::string find_arg1(ResourceConfig cfg) { return *arg1_string; } -void MyGizmo::reconfigure(Dependencies deps, ResourceConfig cfg) { +void MyGizmo::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) { arg1_ = find_arg1(cfg); } diff --git a/src/viam/examples/modules/complex/gizmo/impl.hpp b/src/viam/examples/modules/complex/gizmo/impl.hpp index 3791ededc..b284deb1b 100644 --- a/src/viam/examples/modules/complex/gizmo/impl.hpp +++ b/src/viam/examples/modules/complex/gizmo/impl.hpp @@ -14,10 +14,10 @@ using namespace viam::sdk; class MyGizmo : public Gizmo { public: MyGizmo(std::string name, std::string arg1) : Gizmo(std::move(name)), arg1_(std::move(arg1)){}; - MyGizmo(Dependencies deps, ResourceConfig cfg) : Gizmo(cfg.name()) { + MyGizmo(const Dependencies& deps, const ResourceConfig& cfg) : Gizmo(cfg.name()) { this->reconfigure(deps, cfg); }; - void reconfigure(Dependencies deps, ResourceConfig cfg) override; + void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) override; static std::vector validate(ResourceConfig cfg); bool do_one(std::string arg1) override; diff --git a/src/viam/examples/modules/complex/summation/impl.cpp b/src/viam/examples/modules/complex/summation/impl.cpp index a76eb9a4a..534f4b074 100644 --- a/src/viam/examples/modules/complex/summation/impl.cpp +++ b/src/viam/examples/modules/complex/summation/impl.cpp @@ -24,7 +24,7 @@ bool find_subtract(ResourceConfig cfg) { return *subtract_bool; } -void MySummation::reconfigure(Dependencies deps, ResourceConfig cfg) { +void MySummation::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) { subtract_ = find_subtract(cfg); } diff --git a/src/viam/examples/modules/complex/summation/impl.hpp b/src/viam/examples/modules/complex/summation/impl.hpp index cdef63153..00d4bfba1 100644 --- a/src/viam/examples/modules/complex/summation/impl.hpp +++ b/src/viam/examples/modules/complex/summation/impl.hpp @@ -12,10 +12,10 @@ class MySummation : public Summation { public: MySummation(std::string name, bool subtract) : Summation(std::move(name)), subtract_(subtract){}; - MySummation(Dependencies deps, ResourceConfig cfg) : Summation(cfg.name()) { + MySummation(const Dependencies& deps, const ResourceConfig& cfg) : Summation(cfg.name()) { this->reconfigure(deps, cfg); }; - void reconfigure(Dependencies deps, ResourceConfig cfg) override; + void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) override; static std::vector validate(ResourceConfig cfg); double sum(std::vector numbers) override; diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index 06533fa61..388c28e01 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -247,7 +247,8 @@ std::shared_ptr RobotClient::with_channel(const std::shared_ptr RobotClient::at_address(std::string address, Options options) { +std::shared_ptr RobotClient::at_address(const std::string& address, + const Options& options) { const char* uri = address.c_str(); auto channel = ViamChannel::dial(uri, options.dial_options()); std::shared_ptr robot = RobotClient::with_channel(channel, options); @@ -256,7 +257,8 @@ std::shared_ptr RobotClient::at_address(std::string address, Option return robot; }; -std::shared_ptr RobotClient::at_local_socket(std::string address, Options options) { +std::shared_ptr RobotClient::at_local_socket(const std::string& address, + const Options& options) { const std::string addr = "unix://" + address; const char* uri = addr.c_str(); const std::shared_ptr channel = @@ -271,7 +273,7 @@ std::shared_ptr RobotClient::at_local_socket(std::string address, O }; std::vector RobotClient::get_frame_system_config( - std::vector additional_transforms) { + const std::vector& additional_transforms) { viam::robot::v1::FrameSystemConfigRequest req; viam::robot::v1::FrameSystemConfigResponse resp; ClientContext ctx; @@ -300,13 +302,13 @@ std::vector RobotClient::get_frame_system_config( PoseInFrame RobotClient::transform_pose(PoseInFrame query, std::string destination, - std::vector additional_transforms) { + const std::vector& additional_transforms) { viam::robot::v1::TransformPoseRequest req; viam::robot::v1::TransformPoseResponse resp; ClientContext ctx; - *req.mutable_source() = query; - *req.mutable_destination() = destination; + *req.mutable_source() = std::move(query); + *req.mutable_destination() = std::move(destination); RepeatedPtrField* req_transforms = req.mutable_supplemental_transforms(); for (const Transform& transform : additional_transforms) { @@ -321,7 +323,8 @@ PoseInFrame RobotClient::transform_pose(PoseInFrame query, return resp.pose(); } -std::vector RobotClient::discover_components(std::vector queries) { +std::vector RobotClient::discover_components( + const std::vector& queries) { viam::robot::v1::DiscoverComponentsRequest req; viam::robot::v1::DiscoverComponentsResponse resp; ClientContext ctx; @@ -364,16 +367,16 @@ void RobotClient::stop_all() { } void RobotClient::stop_all( - std::unordered_map>, - ResourceNameHasher, - ResourceNameEqual> extra) { + const std::unordered_map>, + ResourceNameHasher, + ResourceNameEqual>& extra) { viam::robot::v1::StopAllRequest req; viam::robot::v1::StopAllResponse resp; ClientContext ctx; RepeatedPtrField* ep = req.mutable_extra(); - for (auto& xtra : extra) { + for (const auto& xtra : extra) { const ResourceName name = xtra.first; const std::shared_ptr>> params = std::make_shared>>( diff --git a/src/viam/sdk/robot/client.hpp b/src/viam/sdk/robot/client.hpp index f92f85592..02a5f893e 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -51,14 +51,16 @@ class RobotClient { /// @brief Create a robot client connected to the robot at the provided address. /// @param address The address of the robot (IP address, URI, URL, etc.) /// @param options Options for connecting and refreshing. - static std::shared_ptr at_address(std::string address, Options options); + static std::shared_ptr at_address(const std::string& address, + const Options& options); /// @brief Creates a robot client connected to the robot at the provided local socket. /// @param address The local socket of the robot (a .sock file, etc.). /// @param options Options for connecting and refreshing. /// Creates a direct connection to the robot using the `unix://` scheme. /// Only useful for connecting to robots across Unix sockets. - static std::shared_ptr at_local_socket(std::string address, Options options); + static std::shared_ptr at_local_socket(const std::string& address, + const Options& options); /// @brief Creates a robot client connected to the provided channel. /// @param channel The channel to connect with. @@ -103,7 +105,7 @@ class RobotClient { /// @brief Get the configuration of the frame system of the given robot. /// @return The configuration of the calling robot's frame system. std::vector get_frame_system_config( - std::vector additional_transforms = std::vector()); + const std::vector& additional_transforms = std::vector()); /// @brief Get the list of operations currently running on a robot. /// @return The list of operations currently running on the calling robot. @@ -119,7 +121,7 @@ class RobotClient { std::vector get_status(); std::vector discover_components( - std::vector queries); + const std::vector& queries); /// @brief Transform a given `Pose` to a new specified destination which is a reference frame. /// @param query The pose that should be transformed. @@ -128,7 +130,7 @@ class RobotClient { viam::common::v1::PoseInFrame transform_pose( viam::common::v1::PoseInFrame query, std::string destination, - std::vector additional_transforms = std::vector()); + const std::vector& additional_transforms = std::vector()); /// @brief Blocks on the specified operation of the robot, returning when it is complete. /// @param id The ID of the operation to block on. @@ -139,10 +141,11 @@ class RobotClient { /// @brief Cancel all operations for the robot and stop all actuators and movement. /// @param extra Any extra params to pass to resources' `stop` methods, keyed by `ResourceName`. - void stop_all(std::unordered_map>, - ResourceNameHasher, - ResourceNameEqual> extra); + void stop_all( + const std::unordered_map>, + ResourceNameHasher, + ResourceNameEqual>& extra); /// @brief Cancel a specified operation on the robot. /// @param id The ID of the operation to cancel. diff --git a/src/viam/sdk/services/motion/server.cpp b/src/viam/sdk/services/motion/server.cpp index 94edd590b..fa8b0f72a 100644 --- a/src/viam/sdk/services/motion/server.cpp +++ b/src/viam/sdk/services/motion/server.cpp @@ -99,6 +99,9 @@ ::grpc::Status MotionServer::MoveOnGlobe( }); } +// CR erodkin: no lint here only because we already fixed this in the other clang-tidy PR. +// Just do that one first, delete this, then rebase. +// NOLINTBEGIN ::grpc::Status MotionServer::GetPose( ::grpc::ServerContext* context, const ::viam::service::motion::v1::GetPoseRequest* request, @@ -135,6 +138,7 @@ ::grpc::Status MotionServer::GetPose( return ::grpc::Status(); }; +// NOLINTEND ::grpc::Status MotionServer::GetPlan( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/spatialmath/orientation.cpp b/src/viam/sdk/spatialmath/orientation.cpp index ca5229d18..96240a8fe 100644 --- a/src/viam/sdk/spatialmath/orientation.cpp +++ b/src/viam/sdk/spatialmath/orientation.cpp @@ -25,7 +25,7 @@ OrientationConfig::OrientationConfig() { orientation_ = quat; } -OrientationConfig OrientationConfig::from_proto(proto::Orientation proto) { +OrientationConfig OrientationConfig::from_proto(const proto::Orientation& proto) { OrientationConfig cfg; switch (proto.type_case()) { case proto::Orientation::TypeCase::kAxisAngles: { diff --git a/src/viam/sdk/spatialmath/orientation.hpp b/src/viam/sdk/spatialmath/orientation.hpp index ea2247870..68d6c20f9 100644 --- a/src/viam/sdk/spatialmath/orientation.hpp +++ b/src/viam/sdk/spatialmath/orientation.hpp @@ -19,7 +19,7 @@ typedef boost:: class OrientationConfig { public: viam::app::v1::Orientation to_proto() const; - static OrientationConfig from_proto(viam::app::v1::Orientation proto); + static OrientationConfig from_proto(const viam::app::v1::Orientation& proto); OrientationConfig(OrientationType type_, std::vector value, orientation orientation) From 18e42708ea6bc813c32f78e4239c9a42f6f33eb1 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 13 Feb 2024 14:27:39 -0500 Subject: [PATCH 4/7] to_proto methods also --- src/viam/sdk/common/linear_algebra.cpp | 8 ++++---- src/viam/sdk/common/linear_algebra.hpp | 3 ++- src/viam/sdk/common/world_state.hpp | 2 +- src/viam/sdk/components/camera/camera.cpp | 4 ++-- src/viam/sdk/components/camera/camera.hpp | 4 ++-- src/viam/sdk/components/encoder/encoder.cpp | 5 +++-- src/viam/sdk/components/encoder/encoder.hpp | 5 +++-- src/viam/sdk/components/motor/motor.cpp | 6 +++--- src/viam/sdk/components/motor/motor.hpp | 7 +++---- .../movement_sensor/movement_sensor.cpp | 15 ++++++++++++--- .../movement_sensor/movement_sensor.hpp | 9 +++++---- .../sdk/components/power_sensor/power_sensor.cpp | 4 ++-- .../sdk/components/power_sensor/power_sensor.hpp | 4 ++-- 13 files changed, 44 insertions(+), 32 deletions(-) diff --git a/src/viam/sdk/common/linear_algebra.cpp b/src/viam/sdk/common/linear_algebra.cpp index 1ec094ec2..afd7c5b8c 100644 --- a/src/viam/sdk/common/linear_algebra.cpp +++ b/src/viam/sdk/common/linear_algebra.cpp @@ -45,11 +45,11 @@ std::array& Vector3::data() { return this->data_; } -viam::common::v1::Vector3 Vector3::to_proto(const Vector3& vec) { +viam::common::v1::Vector3 Vector3::to_proto() const { viam::common::v1::Vector3 result; - result.set_x(vec.x()); - result.set_y(vec.y()); - result.set_z(vec.z()); + result.set_x(x()); + result.set_y(y()); + result.set_z(z()); return result; }; diff --git a/src/viam/sdk/common/linear_algebra.hpp b/src/viam/sdk/common/linear_algebra.hpp index 3afe00cad..eb78e9a69 100644 --- a/src/viam/sdk/common/linear_algebra.hpp +++ b/src/viam/sdk/common/linear_algebra.hpp @@ -30,7 +30,8 @@ class Vector3 { const std::array& data() const; std::array& data(); - static viam::common::v1::Vector3 to_proto(const Vector3& vec); + // CR erodkin: this was static but there doesn't seem to be a good reason for that + viam::common::v1::Vector3 to_proto() const; static Vector3 from_proto(const viam::common::v1::Vector3& vec); private: diff --git a/src/viam/sdk/common/world_state.hpp b/src/viam/sdk/common/world_state.hpp index 72f229537..3cc582a3c 100644 --- a/src/viam/sdk/common/world_state.hpp +++ b/src/viam/sdk/common/world_state.hpp @@ -33,7 +33,7 @@ class WorldState { WorldState() {} WorldState(std::vector obstacles, std::vector transforms) - : obstacles_(obstacles), transforms_(transforms) {} + : obstacles_(std::move(obstacles)), transforms_(std::move(transforms)) {} friend bool operator==(const WorldState& lhs, const WorldState& rhs); friend bool operator==(const geometries_in_frame& lhs, const geometries_in_frame& rhs); diff --git a/src/viam/sdk/components/camera/camera.cpp b/src/viam/sdk/components/camera/camera.cpp index 14789f426..ecec5be54 100644 --- a/src/viam/sdk/components/camera/camera.cpp +++ b/src/viam/sdk/components/camera/camera.cpp @@ -149,7 +149,7 @@ Camera::properties Camera::from_proto( } viam::component::camera::v1::IntrinsicParameters Camera::to_proto( - Camera::intrinsic_parameters params) { + const Camera::intrinsic_parameters& params) { viam::component::camera::v1::IntrinsicParameters proto; proto.set_width_px(params.width_px); proto.set_height_px(params.height_px); @@ -160,7 +160,7 @@ viam::component::camera::v1::IntrinsicParameters Camera::to_proto( return proto; } viam::component::camera::v1::DistortionParameters Camera::to_proto( - Camera::distortion_parameters params) { + const Camera::distortion_parameters& params) { viam::component::camera::v1::DistortionParameters proto; *proto.mutable_model() = params.model; *proto.mutable_parameters() = {params.parameters.begin(), params.parameters.end()}; diff --git a/src/viam/sdk/components/camera/camera.hpp b/src/viam/sdk/components/camera/camera.hpp index b5794dcbf..3efbb190f 100644 --- a/src/viam/sdk/components/camera/camera.hpp +++ b/src/viam/sdk/components/camera/camera.hpp @@ -114,10 +114,10 @@ class Camera : public Component { static properties from_proto(const viam::component::camera::v1::GetPropertiesResponse& proto); /// @brief converts a `distortion_parameters` struct to its proto representation. - static viam::component::camera::v1::DistortionParameters to_proto(distortion_parameters); + static viam::component::camera::v1::DistortionParameters to_proto(const distortion_parameters&); /// @brief converts an `intrinsic_parameters` struct to its proto representation. - static viam::component::camera::v1::IntrinsicParameters to_proto(intrinsic_parameters); + static viam::component::camera::v1::IntrinsicParameters to_proto(const intrinsic_parameters&); /// @brief Send/receive arbitrary commands to the resource. /// @param Command the command to execute. diff --git a/src/viam/sdk/components/encoder/encoder.cpp b/src/viam/sdk/components/encoder/encoder.cpp index e5d92a318..237769360 100644 --- a/src/viam/sdk/components/encoder/encoder.cpp +++ b/src/viam/sdk/components/encoder/encoder.cpp @@ -71,7 +71,7 @@ viam::component::encoder::v1::PositionType Encoder::to_proto(position_type posit } } -viam::component::encoder::v1::GetPositionResponse Encoder::to_proto(position position) { +viam::component::encoder::v1::GetPositionResponse Encoder::to_proto(const position& position) { viam::component::encoder::v1::GetPositionResponse proto; proto.set_value(position.value); @@ -79,7 +79,8 @@ viam::component::encoder::v1::GetPositionResponse Encoder::to_proto(position pos return proto; } -viam::component::encoder::v1::GetPropertiesResponse Encoder::to_proto(properties properties) { +viam::component::encoder::v1::GetPropertiesResponse Encoder::to_proto( + const properties& properties) { viam::component::encoder::v1::GetPropertiesResponse proto; proto.set_ticks_count_supported(properties.ticks_count_supported); diff --git a/src/viam/sdk/components/encoder/encoder.hpp b/src/viam/sdk/components/encoder/encoder.hpp index 27a934ac7..82ee2f195 100644 --- a/src/viam/sdk/components/encoder/encoder.hpp +++ b/src/viam/sdk/components/encoder/encoder.hpp @@ -59,10 +59,11 @@ class Encoder : public Component { static viam::component::encoder::v1::PositionType to_proto(position_type position_type); /// @brief Converts a `position` struct to its proto representation. - static viam::component::encoder::v1::GetPositionResponse to_proto(position position); + static viam::component::encoder::v1::GetPositionResponse to_proto(const position& position); /// @brief Converts a `properties` struct to its proto representation. - static viam::component::encoder::v1::GetPropertiesResponse to_proto(properties properties); + static viam::component::encoder::v1::GetPropertiesResponse to_proto( + const properties& properties); /// @brief Returns position of the encoder which can either be ticks since last zeroing for an /// incremental encoder or degrees for an absolute encoder. diff --git a/src/viam/sdk/components/motor/motor.cpp b/src/viam/sdk/components/motor/motor.cpp index 978c0be1e..34264eec7 100644 --- a/src/viam/sdk/components/motor/motor.cpp +++ b/src/viam/sdk/components/motor/motor.cpp @@ -37,13 +37,13 @@ Motor::properties Motor::from_proto( return properties; } -viam::component::motor::v1::GetPositionResponse Motor::to_proto(position position) { +viam::component::motor::v1::GetPositionResponse Motor::to_proto(const position& position) { viam::component::motor::v1::GetPositionResponse proto; proto.set_position(position); return proto; } -viam::component::motor::v1::IsPoweredResponse Motor::to_proto(power_status power_status) { +viam::component::motor::v1::IsPoweredResponse Motor::to_proto(const power_status& power_status) { viam::component::motor::v1::IsPoweredResponse proto; proto.set_is_on(power_status.is_on); @@ -51,7 +51,7 @@ viam::component::motor::v1::IsPoweredResponse Motor::to_proto(power_status power return proto; } -viam::component::motor::v1::GetPropertiesResponse Motor::to_proto(properties properties) { +viam::component::motor::v1::GetPropertiesResponse Motor::to_proto(const properties& properties) { viam::component::motor::v1::GetPropertiesResponse proto; proto.set_position_reporting(properties.position_reporting); return proto; diff --git a/src/viam/sdk/components/motor/motor.hpp b/src/viam/sdk/components/motor/motor.hpp index 39284b4dd..b83723d0e 100644 --- a/src/viam/sdk/components/motor/motor.hpp +++ b/src/viam/sdk/components/motor/motor.hpp @@ -54,15 +54,14 @@ class Motor : public Component, public Stoppable { /// @brief Creates a `properties` struct from its proto representation. static properties from_proto(const viam::component::motor::v1::GetPropertiesResponse& proto); - /// // CR erodkin: why aren't the `to_proto` methods complaining about lack of `const&`? /// @brief Converts a `position` struct to its proto representation. - static viam::component::motor::v1::GetPositionResponse to_proto(position position); + static viam::component::motor::v1::GetPositionResponse to_proto(const position& position); /// @brief Converts a `power_status` struct to its proto representation. - static viam::component::motor::v1::IsPoweredResponse to_proto(power_status power_status); + static viam::component::motor::v1::IsPoweredResponse to_proto(const power_status& power_status); /// @brief Converts a `properties` struct to its proto representation. - static viam::component::motor::v1::GetPropertiesResponse to_proto(properties properties); + static viam::component::motor::v1::GetPropertiesResponse to_proto(const properties& properties); /// @brief Sets the percentage of the motor's total power that should be employed. /// @param power_pct Percentage of motor's power, between -1 and 1, where negative values diff --git a/src/viam/sdk/components/movement_sensor/movement_sensor.cpp b/src/viam/sdk/components/movement_sensor/movement_sensor.cpp index a802ca819..0abb535f5 100644 --- a/src/viam/sdk/components/movement_sensor/movement_sensor.cpp +++ b/src/viam/sdk/components/movement_sensor/movement_sensor.cpp @@ -56,13 +56,13 @@ MovementSensor::properties MovementSensor::from_proto( } viam::component::movementsensor::v1::GetCompassHeadingResponse MovementSensor::to_proto( - compassheading compassheading) { + const compassheading& compassheading) { viam::component::movementsensor::v1::GetCompassHeadingResponse proto; proto.set_value(compassheading.value); return proto; } -viam::common::v1::Orientation MovementSensor::to_proto(orientation orientation) { +viam::common::v1::Orientation MovementSensor::to_proto(const orientation& orientation) { viam::common::v1::Orientation proto; proto.set_o_x(orientation.o_x); proto.set_o_y(orientation.o_y); @@ -71,8 +71,17 @@ viam::common::v1::Orientation MovementSensor::to_proto(orientation orientation) return proto; } +// CR erodkin: somehow this wasn't defined! gotta add it +viam::component::movementsensor::v1::GetPositionResponse MovementSensor::to_proto( + const position& position) { + component::movementsensor::v1::GetPositionResponse proto; + proto.set_altitude_m(position.altitude_m); + *proto.mutable_coordinate() = position.coordinate.to_proto(); + return proto; +} + viam::component::movementsensor::v1::GetPropertiesResponse MovementSensor::to_proto( - properties properties) { + const properties& properties) { viam::component::movementsensor::v1::GetPropertiesResponse proto; proto.set_linear_velocity_supported(properties.linear_velocity_supported); proto.set_angular_velocity_supported(properties.angular_velocity_supported); diff --git a/src/viam/sdk/components/movement_sensor/movement_sensor.hpp b/src/viam/sdk/components/movement_sensor/movement_sensor.hpp index d24d82ea1..cb2839abd 100644 --- a/src/viam/sdk/components/movement_sensor/movement_sensor.hpp +++ b/src/viam/sdk/components/movement_sensor/movement_sensor.hpp @@ -72,17 +72,18 @@ class MovementSensor : public Component { /// @brief Converts a `compassheading` struct to its proto representation. static viam::component::movementsensor::v1::GetCompassHeadingResponse to_proto( - compassheading compassheading); + const compassheading& compassheading); /// @brief Converts a `position` struct to its proto representation. - static viam::component::movementsensor::v1::GetPositionResponse to_proto(position position); + static viam::component::movementsensor::v1::GetPositionResponse to_proto( + const position& position); /// @brief Converts an `orientation` struct to its proto representation. - static viam::common::v1::Orientation to_proto(orientation orientation); + static viam::common::v1::Orientation to_proto(const orientation& orientation); /// @brief Converts a `properties` struct to its proto representation. static viam::component::movementsensor::v1::GetPropertiesResponse to_proto( - properties properties); + const properties& properties); inline Vector3 get_linear_velocity() { return get_linear_velocity({}); diff --git a/src/viam/sdk/components/power_sensor/power_sensor.cpp b/src/viam/sdk/components/power_sensor/power_sensor.cpp index bb142a7be..c7e48c5f4 100644 --- a/src/viam/sdk/components/power_sensor/power_sensor.cpp +++ b/src/viam/sdk/components/power_sensor/power_sensor.cpp @@ -33,14 +33,14 @@ PowerSensor::current PowerSensor::from_proto(const GetCurrentResponse& proto) { return c; } -GetVoltageResponse PowerSensor::to_proto(voltage v) { +GetVoltageResponse PowerSensor::to_proto(const voltage& v) { GetVoltageResponse proto; proto.set_volts(v.volts); proto.set_is_ac(v.is_ac); return proto; } -GetCurrentResponse PowerSensor::to_proto(current c) { +GetCurrentResponse PowerSensor::to_proto(const current& c) { GetCurrentResponse proto; proto.set_amperes(c.amperes); proto.set_is_ac(c.is_ac); diff --git a/src/viam/sdk/components/power_sensor/power_sensor.hpp b/src/viam/sdk/components/power_sensor/power_sensor.hpp index f77f531f1..92039387d 100644 --- a/src/viam/sdk/components/power_sensor/power_sensor.hpp +++ b/src/viam/sdk/components/power_sensor/power_sensor.hpp @@ -50,10 +50,10 @@ class PowerSensor : public Component { static current from_proto(const GetCurrentResponse& proto); /// @brief Converts a `voltage` struct to its proto representation. - static GetVoltageResponse to_proto(voltage v); + static GetVoltageResponse to_proto(const voltage& v); /// @brief Converts a `current` struct to its proto representation. - static GetCurrentResponse to_proto(current c); + static GetCurrentResponse to_proto(const current& c); /// @brief Returns the voltage reading of this sensor. /// @return The voltage reading of this sensor. From f7123390c9fef82b7a184ee134676e435515e263 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 13 Feb 2024 14:40:28 -0500 Subject: [PATCH 5/7] lingering Vector3::to_proto fixes --- src/viam/sdk/components/base/client.cpp | 8 ++++---- src/viam/sdk/components/movement_sensor/server.cpp | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/viam/sdk/components/base/client.cpp b/src/viam/sdk/components/base/client.cpp index 1b076a402..41fc21935 100644 --- a/src/viam/sdk/components/base/client.cpp +++ b/src/viam/sdk/components/base/client.cpp @@ -52,8 +52,8 @@ void BaseClient::set_power(const Vector3& linear, return make_client_helper(this, *stub_, &StubType::SetPower) .with(extra, [&](auto& request) { - *request.mutable_linear() = Vector3::to_proto(linear); - *request.mutable_angular() = Vector3::to_proto(angular); + *request.mutable_linear() = linear.to_proto(); + *request.mutable_angular() = angular.to_proto(); }) .invoke(); } @@ -64,8 +64,8 @@ void BaseClient::set_velocity(const Vector3& linear, return make_client_helper(this, *stub_, &StubType::SetVelocity) .with(extra, [&](auto& request) { - *request.mutable_linear() = Vector3::to_proto(linear); - *request.mutable_angular() = Vector3::to_proto(angular); + *request.mutable_linear() = linear.to_proto(); + *request.mutable_angular() = angular.to_proto(); }) .invoke(); } diff --git a/src/viam/sdk/components/movement_sensor/server.cpp b/src/viam/sdk/components/movement_sensor/server.cpp index a38de44e9..62d496523 100644 --- a/src/viam/sdk/components/movement_sensor/server.cpp +++ b/src/viam/sdk/components/movement_sensor/server.cpp @@ -23,7 +23,7 @@ ::grpc::Status MovementSensorServer::GetLinearVelocity( this, request)([&](auto& helper, auto& movementsensor) { const Vector3 result = movementsensor->get_linear_velocity(helper.getExtra()); - *response->mutable_linear_velocity() = Vector3::to_proto(result); + *response->mutable_linear_velocity() = result.to_proto(); }); } @@ -35,7 +35,7 @@ ::grpc::Status MovementSensorServer::GetAngularVelocity( this, request)([&](auto& helper, auto& movementsensor) { const Vector3 result = movementsensor->get_angular_velocity(helper.getExtra()); - *response->mutable_angular_velocity() = Vector3::to_proto(result); + *response->mutable_angular_velocity() = result.to_proto(); }); } @@ -111,7 +111,7 @@ ::grpc::Status MovementSensorServer::GetLinearAcceleration( this, request)([&](auto& helper, auto& movementsensor) { const Vector3 result = movementsensor->get_linear_acceleration(helper.getExtra()); - *response->mutable_linear_acceleration() = Vector3::to_proto(result); + *response->mutable_linear_acceleration() = result.to_proto(); }); } From f08eb4f2b2f7b9775ce3fa75096fc9f606f6bcdf Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 14 Feb 2024 12:08:40 -0500 Subject: [PATCH 6/7] remove CR comments --- src/viam/sdk/common/linear_algebra.hpp | 1 - src/viam/sdk/components/movement_sensor/movement_sensor.cpp | 1 - 2 files changed, 2 deletions(-) diff --git a/src/viam/sdk/common/linear_algebra.hpp b/src/viam/sdk/common/linear_algebra.hpp index eb78e9a69..6af24fe1b 100644 --- a/src/viam/sdk/common/linear_algebra.hpp +++ b/src/viam/sdk/common/linear_algebra.hpp @@ -30,7 +30,6 @@ class Vector3 { const std::array& data() const; std::array& data(); - // CR erodkin: this was static but there doesn't seem to be a good reason for that viam::common::v1::Vector3 to_proto() const; static Vector3 from_proto(const viam::common::v1::Vector3& vec); diff --git a/src/viam/sdk/components/movement_sensor/movement_sensor.cpp b/src/viam/sdk/components/movement_sensor/movement_sensor.cpp index 0abb535f5..141c1d74f 100644 --- a/src/viam/sdk/components/movement_sensor/movement_sensor.cpp +++ b/src/viam/sdk/components/movement_sensor/movement_sensor.cpp @@ -71,7 +71,6 @@ viam::common::v1::Orientation MovementSensor::to_proto(const orientation& orient return proto; } -// CR erodkin: somehow this wasn't defined! gotta add it viam::component::movementsensor::v1::GetPositionResponse MovementSensor::to_proto( const position& position) { component::movementsensor::v1::GetPositionResponse proto; From c7cf6fdd782339dcda8201c09813de00c7358be2 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Fri, 16 Feb 2024 10:09:00 -0500 Subject: [PATCH 7/7] pr comments, lingering fixes --- src/viam/sdk/config/resource.cpp | 2 +- src/viam/sdk/config/resource.hpp | 2 +- src/viam/sdk/module/handler_map.cpp | 4 ++-- src/viam/sdk/module/handler_map.hpp | 2 +- src/viam/sdk/module/service.cpp | 12 ++++++------ src/viam/sdk/module/service.hpp | 6 ++---- src/viam/sdk/robot/client.cpp | 8 ++++---- src/viam/sdk/robot/client.hpp | 2 +- 8 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/viam/sdk/config/resource.cpp b/src/viam/sdk/config/resource.cpp index 277a3cb48..5f82e8753 100644 --- a/src/viam/sdk/config/resource.cpp +++ b/src/viam/sdk/config/resource.cpp @@ -138,7 +138,7 @@ viam::app::v1::ComponentConfig ResourceConfig::to_proto() const { return proto_cfg; } -ResourceConfig::ResourceConfig(const std::string& type) : api_({kRDK, type, ""}), type_(type){}; +ResourceConfig::ResourceConfig(std::string type) : api_({kRDK, type, ""}), type_(std::move(type)){}; } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/config/resource.hpp b/src/viam/sdk/config/resource.hpp index 50ae69845..7730f2ca0 100644 --- a/src/viam/sdk/config/resource.hpp +++ b/src/viam/sdk/config/resource.hpp @@ -24,7 +24,7 @@ class ResourceConfig { public: static ResourceConfig from_proto(const viam::app::v1::ComponentConfig& proto_cfg); viam::app::v1::ComponentConfig to_proto() const; - ResourceConfig(const std::string& type); + ResourceConfig(std::string type); Name resource_name(); const API& api() const; const LinkConfig& frame() const; diff --git a/src/viam/sdk/module/handler_map.cpp b/src/viam/sdk/module/handler_map.cpp index 1eb9f644b..d6e5fe0ed 100644 --- a/src/viam/sdk/module/handler_map.cpp +++ b/src/viam/sdk/module/handler_map.cpp @@ -65,8 +65,8 @@ const HandlerMap_ HandlerMap_::from_proto(const viam::module::v1::HandlerMap& pr return hm; } -void HandlerMap_::add_model(const Model& model, const RPCSubtype& subtype) { - handles_[subtype].push_back(model); +void HandlerMap_::add_model(Model model, const RPCSubtype& subtype) { + handles_[subtype].push_back(std::move(model)); } std::ostream& operator<<(std::ostream& os, const HandlerMap_& hm) { diff --git a/src/viam/sdk/module/handler_map.hpp b/src/viam/sdk/module/handler_map.hpp index d15bcf5d3..ad83db867 100644 --- a/src/viam/sdk/module/handler_map.hpp +++ b/src/viam/sdk/module/handler_map.hpp @@ -10,7 +10,7 @@ namespace sdk { class HandlerMap_ { public: HandlerMap_(); - void add_model(const Model& model, const RPCSubtype& subtype); + void add_model(Model model, const RPCSubtype& subtype); viam::module::v1::HandlerMap to_proto() const; static const HandlerMap_ from_proto(const viam::module::v1::HandlerMap& proto); diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index af5ea0b56..c2f9d0d52 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -265,8 +265,8 @@ ModuleService::~ModuleService() { } } -void ModuleService::add_model_from_registry_inlock_(const API& api, - const Model& model, +void ModuleService::add_model_from_registry_inlock_(API api, + Model model, const std::lock_guard& lock) { const std::shared_ptr creator = Registry::lookup_resource_server(api); @@ -276,13 +276,13 @@ void ModuleService::add_model_from_registry_inlock_(const API& api, name = creator->service_descriptor()->full_name(); sd = creator->service_descriptor(); } - const RPCSubtype rpc_subtype(api, name, *sd); - module_->mutable_handles().add_model(model, rpc_subtype); + const RPCSubtype rpc_subtype(std::move(api), name, *sd); + module_->mutable_handles().add_model(std::move(model), rpc_subtype); }; -void ModuleService::add_model_from_registry(const API& api, const Model& model) { +void ModuleService::add_model_from_registry(API api, Model model) { const std::lock_guard lock(lock_); - return add_model_from_registry_inlock_(api, model, lock); + return add_model_from_registry_inlock_(std::move(api), std::move(model), lock); } } // namespace sdk diff --git a/src/viam/sdk/module/service.hpp b/src/viam/sdk/module/service.hpp index 86b772214..4925e279c 100644 --- a/src/viam/sdk/module/service.hpp +++ b/src/viam/sdk/module/service.hpp @@ -47,7 +47,7 @@ class ModuleService : viam::module::v1::ModuleService::Service { /// of ModelRegistration, the passed in models will already be registered and added. /// @param api The API to add. /// @param model The model to add. - void add_model_from_registry(const API& api, const Model& model); + void add_model_from_registry(API api, Model model); private: ::grpc::Status AddResource(::grpc::ServerContext* context, @@ -71,9 +71,7 @@ class ModuleService : viam::module::v1::ModuleService::Service { const ::viam::module::v1::ValidateConfigRequest* request, ::viam::module::v1::ValidateConfigResponse* response) override; - void add_model_from_registry_inlock_(const API& api, - const Model& model, - const std::lock_guard&); + void add_model_from_registry_inlock_(API api, Model model, const std::lock_guard&); Dependencies get_dependencies_(google::protobuf::RepeatedPtrField const& proto, std::string const& resource_name); std::shared_ptr get_parent_resource_(const Name& name); diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index 5935d1528..2d2b564f0 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -318,9 +318,9 @@ std::vector RobotClient::resource_names() const { return resource_names_; } -std::shared_ptr RobotClient::with_channel(const std::shared_ptr& channel, +std::shared_ptr RobotClient::with_channel(std::shared_ptr channel, const Options& options) { - std::shared_ptr robot = std::make_shared(channel); + std::shared_ptr robot = std::make_shared(std::move(channel)); robot->refresh_interval_ = options.refresh_interval(); robot->should_refresh_ = (robot->refresh_interval_ > 0); if (robot->should_refresh_) { @@ -460,8 +460,8 @@ void RobotClient::stop_all(const std::unordered_map& extra) RepeatedPtrField* ep = req.mutable_extra(); for (const auto& xtra : extra) { - const Name name = xtra.first; - const AttributeMap params = xtra.second; + const Name& name = xtra.first; + const AttributeMap& params = xtra.second; const google::protobuf::Struct s = map_to_struct(params); viam::robot::v1::StopExtraParameters stop; *stop.mutable_name() = name.to_proto(); diff --git a/src/viam/sdk/robot/client.hpp b/src/viam/sdk/robot/client.hpp index 08a4dc887..55088db5c 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -96,7 +96,7 @@ class RobotClient { /// @param options Options for connecting and refreshing. /// Connects directly to a pre-existing channel. A robot created this way must be /// `close()`d manually. - static std::shared_ptr with_channel(const std::shared_ptr& channel, + static std::shared_ptr with_channel(std::shared_ptr channel, const Options& options); RobotClient(std::shared_ptr channel); std::vector resource_names() const;