The spaceball.c module was not properly parsing the movement reports
coming from the device. The code read axis data as signed 16-bit
little-endian values starting at offset 2.
In fact, axis data in Spaceball movement reports are signed 16-bit
big-endian values starting at offset 3. This was determined first by
visually inspecting the data packets, and later verified by consulting:
http://spacemice.org/pdf/SpaceBall_2003-3003_Protocol.pdf
If this ever worked properly, it was in the time before Git...
Signed-off-by: Leo L. Schwab <[email protected]>
---
drivers/input/joystick/spaceball.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/input/joystick/spaceball.c b/drivers/input/joystick/spaceball.c
index 429411c6c0a8..43bfb3d2fa8a 100644
--- a/drivers/input/joystick/spaceball.c
+++ b/drivers/input/joystick/spaceball.c
@@ -74,10 +74,20 @@ static void spaceball_process_packet(struct spaceball* spaceball)
switch (spaceball->data[0]) {
case 'D': /* Ball data */
+ /*
+ * Skip first three bytes; read six axes worth of data.
+ * Axis values are signed 16-bit big-endian.
+ */
if (spaceball->idx != 15) return;
- for (i = 0; i < 6; i++)
- input_report_abs(dev, spaceball_axes[i],
- (__s16)((data[2 * i + 3] << 8) | data[2 * i + 2]));
+ data += 3;
+ for (i = 0;
+ i < ARRAY_SIZE(spaceball_axes);
+ ++i, data += sizeof(__s16)) {
+ input_report_abs(
+ dev,
+ spaceball_axes[i],
+ (__s16)((data[0] << 8) | data[1]));
+ }
break;
case 'K': /* Button data */
--
2.32.0
On Mon, Jul 26, 2021 at 09:06:24PM -0700, Leo L. Schwab wrote:
> The spaceball.c module was not properly parsing the movement reports
> coming from the device. [ ... ]
Um... Ping? This issue still exists, and it's been around a really
long time. I've been using this patch to successfully fly around QuakeWorld
for several months now.
Schwab
Hi Leo,
On Mon, Jul 26, 2021 at 09:06:24PM -0700, Leo L. Schwab wrote:
> The spaceball.c module was not properly parsing the movement reports
> coming from the device. The code read axis data as signed 16-bit
> little-endian values starting at offset 2.
>
> In fact, axis data in Spaceball movement reports are signed 16-bit
> big-endian values starting at offset 3. This was determined first by
> visually inspecting the data packets, and later verified by consulting:
> http://spacemice.org/pdf/SpaceBall_2003-3003_Protocol.pdf
>
> If this ever worked properly, it was in the time before Git...
Thank you for the patch.
>
> Signed-off-by: Leo L. Schwab <[email protected]>
> ---
> drivers/input/joystick/spaceball.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/joystick/spaceball.c b/drivers/input/joystick/spaceball.c
> index 429411c6c0a8..43bfb3d2fa8a 100644
> --- a/drivers/input/joystick/spaceball.c
> +++ b/drivers/input/joystick/spaceball.c
> @@ -74,10 +74,20 @@ static void spaceball_process_packet(struct spaceball* spaceball)
> switch (spaceball->data[0]) {
>
> case 'D': /* Ball data */
> + /*
> + * Skip first three bytes; read six axes worth of data.
> + * Axis values are signed 16-bit big-endian.
> + */
> if (spaceball->idx != 15) return;
> - for (i = 0; i < 6; i++)
> - input_report_abs(dev, spaceball_axes[i],
> - (__s16)((data[2 * i + 3] << 8) | data[2 * i + 2]));
> + data += 3;
> + for (i = 0;
> + i < ARRAY_SIZE(spaceball_axes);
> + ++i, data += sizeof(__s16)) {
> + input_report_abs(
> + dev,
> + spaceball_axes[i],
> + (__s16)((data[0] << 8) | data[1]));
> + }
Could we write
for (i == 0; i < ARRAY_SIZE(spaceball_axes); i++)
input_report_abs(dev, spaceball_axes[i],
(__s16)(get_unaligned_be16(&data[i * 2]);
instead?
Thanks!
--
Dmitry
On Mon, Dec 20, 2021 at 12:59:21AM -0800, Dmitry Torokhov wrote:
> Could we write
>
> for (i == 0; i < ARRAY_SIZE(spaceball_axes); i++)
> input_report_abs(dev, spaceball_axes[i],
> (__s16)(get_unaligned_be16(&data[i * 2]);
>
> instead?
>
It's not as readable, but sure, I could do that.
> for (i == 0; i < ARRAY_SIZE(spaceball_axes); i++)
^^
Pretty sure you didn't mean that :-).
> input_report_abs(dev, spaceball_axes[i],
> (__s16)(get_unaligned_be16(&data[i * 2]);
^^^^^^^
I'm new here, but it seems odd that an array index (shift plus add
to the base pointer) is preferred over a direct pointer reference.
> (__s16)(get_unaligned_be16(&data[i * 2]);
^^^^^^^^^^^^^^^^^^
Ooo! Didn't know about this; thank you!
Schwab