service: hid: Quick RE fixes and comments

src/core/hle/service/hid/controllers/npad.cpp

@@ -672,6 +672,12 @@ std::size_t Controller_NPad::GetSupportedNpadIdTypesSize() const {
 }
 
 void Controller_NPad::SetHoldType(NpadJoyHoldType joy_hold_type) {
+    if (joy_hold_type != NpadJoyHoldType::Horizontal &&
+        joy_hold_type != NpadJoyHoldType::Vertical) {
+        LOG_ERROR(Service_HID, "Npad joy hold type needs to be valid, joy_hold_type={}",
+                  joy_hold_type);
+        return;
+    }
     hold_type = joy_hold_type;
 }
 
@@ -957,10 +963,10 @@ void Controller_NPad::UpdateControllerAt(Core::HID::NpadStyleIndex type,
     InitNewlyAddedController(npad_id);
 }
 
-void Controller_NPad::DisconnectNpad(Core::HID::NpadIdType npad_id) {
+ResultCode Controller_NPad::DisconnectNpad(Core::HID::NpadIdType npad_id) {
     if (!IsNpadIdValid(npad_id)) {
         LOG_ERROR(Service_HID, "Invalid NpadIdType npad_id:{}", npad_id);
-        return;
+        return InvalidNpadId;
     }
 
     LOG_DEBUG(Service_HID, "Npad disconnected {}", npad_id);
@@ -997,6 +1003,7 @@ void Controller_NPad::DisconnectNpad(Core::HID::NpadIdType npad_id) {
     controller.device->Disconnect();
     SignalStyleSetChangedEvent(npad_id);
     WriteEmptyEntry(shared_memory);
+    return ResultSuccess;
 }
 
 ResultCode Controller_NPad::SetGyroscopeZeroDriftMode(Core::HID::SixAxisSensorHandle sixaxis_handle,
@@ -1349,17 +1356,17 @@ void Controller_NPad::StopLRAssignmentMode() {
     is_in_lr_assignment_mode = false;
 }
 
-bool Controller_NPad::SwapNpadAssignment(Core::HID::NpadIdType npad_id_1,
+ResultCode Controller_NPad::SwapNpadAssignment(Core::HID::NpadIdType npad_id_1,
-                                         Core::HID::NpadIdType npad_id_2) {
+                                               Core::HID::NpadIdType npad_id_2) {
     if (!IsNpadIdValid(npad_id_1) || !IsNpadIdValid(npad_id_2)) {
         LOG_ERROR(Service_HID, "Invalid NpadIdType npad_id_1:{}, npad_id_2:{}", npad_id_1,
                   npad_id_2);
-        return false;
+        return InvalidNpadId;
     }
     if (npad_id_1 == Core::HID::NpadIdType::Handheld ||
         npad_id_2 == Core::HID::NpadIdType::Handheld || npad_id_1 == Core::HID::NpadIdType::Other ||
         npad_id_2 == Core::HID::NpadIdType::Other) {
-        return true;
+        return ResultSuccess;
     }
     const auto& controller_1 = GetControllerFromNpadIdType(npad_id_1).device;
     const auto& controller_2 = GetControllerFromNpadIdType(npad_id_2).device;
@@ -1369,46 +1376,49 @@ bool Controller_NPad::SwapNpadAssignment(Core::HID::NpadIdType npad_id_1,
     const auto is_connected_2 = controller_2->IsConnected();
 
     if (!IsControllerSupported(type_index_1) && is_connected_1) {
-        return false;
+        return NpadNotConnected;
     }
     if (!IsControllerSupported(type_index_2) && is_connected_2) {
-        return false;
+        return NpadNotConnected;
     }
 
     UpdateControllerAt(type_index_2, npad_id_1, is_connected_2);
     UpdateControllerAt(type_index_1, npad_id_2, is_connected_1);
 
-    return true;
+    return ResultSuccess;
 }
 
-Core::HID::LedPattern Controller_NPad::GetLedPattern(Core::HID::NpadIdType npad_id) {
+ResultCode Controller_NPad::GetLedPattern(Core::HID::NpadIdType npad_id,
+                                          Core::HID::LedPattern& pattern) const {
     if (!IsNpadIdValid(npad_id)) {
         LOG_ERROR(Service_HID, "Invalid NpadIdType npad_id:{}", npad_id);
-        return Core::HID::LedPattern{0, 0, 0, 0};
+        return InvalidNpadId;
     }
     const auto& controller = GetControllerFromNpadIdType(npad_id).device;
-    return controller->GetLedPattern();
+    pattern = controller->GetLedPattern();
+    return ResultSuccess;
 }
 
-bool Controller_NPad::IsUnintendedHomeButtonInputProtectionEnabled(
+ResultCode Controller_NPad::IsUnintendedHomeButtonInputProtectionEnabled(
-    Core::HID::NpadIdType npad_id) const {
+    Core::HID::NpadIdType npad_id, bool& is_valid) const {
     if (!IsNpadIdValid(npad_id)) {
         LOG_ERROR(Service_HID, "Invalid NpadIdType npad_id:{}", npad_id);
-        // Return the default value
+        return InvalidNpadId;
-        return false;
     }
     const auto& controller = GetControllerFromNpadIdType(npad_id);
-    return controller.unintended_home_button_input_protection;
+    is_valid = controller.unintended_home_button_input_protection;
+    return ResultSuccess;
 }
 
-void Controller_NPad::SetUnintendedHomeButtonInputProtectionEnabled(bool is_protection_enabled,
+ResultCode Controller_NPad::SetUnintendedHomeButtonInputProtectionEnabled(
-                                                                    Core::HID::NpadIdType npad_id) {
+    bool is_protection_enabled, Core::HID::NpadIdType npad_id) {
     if (!IsNpadIdValid(npad_id)) {
         LOG_ERROR(Service_HID, "Invalid NpadIdType npad_id:{}", npad_id);
-        return;
+        return InvalidNpadId;
     }
     auto& controller = GetControllerFromNpadIdType(npad_id);
     controller.unintended_home_button_input_protection = is_protection_enabled;
+    return ResultSuccess;
 }
 
 void Controller_NPad::SetAnalogStickUseCenterClamp(bool use_center_clamp) {

src/core/hle/service/hid/controllers/npad.h

@@ -141,7 +141,7 @@ public:
     void UpdateControllerAt(Core::HID::NpadStyleIndex controller, Core::HID::NpadIdType npad_id,
                             bool connected);
 
-    void DisconnectNpad(Core::HID::NpadIdType npad_id);
+    ResultCode DisconnectNpad(Core::HID::NpadIdType npad_id);
 
     ResultCode SetGyroscopeZeroDriftMode(Core::HID::SixAxisSensorHandle sixaxis_handle,
                                          GyroscopeZeroDriftMode drift_mode);
@@ -163,10 +163,11 @@ public:
     ResultCode GetSixAxisFusionParameters(
         Core::HID::SixAxisSensorHandle sixaxis_handle,
         Core::HID::SixAxisSensorFusionParameters& parameters) const;
-    Core::HID::LedPattern GetLedPattern(Core::HID::NpadIdType npad_id);
+    ResultCode GetLedPattern(Core::HID::NpadIdType npad_id, Core::HID::LedPattern& pattern) const;
-    bool IsUnintendedHomeButtonInputProtectionEnabled(Core::HID::NpadIdType npad_id) const;
+    ResultCode IsUnintendedHomeButtonInputProtectionEnabled(Core::HID::NpadIdType npad_id,
-    void SetUnintendedHomeButtonInputProtectionEnabled(bool is_protection_enabled,
+                                                            bool& is_enabled) const;
-                                                       Core::HID::NpadIdType npad_id);
+    ResultCode SetUnintendedHomeButtonInputProtectionEnabled(bool is_protection_enabled,
+                                                             Core::HID::NpadIdType npad_id);
     void SetAnalogStickUseCenterClamp(bool use_center_clamp);
     void ClearAllConnectedControllers();
     void DisconnectAllConnectedControllers();
@@ -176,7 +177,7 @@ public:
     void MergeSingleJoyAsDualJoy(Core::HID::NpadIdType npad_id_1, Core::HID::NpadIdType npad_id_2);
     void StartLRAssignmentMode();
     void StopLRAssignmentMode();
-    bool SwapNpadAssignment(Core::HID::NpadIdType npad_id_1, Core::HID::NpadIdType npad_id_2);
+    ResultCode SwapNpadAssignment(Core::HID::NpadIdType npad_id_1, Core::HID::NpadIdType npad_id_2);
 
     // Logical OR for all buttons presses on all controllers
     // Specifically for cheat engine and other features.

src/core/hle/service/hid/errors.h

@@ -9,6 +9,7 @@ namespace Service::HID {
 
 constexpr ResultCode NpadInvalidHandle{ErrorModule::HID, 100};
 constexpr ResultCode InvalidSixAxisFusionRange{ErrorModule::HID, 423};
+constexpr ResultCode InvalidNpadId{ErrorModule::HID, 709};
 constexpr ResultCode NpadNotConnected{ErrorModule::HID, 710};
 
 } // namespace Service::HID

src/core/hle/service/hid/hid.cpp

@@ -694,11 +694,7 @@ void Hid::ResetSixAxisSensorFusionParameters(Kernel::HLERequestContext& ctx) {
         rb.Push(result1);
         return;
     }
-    if (result2.IsError()) {
+    rb.Push(result2);
-        rb.Push(result2);
-        return;
-    }
-    rb.Push(ResultSuccess);
 }
 
 void Hid::SetGyroscopeZeroDriftMode(Kernel::HLERequestContext& ctx) {
@@ -948,27 +944,29 @@ void Hid::DisconnectNpad(Kernel::HLERequestContext& ctx) {
 
     const auto parameters{rp.PopRaw<Parameters>()};
 
-    applet_resource->GetController<Controller_NPad>(HidController::NPad)
+    auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
-        .DisconnectNpad(parameters.npad_id);
+    const auto result = controller.DisconnectNpad(parameters.npad_id);
 
     LOG_DEBUG(Service_HID, "called, npad_id={}, applet_resource_user_id={}", parameters.npad_id,
               parameters.applet_resource_user_id);
 
     IPC::ResponseBuilder rb{ctx, 2};
-    rb.Push(ResultSuccess);
+    rb.Push(result);
 }
 
 void Hid::GetPlayerLedPattern(Kernel::HLERequestContext& ctx) {
     IPC::RequestParser rp{ctx};
     const auto npad_id{rp.PopEnum<Core::HID::NpadIdType>()};
 
+    Core::HID::LedPattern pattern{0, 0, 0, 0};
+    auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
+    const auto result = controller.GetLedPattern(npad_id, pattern);
+
     LOG_DEBUG(Service_HID, "called, npad_id={}", npad_id);
 
     IPC::ResponseBuilder rb{ctx, 4};
-    rb.Push(ResultSuccess);
+    rb.Push(result);
-    rb.Push(applet_resource->GetController<Controller_NPad>(HidController::NPad)
+    rb.Push(pattern.raw);
-                .GetLedPattern(npad_id)
-                .raw);
 }
 
 void Hid::ActivateNpadWithRevision(Kernel::HLERequestContext& ctx) {
@@ -1157,19 +1155,14 @@ void Hid::SwapNpadAssignment(Kernel::HLERequestContext& ctx) {
     const auto npad_id_2{rp.PopEnum<Core::HID::NpadIdType>()};
     const auto applet_resource_user_id{rp.Pop<u64>()};
 
-    const bool res = applet_resource->GetController<Controller_NPad>(HidController::NPad)
+    auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
-                         .SwapNpadAssignment(npad_id_1, npad_id_2);
+    const auto result = controller.SwapNpadAssignment(npad_id_1, npad_id_2);
 
     LOG_DEBUG(Service_HID, "called, npad_id_1={}, npad_id_2={}, applet_resource_user_id={}",
               npad_id_1, npad_id_2, applet_resource_user_id);
 
     IPC::ResponseBuilder rb{ctx, 2};
-    if (res) {
+    rb.Push(result);
-        rb.Push(ResultSuccess);
-    } else {
-        LOG_ERROR(Service_HID, "Npads are not connected!");
-        rb.Push(NpadNotConnected);
-    }
 }
 
 void Hid::IsUnintendedHomeButtonInputProtectionEnabled(Kernel::HLERequestContext& ctx) {
@@ -1183,13 +1176,17 @@ void Hid::IsUnintendedHomeButtonInputProtectionEnabled(Kernel::HLERequestContext
 
     const auto parameters{rp.PopRaw<Parameters>()};
 
+    bool is_enabled = false;
+    auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
+    const auto result =
+        controller.IsUnintendedHomeButtonInputProtectionEnabled(parameters.npad_id, is_enabled);
+
     LOG_WARNING(Service_HID, "(STUBBED) called, npad_id={}, applet_resource_user_id={}",
                 parameters.npad_id, parameters.applet_resource_user_id);
 
     IPC::ResponseBuilder rb{ctx, 3};
-    rb.Push(ResultSuccess);
+    rb.Push(result);
-    rb.Push(applet_resource->GetController<Controller_NPad>(HidController::NPad)
+    rb.Push(is_enabled);
-                .IsUnintendedHomeButtonInputProtectionEnabled(parameters.npad_id));
 }
 
 void Hid::EnableUnintendedHomeButtonInputProtection(Kernel::HLERequestContext& ctx) {
@@ -1204,9 +1201,9 @@ void Hid::EnableUnintendedHomeButtonInputProtection(Kernel::HLERequestContext& c
 
     const auto parameters{rp.PopRaw<Parameters>()};
 
-    applet_resource->GetController<Controller_NPad>(HidController::NPad)
+    auto& controller = GetAppletResource()->GetController<Controller_NPad>(HidController::NPad);
-        .SetUnintendedHomeButtonInputProtectionEnabled(
+    const auto result = controller.SetUnintendedHomeButtonInputProtectionEnabled(
-            parameters.unintended_home_button_input_protection, parameters.npad_id);
+        parameters.unintended_home_button_input_protection, parameters.npad_id);
 
     LOG_WARNING(Service_HID,
                 "(STUBBED) called, unintended_home_button_input_protection={}, npad_id={},"
@@ -1215,7 +1212,7 @@ void Hid::EnableUnintendedHomeButtonInputProtection(Kernel::HLERequestContext& c
                 parameters.applet_resource_user_id);
 
     IPC::ResponseBuilder rb{ctx, 2};
-    rb.Push(ResultSuccess);
+    rb.Push(result);
 }
 
 void Hid::SetNpadAnalogStickUseCenterClamp(Kernel::HLERequestContext& ctx) {
@@ -1377,6 +1374,8 @@ void Hid::PermitVibration(Kernel::HLERequestContext& ctx) {
     IPC::RequestParser rp{ctx};
     const auto can_vibrate{rp.Pop<bool>()};
 
+    // nnSDK saves this value as a float. Since it can only be 1.0f or 0.0f we simplify this value
+    // by converting it to a bool
     Settings::values.vibration_enabled.SetValue(can_vibrate);
 
     LOG_DEBUG(Service_HID, "called, can_vibrate={}", can_vibrate);
@@ -1388,9 +1387,12 @@ void Hid::PermitVibration(Kernel::HLERequestContext& ctx) {
 void Hid::IsVibrationPermitted(Kernel::HLERequestContext& ctx) {
     LOG_DEBUG(Service_HID, "called");
 
+    // nnSDK checks if a float is greater than zero. We return the bool we stored earlier
+    const auto is_enabled = Settings::values.vibration_enabled.GetValue();
+
     IPC::ResponseBuilder rb{ctx, 3};
     rb.Push(ResultSuccess);
-    rb.Push(Settings::values.vibration_enabled.GetValue());
+    rb.Push(is_enabled);
 }
 
 void Hid::SendVibrationValues(Kernel::HLERequestContext& ctx) {