Type conversions fixes (#901)

* Fix type conversions

Signed-off-by: Monika Idzik <monika.idzik@apex.ai>

* Add static_casts

Signed-off-by: Monika Idzik <monika.idzik@apex.ai>

* Address PR comments

Signed-off-by: Monika Idzik <monika.idzik@apex.ai>

* Remove one time use variable

Signed-off-by: Monika Idzik <monika.idzik@apex.ai>

rclcpp/src/rclcpp/duration.cpp

@@ -55,7 +55,8 @@ Duration::Duration(const Duration & rhs)
 Duration::Duration(
   const builtin_interfaces::msg::Duration & duration_msg)
 {
-  rcl_duration_.nanoseconds = RCL_S_TO_NS(static_cast<uint64_t>(duration_msg.sec));
+  rcl_duration_.nanoseconds =
+    static_cast<rcl_duration_value_t>(RCL_S_TO_NS(duration_msg.sec));
   rcl_duration_.nanoseconds += duration_msg.nanosec;
 }
 
@@ -122,15 +123,15 @@ Duration::operator>(const rclcpp::Duration & rhs) const
 void
 bounds_check_duration_sum(int64_t lhsns, int64_t rhsns, uint64_t max)
 {
-  auto abs_lhs = (uint64_t)std::abs(lhsns);
-  auto abs_rhs = (uint64_t)std::abs(rhsns);
+  auto abs_lhs = static_cast<uint64_t>(std::abs(lhsns));
+  auto abs_rhs = static_cast<uint64_t>(std::abs(rhsns));
 
   if (lhsns > 0 && rhsns > 0) {
-    if (abs_lhs + abs_rhs > (uint64_t) max) {
+    if (abs_lhs + abs_rhs > max) {
       throw std::overflow_error("addition leads to int64_t overflow");
     }
   } else if (lhsns < 0 && rhsns < 0) {
-    if (abs_lhs + abs_rhs > (uint64_t) max) {
+    if (abs_lhs + abs_rhs > max) {
       throw std::underflow_error("addition leads to int64_t underflow");
     }
   }
@@ -150,15 +151,15 @@ Duration::operator+(const rclcpp::Duration & rhs) const
 void
 bounds_check_duration_difference(int64_t lhsns, int64_t rhsns, uint64_t max)
 {
-  auto abs_lhs = (uint64_t)std::abs(lhsns);
-  auto abs_rhs = (uint64_t)std::abs(rhsns);
+  auto abs_lhs = static_cast<uint64_t>(std::abs(lhsns));
+  auto abs_rhs = static_cast<uint64_t>(std::abs(rhsns));
 
   if (lhsns > 0 && rhsns < 0) {
-    if (abs_lhs + abs_rhs > (uint64_t) max) {
+    if (abs_lhs + abs_rhs > max) {
       throw std::overflow_error("duration subtraction leads to int64_t overflow");
     }
   } else if (lhsns < 0 && rhsns > 0) {
-    if (abs_lhs + abs_rhs > (uint64_t) max) {
+    if (abs_lhs + abs_rhs > max) {
       throw std::underflow_error("duration subtraction leads to int64_t underflow");
     }
   }
@@ -181,8 +182,9 @@ bounds_check_duration_scale(int64_t dns, double scale, uint64_t max)
 {
   auto abs_dns = static_cast<uint64_t>(std::abs(dns));
   auto abs_scale = std::abs(scale);
-
-  if (abs_scale > 1.0 && abs_dns > static_cast<uint64_t>(max / abs_scale)) {
+  if (abs_scale > 1.0 &&
+    abs_dns > static_cast<uint64_t>(static_cast<long double>(max) / static_cast<long double>(abs_scale)))
+  {
     if ((dns > 0 && scale > 0) || (dns < 0 && scale < 0)) {
       throw std::overflow_error("duration scaling leads to int64_t overflow");
     } else {
@@ -201,7 +203,9 @@ Duration::operator*(double scale) const
     this->rcl_duration_.nanoseconds,
     scale,
     std::numeric_limits<rcl_duration_value_t>::max());
-  return Duration(static_cast<rcl_duration_value_t>(rcl_duration_.nanoseconds * scale));
+  long double scale_ld = static_cast<long double>(scale);
+  return Duration(static_cast<rcl_duration_value_t>(
+             static_cast<long double>(rcl_duration_.nanoseconds) * scale_ld));
 }
 
 rcl_duration_value_t
@@ -228,8 +232,8 @@ Duration::to_rmw_time() const
   // reuse conversion logic from msg creation
   builtin_interfaces::msg::Duration msg = *this;
   rmw_time_t result;
-  result.sec = msg.sec;
-  result.nsec = msg.nanosec;
+  result.sec = static_cast<uint64_t>(msg.sec);
+  result.nsec = static_cast<uint64_t>(msg.nanosec);
   return result;
 }
 

rclcpp/src/rclcpp/parameter_client.cpp

@@ -152,7 +152,7 @@ AsyncParametersClient::get_parameters(
       auto & pvalues = cb_f.get()->values;
 
       for (auto & pvalue : pvalues) {
-        auto i = &pvalue - &pvalues[0];
+        auto i = static_cast<size_t>(&pvalue - &pvalues[0]);
         rcl_interfaces::msg::Parameter parameter;
         parameter.name = request->names[i];
         parameter.value = pvalue;

rclcpp/src/rclcpp/parameter_value.cpp

@@ -154,7 +154,7 @@ ParameterValue::ParameterValue(const int64_t int_value)
 
 ParameterValue::ParameterValue(const float double_value)
 {
-  value_.double_value = double_value;
+  value_.double_value = static_cast<double>(double_value);
   value_.type = rcl_interfaces::msg::ParameterType::PARAMETER_DOUBLE;
 }
 

rclcpp/src/rclcpp/utilities.cpp

@@ -103,9 +103,9 @@ remove_ros_arguments(int argc, char const * const argv[])
     throw exceptions::RCLError(base_exc, "");
   }
 
-  std::vector<std::string> return_arguments(nonros_argc);
+  std::vector<std::string> return_arguments(static_cast<size_t>(nonros_argc));
 
-  for (int ii = 0; ii < nonros_argc; ++ii) {
+  for (size_t ii = 0; ii < static_cast<size_t>(nonros_argc); ++ii) {
     return_arguments[ii] = std::string(nonros_argv[ii]);
   }
 

rclcpp/test/test_duration.cpp

@@ -47,15 +47,15 @@ TEST(TestDuration, operators) {
   EXPECT_FALSE(young == old);
 
   rclcpp::Duration add = old + young;
-  EXPECT_EQ(add.nanoseconds(), (rcl_duration_value_t)(old.nanoseconds() + young.nanoseconds()));
+  EXPECT_EQ(add.nanoseconds(), old.nanoseconds() + young.nanoseconds());
   EXPECT_EQ(add, old + young);
 
   rclcpp::Duration sub = young - old;
-  EXPECT_EQ(sub.nanoseconds(), (rcl_duration_value_t)(young.nanoseconds() - old.nanoseconds()));
+  EXPECT_EQ(sub.nanoseconds(), young.nanoseconds() - old.nanoseconds());
   EXPECT_EQ(sub, young - old);
 
   rclcpp::Duration scale = old * 3;
-  EXPECT_EQ(scale.nanoseconds(), (rcl_duration_value_t)(old.nanoseconds() * 3));
+  EXPECT_EQ(scale.nanoseconds(), old.nanoseconds() * 3);
 
   rclcpp::Duration time = rclcpp::Duration(0, 0);
   rclcpp::Duration copy_constructor_duration(time);

rclcpp/test/test_time.cpp

@@ -123,7 +123,7 @@ TEST(TestTime, operators) {
   EXPECT_TRUE(young != old);
 
   rclcpp::Duration sub = young - old;
-  EXPECT_EQ(sub.nanoseconds(), (rcl_duration_value_t)(young.nanoseconds() - old.nanoseconds()));
+  EXPECT_EQ(sub.nanoseconds(), (young.nanoseconds() - old.nanoseconds()));
   EXPECT_EQ(sub, young - old);
 
   rclcpp::Time young_changed(young);

rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp

@@ -224,7 +224,7 @@ public:
         resp->success = false;
         return;
       }
-      transition_id = rcl_transition->id;
+      transition_id = static_cast<std::uint8_t>(rcl_transition->id);
     }
 
     node_interfaces::LifecycleNodeInterface::CallbackReturn cb_return_code;
@@ -289,11 +289,11 @@ public:
     for (uint8_t i = 0; i < state_machine_.current_state->valid_transition_size; ++i) {
       auto rcl_transition = state_machine_.current_state->valid_transitions[i];
       lifecycle_msgs::msg::TransitionDescription trans_desc;
-      trans_desc.transition.id = rcl_transition.id;
+      trans_desc.transition.id = static_cast<uint8_t>(rcl_transition.id);
       trans_desc.transition.label = rcl_transition.label;
-      trans_desc.start_state.id = rcl_transition.start->id;
+      trans_desc.start_state.id = static_cast<uint8_t>(rcl_transition.start->id);
       trans_desc.start_state.label = rcl_transition.start->label;
-      trans_desc.goal_state.id = rcl_transition.goal->id;
+      trans_desc.goal_state.id = static_cast<uint8_t>(rcl_transition.goal->id);
       trans_desc.goal_state.label = rcl_transition.goal->label;
       resp->available_transitions.push_back(trans_desc);
     }
@@ -315,11 +315,11 @@ public:
     for (uint8_t i = 0; i < state_machine_.transition_map.transitions_size; ++i) {
       auto rcl_transition = state_machine_.transition_map.transitions[i];
       lifecycle_msgs::msg::TransitionDescription trans_desc;
-      trans_desc.transition.id = rcl_transition.id;
+      trans_desc.transition.id = static_cast<uint8_t>(rcl_transition.id);
       trans_desc.transition.label = rcl_transition.label;
-      trans_desc.start_state.id = rcl_transition.start->id;
+      trans_desc.start_state.id = static_cast<uint8_t>(rcl_transition.start->id);
       trans_desc.start_state.label = rcl_transition.start->label;
-      trans_desc.goal_state.id = rcl_transition.goal->id;
+      trans_desc.goal_state.id = static_cast<uint8_t>(rcl_transition.goal->id);
       trans_desc.goal_state.label = rcl_transition.goal->label;
       resp->available_transitions.push_back(trans_desc);
     }
@@ -434,7 +434,7 @@ public:
     // in case no callback was attached, we forward directly
     auto cb_success = node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS;
 
-    auto it = cb_map_.find(cb_id);
+    auto it = cb_map_.find(static_cast<uint8_t>(cb_id));
     if (it != cb_map_.end()) {
       auto callback = it->second;
       try {
@@ -460,7 +460,7 @@ public:
     auto transition =
       rcl_lifecycle_get_transition_by_label(state_machine_.current_state, transition_label);
     if (transition) {
-      change_state(transition->id, cb_return_code);
+      change_state(static_cast<uint8_t>(transition->id), cb_return_code);
     }
     return get_current_state();
   }

rclcpp_lifecycle/src/state.cpp

@@ -130,7 +130,7 @@ State::id() const
   if (!state_handle_) {
     throw std::runtime_error("Error in state! Internal state_handle is NULL.");
   }
-  return state_handle_->id;
+  return static_cast<uint8_t>(state_handle_->id);
 }
 
 std::string

rclcpp_lifecycle/src/transition.cpp

@@ -213,7 +213,7 @@ Transition::id() const
   if (!transition_handle_) {
     throw std::runtime_error("internal transition_handle is null");
   }
-  return transition_handle_->id;
+  return static_cast<uint8_t>(transition_handle_->id);
 }
 
 std::string