From 5d59225ab2107d0c10aa37d255a4156ed3887a55 Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Sat, 6 Apr 2024 08:17:34 +1300 Subject: [PATCH] action: return invalid argument on wrong index (#2271) This catches the user error trying to set actuator 0 when it should start at 1. And also adds a note in the docstring about it. Signed-off-by: Julian Oes --- proto | 2 +- src/mavsdk/plugins/action/action.cpp | 2 + src/mavsdk/plugins/action/action_impl.cpp | 7 + .../action/include/plugins/action/action.h | 5 + .../src/generated/action/action.grpc.pb.h | 6 + .../src/generated/action/action.pb.cc | 139 +++++++++--------- .../src/generated/action/action.pb.h | 8 +- .../src/plugins/action/action_service_impl.h | 4 + 8 files changed, 100 insertions(+), 73 deletions(-) diff --git a/proto b/proto index e37a279c42..c61e60d48c 160000 --- a/proto +++ b/proto @@ -1 +1 @@ -Subproject commit e37a279c42f2e8b139b97bdbd37644d275d4997e +Subproject commit c61e60d48c1148cdb09b2256e1943d5d10640606 diff --git a/src/mavsdk/plugins/action/action.cpp b/src/mavsdk/plugins/action/action.cpp index ac6b9993ba..344aacd768 100644 --- a/src/mavsdk/plugins/action/action.cpp +++ b/src/mavsdk/plugins/action/action.cpp @@ -297,6 +297,8 @@ std::ostream& operator<<(std::ostream& str, Action::Result const& result) return str << "Unsupported"; case Action::Result::Failed: return str << "Failed"; + case Action::Result::InvalidArgument: + return str << "Invalid Argument"; default: return str << "Unknown"; } diff --git a/src/mavsdk/plugins/action/action_impl.cpp b/src/mavsdk/plugins/action/action_impl.cpp index e75dc7ec6d..bf0a14f90f 100644 --- a/src/mavsdk/plugins/action/action_impl.cpp +++ b/src/mavsdk/plugins/action/action_impl.cpp @@ -555,6 +555,13 @@ void ActionImpl::set_actuator_async( break; } command.params.maybe_param7 = static_cast(zero_based_index) / 6.0f; + } else { + if (callback) { + _system_impl->call_user_callback([temp_callback = callback]() { + temp_callback(Action::Result::InvalidArgument); + }); + } + return; } } diff --git a/src/mavsdk/plugins/action/include/plugins/action/action.h b/src/mavsdk/plugins/action/include/plugins/action/action.h index 1a67023077..bd545e9bfb 100644 --- a/src/mavsdk/plugins/action/include/plugins/action/action.h +++ b/src/mavsdk/plugins/action/include/plugins/action/action.h @@ -97,6 +97,7 @@ class Action : public PluginBase { ParameterError, /**< @brief Error getting or setting parameter. */ Unsupported, /**< @brief Action not supported. */ Failed, /**< @brief Action failed. */ + InvalidArgument, /**< @brief Invalid argument. */ }; /** @@ -411,6 +412,8 @@ class Action : public PluginBase { /** * @brief Send command to set the value of an actuator. * + * Note that the index of the actuator starts at 1 and that the value goes from -1 to 1. + * * This function is non-blocking. See 'set_actuator' for the blocking counterpart. */ void set_actuator_async(int32_t index, float value, const ResultCallback callback); @@ -418,6 +421,8 @@ class Action : public PluginBase { /** * @brief Send command to set the value of an actuator. * + * Note that the index of the actuator starts at 1 and that the value goes from -1 to 1. + * * This function is blocking. See 'set_actuator_async' for the non-blocking counterpart. * * @return Result of request. diff --git a/src/mavsdk_server/src/generated/action/action.grpc.pb.h b/src/mavsdk_server/src/generated/action/action.grpc.pb.h index 045fd344e5..050542786f 100644 --- a/src/mavsdk_server/src/generated/action/action.grpc.pb.h +++ b/src/mavsdk_server/src/generated/action/action.grpc.pb.h @@ -189,6 +189,8 @@ class ActionService final { } // // Send command to set the value of an actuator. + // + // Note that the index of the actuator starts at 1 and that the value goes from -1 to 1. virtual ::grpc::Status SetActuator(::grpc::ClientContext* context, const ::mavsdk::rpc::action::SetActuatorRequest& request, ::mavsdk::rpc::action::SetActuatorResponse* response) = 0; std::unique_ptr< ::grpc::ClientAsyncResponseReaderInterface< ::mavsdk::rpc::action::SetActuatorResponse>> AsyncSetActuator(::grpc::ClientContext* context, const ::mavsdk::rpc::action::SetActuatorRequest& request, ::grpc::CompletionQueue* cq) { return std::unique_ptr< ::grpc::ClientAsyncResponseReaderInterface< ::mavsdk::rpc::action::SetActuatorResponse>>(AsyncSetActuatorRaw(context, request, cq)); @@ -382,6 +384,8 @@ class ActionService final { virtual void Hold(::grpc::ClientContext* context, const ::mavsdk::rpc::action::HoldRequest* request, ::mavsdk::rpc::action::HoldResponse* response, ::grpc::ClientUnaryReactor* reactor) = 0; // // Send command to set the value of an actuator. + // + // Note that the index of the actuator starts at 1 and that the value goes from -1 to 1. virtual void SetActuator(::grpc::ClientContext* context, const ::mavsdk::rpc::action::SetActuatorRequest* request, ::mavsdk::rpc::action::SetActuatorResponse* response, std::function) = 0; virtual void SetActuator(::grpc::ClientContext* context, const ::mavsdk::rpc::action::SetActuatorRequest* request, ::mavsdk::rpc::action::SetActuatorResponse* response, ::grpc::ClientUnaryReactor* reactor) = 0; // @@ -848,6 +852,8 @@ class ActionService final { virtual ::grpc::Status Hold(::grpc::ServerContext* context, const ::mavsdk::rpc::action::HoldRequest* request, ::mavsdk::rpc::action::HoldResponse* response); // // Send command to set the value of an actuator. + // + // Note that the index of the actuator starts at 1 and that the value goes from -1 to 1. virtual ::grpc::Status SetActuator(::grpc::ServerContext* context, const ::mavsdk::rpc::action::SetActuatorRequest* request, ::mavsdk::rpc::action::SetActuatorResponse* response); // // Send command to transition the drone to fixedwing. diff --git a/src/mavsdk_server/src/generated/action/action.pb.cc b/src/mavsdk_server/src/generated/action/action.pb.cc index d73d1cfdff..e5751d80d1 100644 --- a/src/mavsdk_server/src/generated/action/action.pb.cc +++ b/src/mavsdk_server/src/generated/action/action.pb.cc @@ -1394,9 +1394,9 @@ const char descriptor_table_protodef_action_2faction_2eproto[] PROTOBUF_SECTION_ "SetCurrentSpeedRequest\022\021\n\tspeed_m_s\030\001 \001(" "\002\"Q\n\027SetCurrentSpeedResponse\0226\n\raction_r" "esult\030\001 \001(\0132\037.mavsdk.rpc.action.ActionRe" - "sult\"\360\003\n\014ActionResult\0226\n\006result\030\001 \001(\0162&." + "sult\"\215\004\n\014ActionResult\0226\n\006result\030\001 \001(\0162&." "mavsdk.rpc.action.ActionResult.Result\022\022\n" - "\nresult_str\030\002 \001(\t\"\223\003\n\006Result\022\022\n\016RESULT_U" + "\nresult_str\030\002 \001(\t\"\260\003\n\006Result\022\022\n\016RESULT_U" "NKNOWN\020\000\022\022\n\016RESULT_SUCCESS\020\001\022\024\n\020RESULT_N" "O_SYSTEM\020\002\022\033\n\027RESULT_CONNECTION_ERROR\020\003\022" "\017\n\013RESULT_BUSY\020\004\022\031\n\025RESULT_COMMAND_DENIE" @@ -1406,70 +1406,70 @@ const char descriptor_table_protodef_action_2faction_2eproto[] PROTOBUF_SECTION_ "VTOL_TRANSITION_SUPPORT_UNKNOWN\020\t\022%\n!RES" "ULT_NO_VTOL_TRANSITION_SUPPORT\020\n\022\032\n\026RESU" "LT_PARAMETER_ERROR\020\013\022\026\n\022RESULT_UNSUPPORT" - "ED\020\014\022\021\n\rRESULT_FAILED\020\r*\363\001\n\020OrbitYawBeha" - "vior\0222\n.ORBIT_YAW_BEHAVIOR_HOLD_FRONT_TO" - "_CIRCLE_CENTER\020\000\022+\n\'ORBIT_YAW_BEHAVIOR_H" - "OLD_INITIAL_HEADING\020\001\022#\n\037ORBIT_YAW_BEHAV" - "IOR_UNCONTROLLED\020\002\0223\n/ORBIT_YAW_BEHAVIOR" - "_HOLD_FRONT_TANGENT_TO_CIRCLE\020\003\022$\n ORBIT" - "_YAW_BEHAVIOR_RC_CONTROLLED\020\0042\246\021\n\rAction" - "Service\022F\n\003Arm\022\035.mavsdk.rpc.action.ArmRe" - "quest\032\036.mavsdk.rpc.action.ArmResponse\"\000\022" - "O\n\006Disarm\022 .mavsdk.rpc.action.DisarmRequ" - "est\032!.mavsdk.rpc.action.DisarmResponse\"\000" - "\022R\n\007Takeoff\022!.mavsdk.rpc.action.TakeoffR" - "equest\032\".mavsdk.rpc.action.TakeoffRespon" - "se\"\000\022I\n\004Land\022\036.mavsdk.rpc.action.LandReq" - "uest\032\037.mavsdk.rpc.action.LandResponse\"\000\022" - "O\n\006Reboot\022 .mavsdk.rpc.action.RebootRequ" - "est\032!.mavsdk.rpc.action.RebootResponse\"\000" - "\022U\n\010Shutdown\022\".mavsdk.rpc.action.Shutdow" - "nRequest\032#.mavsdk.rpc.action.ShutdownRes" - "ponse\"\000\022X\n\tTerminate\022#.mavsdk.rpc.action" - ".TerminateRequest\032$.mavsdk.rpc.action.Te" - "rminateResponse\"\000\022I\n\004Kill\022\036.mavsdk.rpc.a" - "ction.KillRequest\032\037.mavsdk.rpc.action.Ki" - "llResponse\"\000\022g\n\016ReturnToLaunch\022(.mavsdk." - "rpc.action.ReturnToLaunchRequest\032).mavsd" - "k.rpc.action.ReturnToLaunchResponse\"\000\022a\n" - "\014GotoLocation\022&.mavsdk.rpc.action.GotoLo" - "cationRequest\032\'.mavsdk.rpc.action.GotoLo" - "cationResponse\"\000\022R\n\007DoOrbit\022!.mavsdk.rpc" - ".action.DoOrbitRequest\032\".mavsdk.rpc.acti" - "on.DoOrbitResponse\"\000\022I\n\004Hold\022\036.mavsdk.rp" - "c.action.HoldRequest\032\037.mavsdk.rpc.action" - ".HoldResponse\"\000\022^\n\013SetActuator\022%.mavsdk." - "rpc.action.SetActuatorRequest\032&.mavsdk.r" - "pc.action.SetActuatorResponse\"\000\022|\n\025Trans" - "itionToFixedwing\022/.mavsdk.rpc.action.Tra" - "nsitionToFixedwingRequest\0320.mavsdk.rpc.a" - "ction.TransitionToFixedwingResponse\"\000\022\202\001" - "\n\027TransitionToMulticopter\0221.mavsdk.rpc.a" - "ction.TransitionToMulticopterRequest\0322.m" - "avsdk.rpc.action.TransitionToMulticopter" - "Response\"\000\022s\n\022GetTakeoffAltitude\022,.mavsd" - "k.rpc.action.GetTakeoffAltitudeRequest\032-" - ".mavsdk.rpc.action.GetTakeoffAltitudeRes" - "ponse\"\000\022s\n\022SetTakeoffAltitude\022,.mavsdk.r" - "pc.action.SetTakeoffAltitudeRequest\032-.ma" - "vsdk.rpc.action.SetTakeoffAltitudeRespon" - "se\"\000\022j\n\017GetMaximumSpeed\022).mavsdk.rpc.act" - "ion.GetMaximumSpeedRequest\032*.mavsdk.rpc." - "action.GetMaximumSpeedResponse\"\000\022j\n\017SetM" - "aximumSpeed\022).mavsdk.rpc.action.SetMaxim" - "umSpeedRequest\032*.mavsdk.rpc.action.SetMa" - "ximumSpeedResponse\"\000\022\210\001\n\031GetReturnToLaun" - "chAltitude\0223.mavsdk.rpc.action.GetReturn" - "ToLaunchAltitudeRequest\0324.mavsdk.rpc.act" - "ion.GetReturnToLaunchAltitudeResponse\"\000\022" - "\210\001\n\031SetReturnToLaunchAltitude\0223.mavsdk.r" - "pc.action.SetReturnToLaunchAltitudeReque" - "st\0324.mavsdk.rpc.action.SetReturnToLaunch" - "AltitudeResponse\"\000\022j\n\017SetCurrentSpeed\022)." - "mavsdk.rpc.action.SetCurrentSpeedRequest" - "\032*.mavsdk.rpc.action.SetCurrentSpeedResp" - "onse\"\000B\037\n\020io.mavsdk.actionB\013ActionProtob" - "\006proto3" + "ED\020\014\022\021\n\rRESULT_FAILED\020\r\022\033\n\027RESULT_INVALI" + "D_ARGUMENT\020\016*\363\001\n\020OrbitYawBehavior\0222\n.ORB" + "IT_YAW_BEHAVIOR_HOLD_FRONT_TO_CIRCLE_CEN" + "TER\020\000\022+\n\'ORBIT_YAW_BEHAVIOR_HOLD_INITIAL" + "_HEADING\020\001\022#\n\037ORBIT_YAW_BEHAVIOR_UNCONTR" + "OLLED\020\002\0223\n/ORBIT_YAW_BEHAVIOR_HOLD_FRONT" + "_TANGENT_TO_CIRCLE\020\003\022$\n ORBIT_YAW_BEHAVI" + "OR_RC_CONTROLLED\020\0042\246\021\n\rActionService\022F\n\003" + "Arm\022\035.mavsdk.rpc.action.ArmRequest\032\036.mav" + "sdk.rpc.action.ArmResponse\"\000\022O\n\006Disarm\022 " + ".mavsdk.rpc.action.DisarmRequest\032!.mavsd" + "k.rpc.action.DisarmResponse\"\000\022R\n\007Takeoff" + "\022!.mavsdk.rpc.action.TakeoffRequest\032\".ma" + "vsdk.rpc.action.TakeoffResponse\"\000\022I\n\004Lan" + "d\022\036.mavsdk.rpc.action.LandRequest\032\037.mavs" + "dk.rpc.action.LandResponse\"\000\022O\n\006Reboot\022 " + ".mavsdk.rpc.action.RebootRequest\032!.mavsd" + "k.rpc.action.RebootResponse\"\000\022U\n\010Shutdow" + "n\022\".mavsdk.rpc.action.ShutdownRequest\032#." + "mavsdk.rpc.action.ShutdownResponse\"\000\022X\n\t" + "Terminate\022#.mavsdk.rpc.action.TerminateR" + "equest\032$.mavsdk.rpc.action.TerminateResp" + "onse\"\000\022I\n\004Kill\022\036.mavsdk.rpc.action.KillR" + "equest\032\037.mavsdk.rpc.action.KillResponse\"" + "\000\022g\n\016ReturnToLaunch\022(.mavsdk.rpc.action." + "ReturnToLaunchRequest\032).mavsdk.rpc.actio" + "n.ReturnToLaunchResponse\"\000\022a\n\014GotoLocati" + "on\022&.mavsdk.rpc.action.GotoLocationReque" + "st\032\'.mavsdk.rpc.action.GotoLocationRespo" + "nse\"\000\022R\n\007DoOrbit\022!.mavsdk.rpc.action.DoO" + "rbitRequest\032\".mavsdk.rpc.action.DoOrbitR" + "esponse\"\000\022I\n\004Hold\022\036.mavsdk.rpc.action.Ho" + "ldRequest\032\037.mavsdk.rpc.action.HoldRespon" + "se\"\000\022^\n\013SetActuator\022%.mavsdk.rpc.action." + "SetActuatorRequest\032&.mavsdk.rpc.action.S" + "etActuatorResponse\"\000\022|\n\025TransitionToFixe" + "dwing\022/.mavsdk.rpc.action.TransitionToFi" + "xedwingRequest\0320.mavsdk.rpc.action.Trans" + "itionToFixedwingResponse\"\000\022\202\001\n\027Transitio" + "nToMulticopter\0221.mavsdk.rpc.action.Trans" + "itionToMulticopterRequest\0322.mavsdk.rpc.a" + "ction.TransitionToMulticopterResponse\"\000\022" + "s\n\022GetTakeoffAltitude\022,.mavsdk.rpc.actio" + "n.GetTakeoffAltitudeRequest\032-.mavsdk.rpc" + ".action.GetTakeoffAltitudeResponse\"\000\022s\n\022" + "SetTakeoffAltitude\022,.mavsdk.rpc.action.S" + "etTakeoffAltitudeRequest\032-.mavsdk.rpc.ac" + "tion.SetTakeoffAltitudeResponse\"\000\022j\n\017Get" + "MaximumSpeed\022).mavsdk.rpc.action.GetMaxi" + "mumSpeedRequest\032*.mavsdk.rpc.action.GetM" + "aximumSpeedResponse\"\000\022j\n\017SetMaximumSpeed" + "\022).mavsdk.rpc.action.SetMaximumSpeedRequ" + "est\032*.mavsdk.rpc.action.SetMaximumSpeedR" + "esponse\"\000\022\210\001\n\031GetReturnToLaunchAltitude\022" + "3.mavsdk.rpc.action.GetReturnToLaunchAlt" + "itudeRequest\0324.mavsdk.rpc.action.GetRetu" + "rnToLaunchAltitudeResponse\"\000\022\210\001\n\031SetRetu" + "rnToLaunchAltitude\0223.mavsdk.rpc.action.S" + "etReturnToLaunchAltitudeRequest\0324.mavsdk" + ".rpc.action.SetReturnToLaunchAltitudeRes" + "ponse\"\000\022j\n\017SetCurrentSpeed\022).mavsdk.rpc." + "action.SetCurrentSpeedRequest\032*.mavsdk.r" + "pc.action.SetCurrentSpeedResponse\"\000B\037\n\020i" + "o.mavsdk.actionB\013ActionProtob\006proto3" }; static const ::_pbi::DescriptorTable* const descriptor_table_action_2faction_2eproto_deps[1] = { @@ -1479,7 +1479,7 @@ static ::absl::once_flag descriptor_table_action_2faction_2eproto_once; const ::_pbi::DescriptorTable descriptor_table_action_2faction_2eproto = { false, false, - 5807, + 5836, descriptor_table_protodef_action_2faction_2eproto, "action/action.proto", &descriptor_table_action_2faction_2eproto_once, @@ -1519,9 +1519,9 @@ const ::google::protobuf::EnumDescriptor* ActionResult_Result_descriptor() { return file_level_enum_descriptors_action_2faction_2eproto[0]; } PROTOBUF_CONSTINIT const uint32_t ActionResult_Result_internal_data_[] = { - 917504u, 0u, }; + 983040u, 0u, }; bool ActionResult_Result_IsValid(int value) { - return 0 <= value && value <= 13; + return 0 <= value && value <= 14; } #if (__cplusplus < 201703) && \ (!defined(_MSC_VER) || (_MSC_VER >= 1900 && _MSC_VER < 1912)) @@ -1540,6 +1540,7 @@ constexpr ActionResult_Result ActionResult::RESULT_NO_VTOL_TRANSITION_SUPPORT; constexpr ActionResult_Result ActionResult::RESULT_PARAMETER_ERROR; constexpr ActionResult_Result ActionResult::RESULT_UNSUPPORTED; constexpr ActionResult_Result ActionResult::RESULT_FAILED; +constexpr ActionResult_Result ActionResult::RESULT_INVALID_ARGUMENT; constexpr ActionResult_Result ActionResult::Result_MIN; constexpr ActionResult_Result ActionResult::Result_MAX; constexpr int ActionResult::Result_ARRAYSIZE; diff --git a/src/mavsdk_server/src/generated/action/action.pb.h b/src/mavsdk_server/src/generated/action/action.pb.h index b01d3eb7e3..6895027a85 100644 --- a/src/mavsdk_server/src/generated/action/action.pb.h +++ b/src/mavsdk_server/src/generated/action/action.pb.h @@ -222,6 +222,7 @@ enum ActionResult_Result : int { ActionResult_Result_RESULT_PARAMETER_ERROR = 11, ActionResult_Result_RESULT_UNSUPPORTED = 12, ActionResult_Result_RESULT_FAILED = 13, + ActionResult_Result_RESULT_INVALID_ARGUMENT = 14, ActionResult_Result_ActionResult_Result_INT_MIN_SENTINEL_DO_NOT_USE_ = std::numeric_limits<::int32_t>::min(), ActionResult_Result_ActionResult_Result_INT_MAX_SENTINEL_DO_NOT_USE_ = @@ -231,8 +232,8 @@ enum ActionResult_Result : int { bool ActionResult_Result_IsValid(int value); extern const uint32_t ActionResult_Result_internal_data_[]; constexpr ActionResult_Result ActionResult_Result_Result_MIN = static_cast(0); -constexpr ActionResult_Result ActionResult_Result_Result_MAX = static_cast(13); -constexpr int ActionResult_Result_Result_ARRAYSIZE = 13 + 1; +constexpr ActionResult_Result ActionResult_Result_Result_MAX = static_cast(14); +constexpr int ActionResult_Result_Result_ARRAYSIZE = 14 + 1; const ::google::protobuf::EnumDescriptor* ActionResult_Result_descriptor(); template @@ -245,7 +246,7 @@ const std::string& ActionResult_Result_Name(T value) { template <> inline const std::string& ActionResult_Result_Name(ActionResult_Result value) { return ::google::protobuf::internal::NameOfDenseEnum( + 0, 14>( static_cast(value)); } inline bool ActionResult_Result_Parse(absl::string_view name, ActionResult_Result* value) { @@ -3811,6 +3812,7 @@ class ActionResult final : static constexpr Result RESULT_PARAMETER_ERROR = ActionResult_Result_RESULT_PARAMETER_ERROR; static constexpr Result RESULT_UNSUPPORTED = ActionResult_Result_RESULT_UNSUPPORTED; static constexpr Result RESULT_FAILED = ActionResult_Result_RESULT_FAILED; + static constexpr Result RESULT_INVALID_ARGUMENT = ActionResult_Result_RESULT_INVALID_ARGUMENT; static inline bool Result_IsValid(int value) { return ActionResult_Result_IsValid(value); } diff --git a/src/mavsdk_server/src/plugins/action/action_service_impl.h b/src/mavsdk_server/src/plugins/action/action_service_impl.h index 0d6179cad2..5529c8642c 100644 --- a/src/mavsdk_server/src/plugins/action/action_service_impl.h +++ b/src/mavsdk_server/src/plugins/action/action_service_impl.h @@ -118,6 +118,8 @@ class ActionServiceImpl final : public rpc::action::ActionService::Service { return rpc::action::ActionResult_Result_RESULT_UNSUPPORTED; case mavsdk::Action::Result::Failed: return rpc::action::ActionResult_Result_RESULT_FAILED; + case mavsdk::Action::Result::InvalidArgument: + return rpc::action::ActionResult_Result_RESULT_INVALID_ARGUMENT; } } @@ -156,6 +158,8 @@ class ActionServiceImpl final : public rpc::action::ActionService::Service { return mavsdk::Action::Result::Unsupported; case rpc::action::ActionResult_Result_RESULT_FAILED: return mavsdk::Action::Result::Failed; + case rpc::action::ActionResult_Result_RESULT_INVALID_ARGUMENT: + return mavsdk::Action::Result::InvalidArgument; } }