Joystick 16-bit support

[seb-pau]

Add optional 16-bit Joystick support

Description

Currently the Joystick implementation (#4226) only supports 8-bit signed values (from -127 to 127). As even cheap MCUs like the Pro Micro have 10-bit ADCs, it would be beneficial to have higher precision.
This PR adds an option to change joystick axes to signed 16-bit values (from -32767 to 32767). This can be enabled by adding #define JOYSTICK_16_BIT, otherwise the original 8-bit functionality stays unchanged (so this shouldn't break anything that already uses the 8-bit values).

Update: Use #define JOYSTICK_AXES_RESOLUTION XX to change joystick resolution to a number of bits, e.g. #define JOYSTICK_AXES_RESOLUTION 16 for 16-bit Joystick axes. Minimum resolution is 8-bit, which is also the default if resolution is not defined, maximum is 16-bit.

I have tested the changes for virtual axes with manually assigned values, as well as using the JOYSTICK_AXIS_IN(...) function.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

#10330

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[fauxpark]

Considering the ADC resolution on all supported AVRs is 10 bits, and on STM32 mostly 12 bits, it might be better to make this a little more configurable, with something like #define JOYSTICK_AXES_RESOLUTION 10.

You can also calculate the js_min/max values in preprocessing by doing (1 << JOYSTICK_AXES_RESOLUTION) - 1

[seb-pau]

I have implemented a configurable resolution like you suggested. To be honest I'm not sure if there is much advantage to use e.g. 12-bit instead of always scaling to 16-bit, but options are always nice I guess, and #define JOYSTICK_AXES_RESOLUTION 16 looks a bit cleaner than #define JOYSTICK_16_BIT.
Should I update the pull request description to reflect the changes?

[fauxpark]

• the following changes in joystick.h:

diff --git a/quantum/joystick.h b/quantum/joystick.h
index a95472b9f..2b5dbd43b 100644
--- a/quantum/joystick.h
+++ b/quantum/joystick.h
@@ -1,5 +1,9 @@
 #pragma once
 
+#include "quantum.h"
+
+#include <stdint.h>
+
 #ifndef JOYSTICK_BUTTON_COUNT
 #    define JOYSTICK_BUTTON_COUNT 8
 #endif
@@ -8,9 +12,13 @@
 #    define JOYSTICK_AXES_COUNT 4
 #endif
 
-#include "quantum.h"
+#ifndef JOYSTICK_AXES_RESOLUTION
+#    define JOYSTICK_AXES_RESOLUTION 8
+#elif JOYSTICK_AXES_RESOLUTION < 8 || JOYSTICK_AXES_RESOLUTION > 16
+#    error JOYSTICK_AXES_RESOLUTION must be between 8 and 16
+#endif
 
-#include <stdint.h>
+#define JOYSTICK_RESOLUTION ((1 << (JOYSTICK_AXES_RESOLUTION - 1)) - 1)
 
 // configure on input_pin of the joystick_axes array entry to JS_VIRTUAL_AXIS
 // to prevent it from being read from the ADC. This allows outputing forged axis value.

[seb-pau]

Thanks for the suggestions, I added the requested changes. I also had to include the "joystick.h" in the usb_descriptor.c file, because it would throw "undeclared" errors otherwise. Please have a look if I should move this somewhere else.
Also, I'm not sure about the joystick endpoint size. Does it have to be increased to 16 as well if the resolution is >8 bit? (JOYTSICK_EPSIZE in the usb_descriptor.h file)

[seb-pau]

I have been testing a bit, and I found that currently without JOYSTICK_AXES_RESOLUTION manually defined, the joystick doesn't work. I have tracked it down to report.h, that would need to have joystick.h included to get the default #define JOYSTICK_AXES_RESOLUTION 8 declaration, but I can't include joystick.h in the report.h file without getting errors.

A simple workaround would be to use #if JOYSTICK_AXES_RESOLUTION <= 8 instead of ==8 since without the definition it assumes the variable is 0. This isn't a very elegant solution imo, but I've tested it and it works.

index f5ba64909..a5b124e95 100644
--- a/tmk_core/common/report.h
+++ b/tmk_core/common/report.h
@@ -192,7 +192,7 @@ typedef struct {

 typedef struct {
 #if JOYSTICK_AXES_COUNT > 0
-    # if JOYSTICK_AXES_RESOLUTION == 8
+    # if JOYSTICK_AXES_RESOLUTION <= 8
         int8_t axes[JOYSTICK_AXES_COUNT];
     # else
         int16_t axes[JOYSTICK_AXES_COUNT];

[fauxpark]

You shouldn't need to include joystick.h in report.h - it's already referencing stuff from joystick.h without issues.

[seb-pau]

Weird, right? But it only works when I put #define JOYSTICK_AXES_RESOLUTION 8 somewhere in my keyboard/keymap files. If I leave it undefined, it should be set to 8 in joystick.h, and it should have the same results, but somehow it doesn't. I added the following lines to the report.h for testing purposes, and it seems like joystick.h doesn't load correctly as I get the error message "error JOYSTICK_AXES_RESOLUTION not defined":

#ifndef JOYSTICK_AXES_RESOLUTION
#   error JOYSTICK_AXES_RESOLUTION not defined
#endif

[fauxpark]

What about the others? JOYSTICK_AXES_COUNT and JOYSTICK_BUTTON_COUNT

[seb-pau]

Same behaviour with the above test, they don't appear to be declared in the report.h. Even more interesting, if I exclude the test and don't declare JOYSTICK_AXES_COUNT, I get this error message:

Compiling: tmk_core/protocol/lufa/lufa.c                                                           tmk_core/protocol/lufa/lufa.c: In function 'send_joystick_packet':
tmk_core/protocol/lufa/lufa.c:319:10: error: 'joystick_report_t' {aka 'struct <anonymous>'} has no member named 'axes'
         .axes =
          ^~~~
tmk_core/protocol/lufa/lufa.c:330:17: error: excess elements in array initializer [-Werror]
                 joystick->axes[3],
                 ^~~~~~~~
tmk_core/protocol/lufa/lufa.c:330:17: note: (near initialization for 'r.buttons')
cc1.exe: all warnings being treated as errors
 [ERRORS]
 |
 |
 |
make[1]: *** [tmk_core/rules.mk:386: .build/obj_handwired_sherbet_fortnite/lufa.o] Error 1
Make finished with errors
make: *** [Makefile:584: handwired/sherbet:fortnite:avrdude] Error 1

Despite it being set to a default value in joystick.h when left undeclared.

[fauxpark]

Not sure why the compiler complains then.

The resolution check in report.h can stay as ==, inspecting the joystick report in usb_descriptor.lst shows what I'd expect with and without the resolution defined in the onekey joystick keymap's config.h. Other than that and the superfluous newlines, this looks good.