diff --git a/.clang-tidy b/.clang-tidy index 3f2373919..23c1ab74b 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. # 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. # readability-function-cognitive-complexity: No, complexity is subjective and sometimes necessary. @@ -27,7 +26,6 @@ Checks: > -readability-named-parameter, -readability-implicit-bool-conversion, -readability-magic-numbers, - -performance-unnecessary-value-param, -misc-unused-parameters, -misc-include-cleaner, -readability-function-cognitive-complexity, 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/common/linear_algebra.cpp b/src/viam/sdk/common/linear_algebra.cpp index da6a64760..afd7c5b8c 100644 --- a/src/viam/sdk/common/linear_algebra.cpp +++ b/src/viam/sdk/common/linear_algebra.cpp @@ -45,15 +45,15 @@ 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; }; -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..6af24fe1b 100644 --- a/src/viam/sdk/common/linear_algebra.hpp +++ b/src/viam/sdk/common/linear_algebra.hpp @@ -30,8 +30,8 @@ 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); + viam::common::v1::Vector3 to_proto() const; + 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 a37ed05b6..c801e921c 100644 --- a/src/viam/sdk/common/proto_type.cpp +++ b/src/viam/sdk/common/proto_type.cpp @@ -20,7 +20,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; @@ -37,7 +37,7 @@ Struct map_to_struct(AttributeMap dict) { } // NOLINTNEXTLINE(misc-no-recursion) -AttributeMap struct_to_map(Struct struct_) { +AttributeMap struct_to_map(const Struct& struct_) { AttributeMap map = std::make_shared>>(); diff --git a/src/viam/sdk/common/proto_type.hpp b/src/viam/sdk/common/proto_type.hpp index 3e1e2e180..41872920a 100644 --- a/src/viam/sdk/common/proto_type.hpp +++ b/src/viam/sdk/common/proto_type.hpp @@ -83,8 +83,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/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/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/board/board.cpp b/src/viam/sdk/components/board/board.cpp index 3abbf46fd..3ad2907b9 100644 --- a/src/viam/sdk/components/board/board.cpp +++ b/src/viam/sdk/components/board/board.cpp @@ -20,7 +20,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()); @@ -31,11 +31,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(); } @@ -55,7 +55,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); diff --git a/src/viam/sdk/components/camera/camera.cpp b/src/viam/sdk/components/camera/camera.cpp index b4bc4ac57..ecec5be54 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; @@ -145,7 +149,7 @@ Camera::properties Camera::from_proto(viam::component::camera::v1::GetProperties } 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); @@ -156,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 6dd4eb5e3..3efbb190f 100644 --- a/src/viam/sdk/components/camera/camera.hpp +++ b/src/viam/sdk/components/camera/camera.hpp @@ -90,32 +90,34 @@ 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); + 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 a155976d0..c3575ddf5 100644 --- a/src/viam/sdk/components/encoder/encoder.cpp +++ b/src/viam/sdk/components/encoder/encoder.cpp @@ -38,7 +38,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(); @@ -46,7 +47,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(); @@ -72,7 +74,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); @@ -80,7 +82,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 85ed6c450..82ee2f195 100644 --- a/src/viam/sdk/components/encoder/encoder.hpp +++ b/src/viam/sdk/components/encoder/encoder.hpp @@ -50,19 +50,20 @@ 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); /// @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 7e87cc25b..34264eec7 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,19 +30,20 @@ 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; } -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); @@ -50,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 fcc27d8c4..4626e1cd9 100644 --- a/src/viam/sdk/components/motor/motor.hpp +++ b/src/viam/sdk/components/motor/motor.hpp @@ -46,22 +46,22 @@ 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); /// @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 f8e5655a1..141c1d74f 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(); @@ -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,16 @@ viam::common::v1::Orientation MovementSensor::to_proto(orientation orientation) return proto; } +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 bd22c2ab1..cb2839abd 100644 --- a/src/viam/sdk/components/movement_sensor/movement_sensor.hpp +++ b/src/viam/sdk/components/movement_sensor/movement_sensor.hpp @@ -57,30 +57,33 @@ 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( - 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/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(); }); } diff --git a/src/viam/sdk/components/power_sensor/power_sensor.cpp b/src/viam/sdk/components/power_sensor/power_sensor.cpp index 137c9a7e2..c7e48c5f4 100644 --- a/src/viam/sdk/components/power_sensor/power_sensor.cpp +++ b/src/viam/sdk/components/power_sensor/power_sensor.cpp @@ -19,28 +19,28 @@ 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(); 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 b2052b7af..92039387d 100644 --- a/src/viam/sdk/components/power_sensor/power_sensor.hpp +++ b/src/viam/sdk/components/power_sensor/power_sensor.hpp @@ -44,16 +44,16 @@ 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); + 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. 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 2a8652f8a..c903ad095 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 15dd21493..5f82e8753 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(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 801e737cc..7730f2ca0 100644 --- a/src/viam/sdk/config/resource.hpp +++ b/src/viam/sdk/config/resource.hpp @@ -22,7 +22,7 @@ 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); Name resource_name(); diff --git a/src/viam/sdk/module/handler_map.cpp b/src/viam/sdk/module/handler_map.cpp index 2067b1957..d6e5fe0ed 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,8 +65,8 @@ const HandlerMap_ HandlerMap_::from_proto(viam::module::v1::HandlerMap proto) { return hm; } -void HandlerMap_::add_model(Model model, 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 704c64f69..ad83db867 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(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 8d2a996e2..c2f9d0d52 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -62,7 +62,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}); } @@ -216,7 +216,7 @@ ModuleService::ModuleService(std::string addr) ModuleService::ModuleService(int argc, char** argv, - std::vector> registrations) { + const std::vector>& registrations) { if (argc < 2) { throw Exception("Need socket path as command line argument"); } @@ -276,13 +276,13 @@ void ModuleService::add_model_from_registry_inlock_(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(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 077bb4616..4925e279c 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 @@ -74,7 +74,7 @@ class ModuleService : viam::module::v1::ModuleService::Service { 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_(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 1d05d2a4d..8e09d773a 100644 --- a/src/viam/sdk/resource/resource.cpp +++ b/src/viam/sdk/resource/resource.cpp @@ -21,7 +21,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 24f8f55f1..4924b1bbe 100644 --- a/src/viam/sdk/resource/resource_api.cpp +++ b/src/viam/sdk/resource/resource_api.cpp @@ -32,7 +32,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_; @@ -72,10 +72,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"); @@ -164,7 +165,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(); @@ -217,7 +218,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 4c999fe4e..2d2b564f0 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 @@ -169,7 +163,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() { @@ -314,8 +308,8 @@ void RobotClient::refresh_every() { }; RobotClient::RobotClient(std::shared_ptr channel) - : viam_channel_(channel), - channel_(channel->channel()), + : channel_(channel->channel()), + viam_channel_(std::move(channel)), should_close_channel_(false), impl_(std::make_unique(RobotService::NewStub(channel_))) {} @@ -325,8 +319,8 @@ std::vector RobotClient::resource_names() const { } std::shared_ptr RobotClient::with_channel(std::shared_ptr channel, - Options options) { - std::shared_ptr robot = std::make_shared(channel); + const Options& options) { + 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_) { @@ -343,7 +337,8 @@ std::shared_ptr RobotClient::with_channel(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); @@ -352,7 +347,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 = @@ -365,7 +361,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; @@ -393,15 +389,15 @@ std::vector RobotClient::get_frame_system_conf } pose_in_frame RobotClient::transform_pose( - pose_in_frame query, + const pose_in_frame& 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.to_proto(); - *req.mutable_destination() = destination; + *req.mutable_destination() = std::move(destination); RepeatedPtrField* req_transforms = req.mutable_supplemental_transforms(); for (const WorldState::transform& transform : additional_transforms) { @@ -417,7 +413,7 @@ pose_in_frame RobotClient::transform_pose( } std::vector RobotClient::discover_components( - std::vector queries) { + const std::vector& queries) { viam::robot::v1::DiscoverComponentsRequest req; viam::robot::v1::DiscoverComponentsResponse resp; ClientContext ctx; @@ -457,15 +453,15 @@ void RobotClient::stop_all() { stop_all(map); } -void RobotClient::stop_all(std::unordered_map extra) { +void RobotClient::stop_all(const std::unordered_map& extra) { viam::robot::v1::StopAllRequest req; viam::robot::v1::StopAllResponse resp; ClientContext ctx; RepeatedPtrField* ep = req.mutable_extra(); - for (auto& xtra : extra) { - const Name name = xtra.first; - const AttributeMap params = xtra.second; + for (const auto& xtra : extra) { + 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 5c2ab2023..55088db5c 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -80,14 +80,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. @@ -95,7 +97,7 @@ class RobotClient { /// 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); + const Options& options); RobotClient(std::shared_ptr channel); std::vector resource_names() const; @@ -124,7 +126,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 = {}); + const std::vector& additional_transforms = {}); /// @brief Get the list of operations currently running on a robot. /// @return The list of operations currently running on the calling robot. @@ -139,15 +141,16 @@ class RobotClient { /// @return A list of statuses. std::vector get_status(); - std::vector discover_components(std::vector queries); + std::vector discover_components(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. /// @param destination The name of the reference frame to transform the given pose to. /// @return the `pose_in_frame` of the transformed pose. - pose_in_frame transform_pose(pose_in_frame query, - std::string destination, - std::vector additional_transforms = {}); + pose_in_frame transform_pose( + const pose_in_frame& query, + std::string destination, + const std::vector& additional_transforms = {}); /// @brief Blocks on the specified operation of the robot, returning when it is complete. /// @param id The ID of the operation to block on. @@ -158,7 +161,7 @@ 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 `Name`. - void stop_all(std::unordered_map extra); + void stop_all(const std::unordered_map& extra); /// @brief Cancel a specified operation on the robot. /// @param id The ID of the operation to cancel. @@ -168,8 +171,8 @@ class RobotClient { std::vector> threads_; std::atomic should_refresh_; unsigned int refresh_interval_; - std::shared_ptr viam_channel_; std::shared_ptr channel_; + std::shared_ptr viam_channel_; bool should_close_channel_; struct impl; std::unique_ptr impl_; diff --git a/src/viam/sdk/robot/service.cpp b/src/viam/sdk/robot/service.cpp index 23aaa600f..bf6b2c5b0 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 Name& 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; @@ -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) { @@ -203,7 +204,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 34ac1c24f..56cdd44e2 100644 --- a/src/viam/sdk/rpc/dial.cpp +++ b/src/viam/sdk/rpc/dial.cpp @@ -48,16 +48,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) { @@ -85,7 +85,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 ff755fe50..449075704 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -62,7 +62,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 Exception("Cannot add a listening port after server has started"); @@ -72,7 +72,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 2f453bb4a..6d9c8ba7b 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -51,7 +51,7 @@ class Server { /// @param address The address to listen at. /// @param creds The server credentials; defaults to a insecure server credentials. /// @throws `Exception` 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. diff --git a/src/viam/sdk/spatialmath/orientation.cpp b/src/viam/sdk/spatialmath/orientation.cpp index 7351c6029..009508b52 100644 --- a/src/viam/sdk/spatialmath/orientation.cpp +++ b/src/viam/sdk/spatialmath/orientation.cpp @@ -26,7 +26,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)