Added support for NES controllers. #38

ConnorS-P · 2025-05-20T03:52:52Z

Added support for nes controllers.

…ollers.

ConnorS-P · 2025-05-20T03:53:38Z

Based on Davidobot#1220

d3xMachina

Thanks for your contribution. Please look at the review.

d3xMachina · 2025-05-20T14:28:35Z

BetterJoy/Joycon.cs

+    {
+        Span<byte> resp = stackalloc byte[ReportLength];
+
+        for (int i = 0; i < 100; ++i) {


100 is too much, reduce this to 5. (same as ReadSPICheck) and keep the style as

for () { }

instead of

for () { }

d3xMachina · 2025-05-20T14:29:02Z

BetterJoy/Joycon.cs

+        Span<byte> resp = stackalloc byte[ReportLength];
+
+        for (int i = 0; i < 100; ++i) {
+            int respLength = SubcommandCheck(0x02, Array.Empty<byte>(), resp, false, 200);


Remove the ", 200" parameter and use a collection expression [] instead of Array.Empty<byte>().

d3xMachina · 2025-05-20T14:33:19Z

BetterJoy/Joycon.cs

+        for (int i = 0; i < 100; ++i) {
+            int respLength = SubcommandCheck(0x02, Array.Empty<byte>(), resp, false, 200);
+
+            if (respLength >= 15 && resp[15] == 0x03) {


15 is incorrect, it's the index to the first byte of the firmware version. You should use 17. (information here)
You might want to change 0x03 too unless it's really the value that is used for the NES controller. (I don't have one to test). 1 is left joycon, 2 is right joycon and 3 is pro controller.

You should use > and not >= since the index starts at 0.

Great catch! I think I misread the output and was for some reason thinking it was replying as a pro controller. I was having a heck of a time getting a response on the original fork (hence the massive attempt numbers) so I also may have read the wrong packet at some point and it just happen to have worked due to firmware versioning.

d3xMachina · 2025-05-20T14:37:36Z

BetterJoy/Joycon.cs

+                break;
+            }
+
+            if (resp[15] == 0x02) {


Any other value than the one for the NES should break the loop. So the content of the for should be changed to :

if (respLength > 17) { if (resp[17] == 0x03) // you should check that the 0x03 value is correct as stated in my previous comment { Type = ControllerType.NES; } break; }

d3xMachina · 2025-05-20T14:38:05Z

BetterJoy/Joycon.cs

            }
+
+            //Make sure we're not actually a nes controller
+            if (this.Type == ControllerType.JoyconRight)


You can remove this.

d3xMachina · 2025-05-20T14:44:06Z

BetterJoy/Joycon.cs

    }

-    private int SubcommandCheck(byte sc, ReadOnlySpan<byte> bufParameters, Span<byte> response, bool print = true)
+    private int SubcommandCheck(byte sc, ReadOnlySpan<byte> bufParameters, Span<byte> response, bool print = true, int retries = 10)


Remove this change , int retries = 10

d3xMachina · 2025-05-20T14:58:51Z

BetterJoy/Joycon.cs

        JoyconLeft,
        JoyconRight,
        SNES,
+        NES,


You need to add NES at the end of the enum or it will break the saved datas for third party controller window.
Also for the third party controller feature to work properly, you need to add to the switch in the TypeToProdId function :

case 6: return ProductNES;

Fixed, and made the numerics explicit in the enum to reduce brittleness.

ConnorS-P · 2025-05-20T22:34:32Z

Thanks for the review! I'll fix these and get back to you.

…d general cleanup.

ConnorS-P · 2025-05-20T23:56:04Z

Hold on, I think I broke something.

…t and left.

ConnorS-P · 2025-05-21T05:13:04Z

Ok, that should have fixed it. Turns out the NES controllers report differently (left or right) as to what they are.

d3xMachina

A few more changes and we're good.

d3xMachina · 2025-05-21T09:22:52Z

BetterJoy/Joycon.cs

+            //Make sure we're not actually a nes controller
+            if (Type == ControllerType.JoyconRight)
+            {
+                CheckIfRightIsNes();
+            }


Move this below just after SetReportMode(ReportMode.SimpleHID, false);

d3xMachina · 2025-05-21T09:24:02Z

BetterJoy/Joycon.cs

+        {
+            var respLength = SubcommandCheck(0x02, [], response, false);
+
+            if (respLength >= 20 && response[0] == 0x21 && response[14] == 0x02)


You can remove && response[0] == 0x21 && response[14] == 0x02 as it is already checked by SubcommandCheck. Keeping respLength >= 20 is fine.

While this is checked, it is not checked in a way that is useful for us here.

private int SubcommandCheck(byte sc, ReadOnlySpan<byte> bufParameters, Span<byte> response, bool print = true) { int length = Subcommand(sc, bufParameters, print); if (length <= 0) { DebugPrint($"Subcommand write error: {ErrorMessage()}", DebugType.Comms); return length; } int tries = 0; bool responseFound; do { length = Read(response, 100); // don't set the timeout lower than 100 or might not always work responseFound = length >= 20 && response[0] == 0x21 && response[14] == sc; if (length < 0) { DebugPrint($"Subcommand read error: {ErrorMessage()}", DebugType.Comms); } tries++; } while (tries < 10 && !responseFound && length >= 0); if (!responseFound) { DebugPrint("No response.", DebugType.Comms); return length; } if (print) { PrintArray<byte>( response, DebugType.Comms, length - 1, 1, $"Response ID {response[0]:X2}." + " Data: {0:S}" ); } return length; }

The while loop continues until a response is found OR until we run out of tries. In the event we run out of tries, the function just gives us the last read item, regardless of if it passed that check or not. We check it again here so we can run another iteration of the for loop if necessary.

Good catch, that was a bug I fixed yesterday in e2d603e, now it returns 0 properly if no response was found so you can remove it.

d3xMachina · 2025-05-21T09:25:59Z

BetterJoy/Joycon.cs

            SetNFCIR(false);
            SetReportMode(ReportMode.StandardFull);
-
+            


Unwanted white spaces.

d3xMachina · 2025-05-21T09:26:38Z

ViGEm.NET

Those files shouldn't be changed. It was changed by your merge commit.

Sorry, this is my first time working with submodules. I think I fixed it.

d3xMachina · 2025-05-21T09:39:24Z

BetterJoy/Joycon.cs

        return true;
    }

+    private void CheckIfRightIsNes()


Should throw a DeviceComFailedException if it failed so it tries to reconnect the controller like so :

private void CheckIfRightIsNes() { Span<byte> response = stackalloc byte[ReportLength]; var ok = false; for (var i = 0; i < 5; ++i) { var respLength = SubcommandCheck(0x02, [], response, false); if (respLength >= 20) { // NES controllers share the hardware ID of the right joycon, but respond here differently. if (response[17] == 0x0A || response[17] == 0x09) { Type = ControllerType.NES; } ok = true; break; } } if (!ok) { throw new DeviceComFailedException("reset device info"); } }

d3xMachina · 2025-05-21T10:24:27Z

BetterJoy/Joycon.cs

+
+            if (respLength >= 20 && response[0] == 0x21 && response[14] == 0x02)
+            {
+                if (response[17] == 0x0A || response[17] == 0x09) //NES controllers share the Right hardware ID of the right joycon, but respond here differently.


Please add to the comment which value correspond to left and right for 0x0A and 0x09

ConnorS-P · 2025-05-22T02:00:22Z

I got all the stuff that needed fixes and replied to your one comment regarding the duplicated message check. Let me know if that is sufficient or if you need anything tweaked. 👍

d3xMachina · 2025-05-22T07:05:12Z

Thank you, only the duplicate check in CheckIfRightIsNes needs to be removed since it was a bug that was fixed very recently and I can merge.

ConnorS-P · 2025-05-23T01:24:20Z

Ok, I removed the redundant check, now we just check for a valid response via looking if the response length is greater than 0.

d3xMachina · 2025-05-23T07:54:25Z

Thanks, merged!

add-support-for-nes-controller-d3xMachina Added support for nes contr…

1216fcb

…ollers.

ConnorS-P changed the title ~~add-support-for-nes-controller-d3xMachina Added support for nes contr…~~ Added support for NES controllers. May 20, 2025

d3xMachina requested changes May 20, 2025

View reviewed changes

ConnorS-P added 2 commits May 20, 2025 17:29

add-support-for-nes-controller-d3xMachina Fixed NES identification an…

7aecca3

…d general cleanup.

Merge branch 'master' into add-support-for-nes-controller-d3xMachina

82a7b88

Fixed the issue where the NES controllers have different IDs for righ…

d163ba7

…t and left.

ConnorS-P requested a review from d3xMachina May 21, 2025 05:14

d3xMachina requested changes May 21, 2025

View reviewed changes

ConnorS-P added 2 commits May 21, 2025 19:40

add-support-for-nes-controller-d3xMachina Cleanup.

19fa404

More fixes and cleanup.

5b2325f

ConnorS-P requested a review from d3xMachina May 22, 2025 02:00

ConnorS-P added 2 commits May 22, 2025 19:21

Removed redundant checks.

ecc3997

Tweaked to just check for a valid response.

6889abe

d3xMachina merged commit 87dbaba into d3xMachina:master May 23, 2025

Added support for NES controllers. #38

Added support for NES controllers. #38

Uh oh!

Conversation

ConnorS-P commented May 20, 2025

Uh oh!

ConnorS-P commented May 20, 2025

Uh oh!

d3xMachina left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ConnorS-P commented May 20, 2025

Uh oh!

ConnorS-P commented May 20, 2025

Uh oh!

ConnorS-P commented May 21, 2025

Uh oh!

d3xMachina left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ConnorS-P commented May 22, 2025

Uh oh!

d3xMachina commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ConnorS-P commented May 23, 2025

d3xMachina commented May 22, 2025 •

edited

Loading