linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Wiimote joystick reports no axes
@ 2012-06-03 10:26 Giuseppe Bilotta
  2012-06-03 12:16 ` David Herrmann
  0 siblings, 1 reply; 6+ messages in thread
From: Giuseppe Bilotta @ 2012-06-03 10:26 UTC (permalink / raw)
  To: linux-input; +Cc: David Herrmann

Hello,

I've recently started playing around with the new HID Wiimote driver,
and found out that the joystick device associated with the remote
reports no axes and 7 buttons (even in the latest linux git). I
believe that the reason for this is that the joystick is created for
the "Nintendo Wii Remote" device, but the actual axis information is
passed on by the "Nintendo Wii Remote Accelerometer" device. I would
have loved to provide a patch to fix this, but I can't grasp correctly
how the joystick device creation is delegated from hid.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Wiimote joystick reports no axes
  2012-06-03 10:26 Wiimote joystick reports no axes Giuseppe Bilotta
@ 2012-06-03 12:16 ` David Herrmann
  2012-06-03 19:16   ` Giuseppe Bilotta
  0 siblings, 1 reply; 6+ messages in thread
From: David Herrmann @ 2012-06-03 12:16 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: linux-input

Hi Giuseppe

On Sun, Jun 3, 2012 at 12:26 PM, Giuseppe Bilotta
<giuseppe.bilotta@gmail.com> wrote:
> Hello,
>
> I've recently started playing around with the new HID Wiimote driver,
> and found out that the joystick device associated with the remote
> reports no axes and 7 buttons (even in the latest linux git).

You cannot use the joystick device with the Wii Remote driver. The Wii
Remote driver supports power management and this is only possible by
splitting the input data across multiple input-devices. We also need
multiple input devices because the Wii Remote reports more ABS data
than we could send via a single device.

> I believe that the reason for this is that the joystick is created for
> the "Nintendo Wii Remote" device, but the actual axis information is
> passed on by the "Nintendo Wii Remote Accelerometer" device. I would
> have loved to provide a patch to fix this, but I can't grasp correctly
> how the joystick device creation is delegated from hid.

Why do you want to fix this? The Wii Remote provides accelerometer
data. Does the joystick interface really need accelerometer data?
Please also see here for more details on the Wii Remote kernel driver
(INTERFACE/INTERFACE_SYSFS/INTERFACE_INPUT):
https://github.com/dvdhrm/xwiimote/tree/master/doc

If you want to extend the Wii Remote interface, I am open for
suggestions but I haven't found a proper way to report the data to
user-space so I let user-space handle the parsed device data.

> --
> Giuseppe "Oblomov" Bilotta

Regards
David

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Wiimote joystick reports no axes
  2012-06-03 12:16 ` David Herrmann
@ 2012-06-03 19:16   ` Giuseppe Bilotta
  2012-06-04  8:54     ` [RFC PATCH] " Giuseppe Bilotta
  0 siblings, 1 reply; 6+ messages in thread
From: Giuseppe Bilotta @ 2012-06-03 19:16 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input

Hello David,

On Sun, Jun 3, 2012 at 2:16 PM, David Herrmann
<dh.herrmann@googlemail.com> wrote:
> You cannot use the joystick device with the Wii Remote driver. The Wii
> Remote driver supports power management and this is only possible by
> splitting the input data across multiple input-devices. We also need
> multiple input devices because the Wii Remote reports more ABS data
> than we could send via a single device.

Thank you very much for the explanation. I didn't actually expect the
wiimote to be used as a joystick "out of the box", but when the new
joystick device appeared after pairing the wiimote with the computer,
I was surprised by it coming up as a "crippled" joystick (buttons, but
no axes).  Just for kicks, I tried replacing the axes reported in the
Accel device as X, Y, Z and that device also popped up as a joystick.

I understand the need for a split between the devices, and I
appreciate the power consumption motivations behind it, and I think
it's possible to expose the wiimote as a proper joystick while still
keeping the separation between the components. I'll discuss it
momentarily.

> Why do you want to fix this? The Wii Remote provides accelerometer
> data. Does the joystick interface really need accelerometer data?

The reason why I want to do this is that it would allow using the
wiimote with games that do not have direct support for it, through the
joystick interface. And of course the joystick driver can cope with
accelerometer data: for example, the accelerometer in my laptop is
also seen as a joystick and I can play neverball by holding the laptop
in my hands and rotating it appropriately (I'm pretty sure the
accelerometer in other laptop models is also supported in the same
way).

> Please also see here for more details on the Wii Remote kernel driver
> (INTERFACE/INTERFACE_SYSFS/INTERFACE_INPUT):
> https://github.com/dvdhrm/xwiimote/tree/master/doc

Thank you very much. However, these documents don't explain why a
joystick device is created for the Wii Remote and not for the Wii
Remote Accelerator. I've tried looking around the code (wiimote
driver, the hid driver and the joystick driver), and I *think* that
what happens is that the devices created when a wiimote is connected
are all passed to the joydev handler, and this catches the Wii Remote
device, because it's marked as a GAMEPAD (I can't see where this is
done by the wiimote driver, but I think hid does it automatically
because of what the hardware itself reports as class). The Wii Remote
Accelerator is NOT grabbed by joydev because it does not report an
ABS_{X,WHEEL,THROTTLE, nor a BTN_{JOYSICK,GAMEPAD,TRIGGER_HAPPY}.
This also explains why the Accelerator _does_ get exposed as a
joystick if I change the reported axes as X, Y and Z instead of RX,
RY, RZ.

If my findings are correct, then it should be sufficient to 'unmark'
the Remote device as gamepad (assuming this is possible) and mark the
Accelerator as such instead. This would expose the wiimote as a
joystick with 3 axes and no buttons, which usage-wise is better than a
joystick with 7 buttons and no axes. However, I'm not sure on how to
do that.

> If you want to extend the Wii Remote interface, I am open for
> suggestions but I haven't found a proper way to report the data to
> user-space so I let user-space handle the parsed device data.

IMO the current interface is quite satisfactory. The only thing that I
would change is that the Accelerator device should ALSO expose buttons
(possibly with a different keymap in that case). I have something in
mind to this end, I should be able to give it a test and submit an RFC
patch within a couple of days.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH] Wiimote joystick reports no axes
  2012-06-03 19:16   ` Giuseppe Bilotta
@ 2012-06-04  8:54     ` Giuseppe Bilotta
  2012-06-04 14:48       ` David Herrmann
  0 siblings, 1 reply; 6+ messages in thread
From: Giuseppe Bilotta @ 2012-06-04  8:54 UTC (permalink / raw)
  To: David Herrmann; +Cc: Giuseppe Bilotta, linux-input

> Thank you very much. However, these documents don't explain why a
> joystick device is created for the Wii Remote and not for the Wii
> Remote Accelerator.

I've done some more inquiring and discovered that the reason why the
Remote device is handled as a gamepad by the input subsystem is that it
reports a BTN_A, so I can't see a trivial way to prevent this from
happening, besides changing the keymap or blacklisting the device at the
joystick driver level.

On the other hand, this also means that just adding a BTN_A to the Accel
device is sufficient to have it automatically register for the joydev
handler. So my idea to add the buttons to the Accel device is enough to
automatically get a proper joystick device from the wiimote.

> IMO the current interface is quite satisfactory. The only thing that I
> would change is that the Accelerator device should ALSO expose buttons
> (possibly with a different keymap in that case). I have something in
> mind to this end, I should be able to give it a test and submit an RFC
> patch within a couple of days.

I'm attaching to this email a draft patch whose only purpose is to show
how I plan to do things. A proper patched based against the current
driver in the linux tree will follow if you think the idea has merit.

In words, the idea is to:

* refactor the reporting of keys; this allows the reporting to work on
  any exposed input device, by passing the report_keys function the
  inpurt device and a mapping.
* use report_keys() in handler_keys()
* register the keys in the accel device too
* use report_keys() in handler_accel() too

With these changes, there are now two joystick devices created from the
wiimote input devices, and due to the order input devices are
registered, the first one is the one based off the Accelerator input
device (and it has 3 axes and 7 buttons now), and the other is the one
based on the Remote device (and which, ideally, we would like the
joystick handler to skip).

-- 
Giuseppe "Oblomov" Bilotta

diff --git a/driver/hid-wiimote-core.c b/driver/hid-wiimote-core.c
index 745667e..f0e9fc9 100644
--- a/driver/hid-wiimote-core.c
+++ b/driver/hid-wiimote-core.c
@@ -686,30 +686,24 @@ static void wiimote_ir_close(struct input_dev *dev)
 	hid_hw_close(wdata->hdev);
 }
 
+static void report_keys(struct input_dev *input, const __u16 keymap[], const __u8 *payload)
+{
+	input_report_key(input, keymap[WIIPROTO_KEY_LEFT], !!(payload[0] & 0x01));
+	input_report_key(input, keymap[WIIPROTO_KEY_RIGHT], !!(payload[0] & 0x02));
+	input_report_key(input, keymap[WIIPROTO_KEY_DOWN], !!(payload[0] & 0x04));
+	input_report_key(input, keymap[WIIPROTO_KEY_UP], !!(payload[0] & 0x08));
+	input_report_key(input, keymap[WIIPROTO_KEY_PLUS], !!(payload[0] & 0x10));
+	input_report_key(input, keymap[WIIPROTO_KEY_TWO], !!(payload[1] & 0x01));
+	input_report_key(input, keymap[WIIPROTO_KEY_ONE], !!(payload[1] & 0x02));
+	input_report_key(input, keymap[WIIPROTO_KEY_B], !!(payload[1] & 0x04));
+	input_report_key(input, keymap[WIIPROTO_KEY_A], !!(payload[1] & 0x08));
+	input_report_key(input, keymap[WIIPROTO_KEY_MINUS], !!(payload[1] & 0x10));
+	input_report_key(input, keymap[WIIPROTO_KEY_HOME], !!(payload[1] & 0x80));
+}
+
 static void handler_keys(struct wiimote_data *wdata, const __u8 *payload)
 {
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_LEFT],
-							!!(payload[0] & 0x01));
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_RIGHT],
-							!!(payload[0] & 0x02));
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_DOWN],
-							!!(payload[0] & 0x04));
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_UP],
-							!!(payload[0] & 0x08));
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_PLUS],
-							!!(payload[0] & 0x10));
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_TWO],
-							!!(payload[1] & 0x01));
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_ONE],
-							!!(payload[1] & 0x02));
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_B],
-							!!(payload[1] & 0x04));
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_A],
-							!!(payload[1] & 0x08));
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_MINUS],
-							!!(payload[1] & 0x10));
-	input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_HOME],
-							!!(payload[1] & 0x80));
+	report_keys(wdata->input, wiiproto_keymap, payload);
 	input_sync(wdata->input);
 }
 
@@ -743,6 +737,7 @@ static void handler_accel(struct wiimote_data *wdata, const __u8 *payload)
 	input_report_abs(wdata->accel, ABS_RX, x - 0x200);
 	input_report_abs(wdata->accel, ABS_RY, y - 0x200);
 	input_report_abs(wdata->accel, ABS_RZ, z - 0x200);
+	report_keys(wdata->accel, wiiproto_keymap, payload);
 	input_sync(wdata->accel);
 }
 
@@ -1104,6 +1099,11 @@ static struct wiimote_data *wiimote_create(struct hid_device *hdev)
 	input_set_abs_params(wdata->accel, ABS_RY, -500, 500, 2, 4);
 	input_set_abs_params(wdata->accel, ABS_RZ, -500, 500, 2, 4);
 
+	set_bit(EV_KEY, wdata->accel->evbit);
+	for (i = 0; i < WIIPROTO_KEY_COUNT; ++i)
+		set_bit(wiiproto_keymap[i], wdata->accel->keybit);
+
+
 	wdata->ir = input_allocate_device();
 	if (!wdata->ir)
 		goto err_ir;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] Wiimote joystick reports no axes
  2012-06-04  8:54     ` [RFC PATCH] " Giuseppe Bilotta
@ 2012-06-04 14:48       ` David Herrmann
  2012-06-04 15:08         ` Giuseppe Bilotta
  0 siblings, 1 reply; 6+ messages in thread
From: David Herrmann @ 2012-06-04 14:48 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: linux-input

Hi Giuseppe

On Mon, Jun 4, 2012 at 10:54 AM, Giuseppe Bilotta
<giuseppe.bilotta@gmail.com> wrote:
>> Thank you very much. However, these documents don't explain why a
>> joystick device is created for the Wii Remote and not for the Wii
>> Remote Accelerator.
>
> I've done some more inquiring and discovered that the reason why the
> Remote device is handled as a gamepad by the input subsystem is that it
> reports a BTN_A, so I can't see a trivial way to prevent this from
> happening, besides changing the keymap or blacklisting the device at the
> joystick driver level.
>
> On the other hand, this also means that just adding a BTN_A to the Accel
> device is sufficient to have it automatically register for the joydev
> handler. So my idea to add the buttons to the Accel device is enough to
> automatically get a proper joystick device from the wiimote.
>
>> IMO the current interface is quite satisfactory. The only thing that I
>> would change is that the Accelerator device should ALSO expose buttons
>> (possibly with a different keymap in that case). I have something in
>> mind to this end, I should be able to give it a test and submit an RFC
>> patch within a couple of days.
>
> I'm attaching to this email a draft patch whose only purpose is to show
> how I plan to do things. A proper patched based against the current
> driver in the linux tree will follow if you think the idea has merit.
>
> In words, the idea is to:
>
> * refactor the reporting of keys; this allows the reporting to work on
>  any exposed input device, by passing the report_keys function the
>  inpurt device and a mapping.
> * use report_keys() in handler_keys()
> * register the keys in the accel device too
> * use report_keys() in handler_accel() too
>
> With these changes, there are now two joystick devices created from the
> wiimote input devices, and due to the order input devices are
> registered, the first one is the one based off the Accelerator input
> device (and it has 3 axes and 7 buttons now), and the other is the one
> based on the Remote device (and which, ideally, we would like the
> joystick handler to skip).

The changes look good to me. They still allow us to disable the
accelerometer on the device, if no-one uses the accelerometer-input so
power-management still works. However, we are now duplicating the
events which I wanted to avoid in the beginning.

I have never used the joydev interface. In fact, I don't even know why
it exists and what it does different compared to evdev. Therefore, I
recommend CC'ing Dmitry or Jiri to let them comment on your ideas. But
I guess they recommend using uinput to get the same result. That is,
creating a fake input device from userspace that collects data from
all the Wii Remote input devices and only sends the requested ones.
This would also allow applications that are _not_ aware of Wii Remotes
to use them. It would also provide much more configurability and
extensibility. There is also a xf86-input-xwiimote XInput2 driver that
can provide the same functionality to programs/games using XInput2.

Anyway, feel free to send this patch to linux-input as proper git
patch and CC Jiri and Dmitry. I have nothing to object here but I also
don't see the reason to support joydev with Wii Remotes.

Regards
David

> --
> Giuseppe "Oblomov" Bilotta
>
> diff --git a/driver/hid-wiimote-core.c b/driver/hid-wiimote-core.c
> index 745667e..f0e9fc9 100644
> --- a/driver/hid-wiimote-core.c
> +++ b/driver/hid-wiimote-core.c
> @@ -686,30 +686,24 @@ static void wiimote_ir_close(struct input_dev *dev)
>        hid_hw_close(wdata->hdev);
>  }
>
> +static void report_keys(struct input_dev *input, const __u16 keymap[], const __u8 *payload)
> +{
> +       input_report_key(input, keymap[WIIPROTO_KEY_LEFT], !!(payload[0] & 0x01));
> +       input_report_key(input, keymap[WIIPROTO_KEY_RIGHT], !!(payload[0] & 0x02));
> +       input_report_key(input, keymap[WIIPROTO_KEY_DOWN], !!(payload[0] & 0x04));
> +       input_report_key(input, keymap[WIIPROTO_KEY_UP], !!(payload[0] & 0x08));
> +       input_report_key(input, keymap[WIIPROTO_KEY_PLUS], !!(payload[0] & 0x10));
> +       input_report_key(input, keymap[WIIPROTO_KEY_TWO], !!(payload[1] & 0x01));
> +       input_report_key(input, keymap[WIIPROTO_KEY_ONE], !!(payload[1] & 0x02));
> +       input_report_key(input, keymap[WIIPROTO_KEY_B], !!(payload[1] & 0x04));
> +       input_report_key(input, keymap[WIIPROTO_KEY_A], !!(payload[1] & 0x08));
> +       input_report_key(input, keymap[WIIPROTO_KEY_MINUS], !!(payload[1] & 0x10));
> +       input_report_key(input, keymap[WIIPROTO_KEY_HOME], !!(payload[1] & 0x80));
> +}
> +
>  static void handler_keys(struct wiimote_data *wdata, const __u8 *payload)
>  {
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_LEFT],
> -                                                       !!(payload[0] & 0x01));
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_RIGHT],
> -                                                       !!(payload[0] & 0x02));
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_DOWN],
> -                                                       !!(payload[0] & 0x04));
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_UP],
> -                                                       !!(payload[0] & 0x08));
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_PLUS],
> -                                                       !!(payload[0] & 0x10));
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_TWO],
> -                                                       !!(payload[1] & 0x01));
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_ONE],
> -                                                       !!(payload[1] & 0x02));
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_B],
> -                                                       !!(payload[1] & 0x04));
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_A],
> -                                                       !!(payload[1] & 0x08));
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_MINUS],
> -                                                       !!(payload[1] & 0x10));
> -       input_report_key(wdata->input, wiiproto_keymap[WIIPROTO_KEY_HOME],
> -                                                       !!(payload[1] & 0x80));
> +       report_keys(wdata->input, wiiproto_keymap, payload);
>        input_sync(wdata->input);
>  }
>
> @@ -743,6 +737,7 @@ static void handler_accel(struct wiimote_data *wdata, const __u8 *payload)
>        input_report_abs(wdata->accel, ABS_RX, x - 0x200);
>        input_report_abs(wdata->accel, ABS_RY, y - 0x200);
>        input_report_abs(wdata->accel, ABS_RZ, z - 0x200);
> +       report_keys(wdata->accel, wiiproto_keymap, payload);
>        input_sync(wdata->accel);
>  }
>
> @@ -1104,6 +1099,11 @@ static struct wiimote_data *wiimote_create(struct hid_device *hdev)
>        input_set_abs_params(wdata->accel, ABS_RY, -500, 500, 2, 4);
>        input_set_abs_params(wdata->accel, ABS_RZ, -500, 500, 2, 4);
>
> +       set_bit(EV_KEY, wdata->accel->evbit);
> +       for (i = 0; i < WIIPROTO_KEY_COUNT; ++i)
> +               set_bit(wiiproto_keymap[i], wdata->accel->keybit);
> +
> +
>        wdata->ir = input_allocate_device();
>        if (!wdata->ir)
>                goto err_ir;
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] Wiimote joystick reports no axes
  2012-06-04 14:48       ` David Herrmann
@ 2012-06-04 15:08         ` Giuseppe Bilotta
  0 siblings, 0 replies; 6+ messages in thread
From: Giuseppe Bilotta @ 2012-06-04 15:08 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input

Hello David

On Mon, Jun 4, 2012 at 4:48 PM, David Herrmann
<dh.herrmann@googlemail.com> wrote:
> Hi Giuseppe
>
> The changes look good to me. They still allow us to disable the
> accelerometer on the device, if no-one uses the accelerometer-input so
> power-management still works. However, we are now duplicating the
> events which I wanted to avoid in the beginning.

Yes, I must say that I'm not crazy about this fact either. A possible
solution could be to not report the keys on the Remote device when
they are reported on the Accel device, or to have some kind of switch
to determine on which device(s) they should be reported (defaulting to
both). But maybe this is overengineering it?

> I have never used the joydev interface. In fact, I don't even know why
> it exists and what it does different compared to evdev. Therefore, I
> recommend CC'ing Dmitry or Jiri to let them comment on your ideas. But
> I guess they recommend using uinput to get the same result. That is,
> creating a fake input device from userspace that collects data from
> all the Wii Remote input devices and only sends the requested ones.
> This would also allow applications that are _not_ aware of Wii Remotes
> to use them. It would also provide much more configurability and
> extensibility. There is also a xf86-input-xwiimote XInput2 driver that
> can provide the same functionality to programs/games using XInput2.

The nice thing about the joydev interfaces is that it allows using the
device (possibly with some limitations) with games that already
support this interface, without needing specific wiimote support or
awareness, and without requiring additional drivers. As long as it
doesn't conflict with other usages, I think it's all value-add 8-)

> Anyway, feel free to send this patch to linux-input as proper git
> patch and CC Jiri and Dmitry. I have nothing to object here but I also
> don't see the reason to support joydev with Wii Remotes.

Ok, I'll prepare the patch, thanks.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-06-04 15:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-03 10:26 Wiimote joystick reports no axes Giuseppe Bilotta
2012-06-03 12:16 ` David Herrmann
2012-06-03 19:16   ` Giuseppe Bilotta
2012-06-04  8:54     ` [RFC PATCH] " Giuseppe Bilotta
2012-06-04 14:48       ` David Herrmann
2012-06-04 15:08         ` Giuseppe Bilotta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).