fasteddy516 / CircuitPython_JoystickXL

Turn a CircuitPython device into a joystick controller with lots of inputs.
MIT License
18 stars 7 forks source link

Joystick resolution #12

Open ntindle opened 1 year ago

ntindle commented 1 year ago

Is it possible to have a higher resolution joystick?

jamesra commented 1 year ago

I've written a patch for a 16-bit joystick output. I'm hoping to package it up and send a pull request soon. My version replaces 8-bit axis outputs entirely with 16-bit, but I'm guessing the owner of this libary would like a flag that can be passed to specify precision?

fasteddy516 commented 1 year ago

What's the motivation to bump up the axis precision? (I'm not asking you to justify the request - I'm genuinely curious! 😃) Is there a practical issue that the increased precision solves (i.e. choppy controls / controls unresponsive to tiny adjustments), or is it more intellectual (i.e. my microcontroller's ADC has more than 8-bits of precision and I don't want to throw any of those bits away.)

I don't necessarily have a problem bumping up the standard axis precision to 16-bit as long as it doesn't cause a large drop in the HID reporting rate due to the increased size of each report. I suspect the extra up-to-8-bytes won't have a dramatic impact, but I've never tried.

As far as flags/configuration options go I wouldn't worry about it too much - the next version of JoystickXL will hopefully be using settings.toml (introduced in CircuiyPython 8) to pass in configuration values using a more standardized method, so all of the existing configuration stuff will be getting a rework anyway. If an option to select axis precision is necessary I can build it in then.

jamesra commented 1 year ago

I am a (first-year) mentor on a high-school FIRST robotics team. For the post-season a small group of younger students is prototyping a controller to drive a large (2 meter?) double jointed arm with a "waldo" mini-arm (~25cm) controller. (The larger arm should mirror the position of the mini-arm). One joint has maybe 160 degrees of range, the second joint has maybe 300 degrees. 8-bit encoding may have worked, but it was an extra source of error (+/- ~1 degree) and truncated the precision of the hardware. The error, if it is a problem, would also compound up the arm. The 16-bit version got the controller output error down to less than 1/4 degree (which is probably near the limit of the board's ADC) so we are no longer removing precision from the ADC measurement.

They are still working on the robot code itself now, so I can't say for certain the 8-bit was too imprecise. After showing them the loss of precision I decided to add 16-bit support because 8-bit precision is tricky for less experienced programmers to troubleshoot and even if they identified the cause this controller project is enough of a reach without them trying to edit USB HID :-)

If you are doing a redesign my student programmer had some trouble getting the output through joystick_xl because they did not want to read the Analog pin directly. They first wanted to map the potentiometer output to the range of arm movement instead of reporting raw potentiometer values. (So min arm angle -> 0 and max arm angle -> 1.0). I helped them dig into the code to understand virtual inputs and manually setting the value. The interface they were used to was Teensy on Arduino IDE, which has a joystick.set_axis(index, value) style API calls followed by a joystick.update().

Please consider adding documentation in the Readme for using your package without reading the pins directly. (Sorry if it is there and I forgot. I just know I had to help them with that part.)

Thanks for writing this library, Circuit Python is a lot easier for these kids to work with than Arduino for the most part.

fasteddy516 commented 1 year ago

That's an awesome use case! It's exciting to hear about JoystickXL seeing real-world use in a learning environment like this. I created it as a learning project of my own, and have no real way of knowing just how much use it is getting elsewhere. Your detailed description is greatly appreciated and helps provide motivation to get to work on updates I have had in mind for months now. 😃

You're right about the lack of examples for the VirtualInput class. I will definitely need to add some to the docs.

Feel free to send a pull request any time. If you've got a fork with changes committed to it I can also just look at that and compare it to what I had in mind for a 16-bit implementation.

henrygab commented 1 year ago

Some cautionary questions / tales:

If my memory serves, there are embedded systems that presume a 16-bit reported value will be 2-byte aligned, and that the full descriptor will be a multiple of 2-bytes. IIRC, HID descriptors are all byte arrays... and thus should have no alignment requirement. Thus, any host that would fault on unaligned access is required to reconstruct the 16-bit value using unaligned-safe (byte-by-byte) reads of the data. Clearly, it is a bug for the host to presume a 16-bit value is aligned in the descriptor. Unfortunately, I believe this is what's out there ... and it's a pain to figure out the cause when this edge case hits.

Anyways, if you do end up allowing 16-bit values (such as a 16-bit axis), please consider two tweaks:

  1. Add padding bytes before any 16-bit value, to ensure 16-bit value is aligned on an even byte boundary (within the descriptor).
  2. Add padding bytes at the end of the descriptor if needed, to ensure each descriptor has an even number of bytes.

Compatibility truly is a mix of smoke, mirrors, and true black magic. 🙂 Here, it may cost up to 2 bytes....

arnew commented 2 months ago

I have started an implementation sketch for this. Somehow did not see the comment about the padding, will have a look into that - @henrygab do you have an example of a descriptor with padding bytes?

Also, to remove code duplication the Axis16 may be implemented as child of the normal Axis? Or maybe it is completely superfluous and can be modelled just by setting min/max of the output in the standard Axis?

See https://github.com/fasteddy516/CircuitPython_JoystickXL/pull/31 and let me know your comments.

henrygab commented 2 months ago

Most keyboards have an unused byte or two. See, for example:

https://github.com/adafruit/Adafruit_CircuitPython_HID/blob/be6db4658d26902b6d5191d16de658adb92d018a/adafruit_hid/keyboard.py#L58-L60

which uses a report descriptor that is essentially:

typedef struct _keyboard_report_t {
    uint8_t modifier_flags; // e.g. shift, capslock, etc.
    uint8_t unused; // listed as a constant value
    uint8_t keycodes[7]; // for up to seven keycodes per report
} keyboard_report_t;

The following adafruit learn example has links to other useful tools:

https://learn.adafruit.com/customizing-usb-devices-in-circuitpython/hid-devices

henrygab commented 2 months ago

And here's a permalink that shows the corresponding descriptor:

https://github.com/adafruit/circuitpython/blob/87f7bc9fcc74e8ada4f255a8d7824f46a039995c/shared-module/usb_hid/Device.c#L18-L66

Note in particular lines 31-33.