Re: [v4 4/4] winejoystick.drv/joystick_osx.c: adds sim page element controls
On Jun 13, 2016, at 12:11 AM, David Lawrie <david.dljunk(a)gmail.com> wrote:
Adds support for input features defined by HID Simulation page: Rudder (Rz), Throttle (Z), Steering (X), Accelerator (Y), Brake (Rz).
+static int axis_for_usage_Sim(int usage) +{ + switch (usage) + { + case kHIDUsage_Sim_Rudder: return AXIS_RZ; + case kHIDUsage_Sim_Throttle: return AXIS_Z; + case kHIDUsage_Sim_Steering: return AXIS_X; + case kHIDUsage_Sim_Accelerator: return AXIS_Y; + case kHIDUsage_Sim_Brake: return AXIS_RZ; + }
Is it intentional that Rudder and Brake both map to the same axis? Later, when enumerating elements, which comes first and thus "wins" the collision (if they ever appear on the same device) is arbitrary based on enumeration order. That OK?
@@ -404,66 +418,100 @@ static void collect_joystick_elements(joystick_t* joystick, IOHIDElementRef coll break; } case kIOHIDElementTypeInput_Axis: - { - TRACE("kIOHIDElementTypeInput_Axis; ignoring\n"); - break; - } case kIOHIDElementTypeInput_Misc:
It seems that treating kIOHIDElementTypeInput_Axis the same as kIOHIDElementTypeInput_Misc is a separate issue from adding support for the Sim page inputs. Or is there an interrelationship that I'm not aware of? Is it the case that both Axis and Misc elements can be present with either GD or Sim usage pages? Some of the debug logging statements have been changed to say "kIOHIDElementTypeInput_Axis/Misc" while others have been left as "kIOHIDElementTypeInput_Misc". Is that indicative of something or just an oversight?
@@ -614,7 +662,7 @@ LRESULT driver_joyGetDevCaps(DWORD_PTR device_id, JOYCAPSW* caps, DWORD size) { int i;
- /* complete 95 structure */ + /* complete Win95 structure */
This is an unrelated change. -Ken
On Mon, Jun 13, 2016 at 2:46 PM, Ken Thomases <ken(a)codeweavers.com> wrote:
On Jun 13, 2016, at 12:11 AM, David Lawrie <david.dljunk(a)gmail.com> wrote:
Adds support for input features defined by HID Simulation page: Rudder (Rz), Throttle (Z), Steering (X), Accelerator (Y), Brake (Rz).
+static int axis_for_usage_Sim(int usage) +{ + switch (usage) + { + case kHIDUsage_Sim_Rudder: return AXIS_RZ; + case kHIDUsage_Sim_Throttle: return AXIS_Z; + case kHIDUsage_Sim_Steering: return AXIS_X; + case kHIDUsage_Sim_Accelerator: return AXIS_Y; + case kHIDUsage_Sim_Brake: return AXIS_RZ; + }
Is it intentional that Rudder and Brake both map to the same axis? Later, when enumerating elements, which comes first and thus "wins" the collision (if they ever appear on the same device) is arbitrary based on enumeration order. That OK?
Yeah I think that's fine as it's unlikely that the same joystick/gamepad/controller will have both a listed Rudder and a listed Brake axis as one is generally found on a device to control sim-planes and the other to control sim-cars. So this is a case where a controller might call its RZ axis GD_RZ, Brake, or Rudder depending on the device. Instead of GD_X or GD_Y a car controller might call it steering and accelerator. Instead of a slider, a joystick might list a throttle. The Linux version had some of these too and I thought it better than not just to check in case. I realized I did put the wrong link into the comment however. It should've been this one: https://msdn.microsoft.com/en-us/library/windows/hardware/ff538338(v=vs.85)....
@@ -404,66 +418,100 @@ static void collect_joystick_elements(joystick_t* joystick, IOHIDElementRef coll break; } case kIOHIDElementTypeInput_Axis: - { - TRACE("kIOHIDElementTypeInput_Axis; ignoring\n"); - break; - } case kIOHIDElementTypeInput_Misc:
It seems that treating kIOHIDElementTypeInput_Axis the same as kIOHIDElementTypeInput_Misc is a separate issue from adding support for the Sim page inputs. Or is there an interrelationship that I'm not aware of? Is it the case that both Axis and Misc elements can be present with either GD or Sim usage pages?
Some of the debug logging statements have been changed to say "kIOHIDElementTypeInput_Axis/Misc" while others have been left as "kIOHIDElementTypeInput_Misc". Is that indicative of something or just an oversight?
Mostly it's because I wasn't sure how the Sim page axes would be listed - under Axis or under Misc. For instance, I was very surprised to see the Generic_Desktop axes listed under Misc instead of Axis. Do you know why that is? If the GD_X_Axis is under Misc, not Axis, what's under Axis? The Mac HID documentation was even more confusing than the Microsoft documentation! (at least for me). The changes in the debug statements is because the general/default case could now be a Axis or Misc type, but the Generic_Desktop page axes are known Misc types. So I decided one but not the other. Not changing the Sim page axes debug statement on line 496 to Axis/Misc to reflect my lack of knowledge as to how they would be listed was indeed an oversight. However, we can go back to ignoring the reported Axes elements, but I noticed the dinput code didn't ignore them and neither did several other examples of OS X joystick programs I found on the web. So I thought, why not just have the axis-type fall through, since I'm not actually sure what falls under axis versus misc, and try to catch everything? That was my thinking.
@@ -614,7 +662,7 @@ LRESULT driver_joyGetDevCaps(DWORD_PTR device_id, JOYCAPSW* caps, DWORD size) { int i;
- /* complete 95 structure */ + /* complete Win95 structure */
This is an unrelated change.
I'd forgotten I'd even made that change. :) That was for myself because I wasn't sure what 95 meant until I looked at the Linux code and saw Win95 and figured out the rest. So I changed the comment to remind myself what that piece of code did. It's not particularly important to keep.
-Ken
On Jun 14, 2016, at 1:56 AM, DavidL <david.dljunk(a)gmail.com> wrote:
On Mon, Jun 13, 2016 at 2:46 PM, Ken Thomases <ken(a)codeweavers.com <mailto:ken(a)codeweavers.com>> wrote: Is it intentional that Rudder and Brake both map to the same axis? Later, when enumerating elements, which comes first and thus "wins" the collision (if they ever appear on the same device) is arbitrary based on enumeration order. That OK?
Yeah I think that's fine as it's unlikely that the same joystick/gamepad/controller will have both a listed Rudder and a listed Brake axis as one is generally found on a device to control sim-planes and the other to control sim-cars. So this is a case where a controller might call its RZ axis GD_RZ, Brake, or Rudder depending on the device. Instead of GD_X or GD_Y a car controller might call it steering and accelerator. Instead of a slider, a joystick might list a throttle. The Linux version had some of these too and I thought it better than not just to check in case. I realized I did put the wrong link into the comment however. It should've been this one: https://msdn.microsoft.com/en-us/library/windows/hardware/ff538338(v=vs.85).... <https://msdn.microsoft.com/en-us/library/windows/hardware/ff538338(v=vs.85).aspx>
OK. By the way, that reminds me: with respect to your commit log, if there's anything in there, such as the v2, v3, etc. changes, that shouldn't actually be in the ultimate commit log, put it below the "---" line. The Signed-off-by line has to be above that line, though. So, depending on how you're doing things, you may need to use git-format-patch to create files, edit those files to put in the notes, and then use git-send-email to send those files. (There's also the git-notes functionality that you can use for this, but I haven't spent the time to figure out how to use it.)
It seems that treating kIOHIDElementTypeInput_Axis the same as kIOHIDElementTypeInput_Misc is a separate issue from adding support for the Sim page inputs. Or is there an interrelationship that I'm not aware of? Is it the case that both Axis and Misc elements can be present with either GD or Sim usage pages?
Some of the debug logging statements have been changed to say "kIOHIDElementTypeInput_Axis/Misc" while others have been left as "kIOHIDElementTypeInput_Misc". Is that indicative of something or just an oversight?
Mostly it's because I wasn't sure how the Sim page axes would be listed - under Axis or under Misc. For instance, I was very surprised to see the Generic_Desktop axes listed under Misc instead of Axis. Do you know why that is?
No, I don't. Maybe it's the fact that all of the GD things are treated alike and, since some aren't axes, they can't go under Axis.
If the GD_X_Axis is under Misc, not Axis, what's under Axis?
I don't know. I suspect it just depends on how the device manufacturer designed the device and its HID reports.
The Mac HID documentation was even more confusing than the Microsoft documentation! (at least for me). The changes in the debug statements is because the general/default case could now be a Axis or Misc type, but the Generic_Desktop page axes are known Misc types.
Hmm? I don't follow how you know that those axes are Misc types (other than by the preexisting state of the code, which may not be reliable).
So I decided one but not the other. Not changing the Sim page axes debug statement on line 496 to Axis/Misc to reflect my lack of knowledge as to how they would be listed was indeed an oversight. However, we can go back to ignoring the reported Axes elements, but I noticed the dinput code didn't ignore them and neither did several other examples of OS X joystick programs I found on the web. So I thought, why not just have the axis-type fall through, since I'm not actually sure what falls under axis versus misc, and try to catch everything? That was my thinking.
I think it's good to handle kIOHIDElementTypeInput_Axis. I just think it's a separate issue from the Sim page and so deserves a separate patch. For what it's worth, the handling in dinput doesn't seem like it could be right. The elements are added to the elements array but then nothing every handles them properly after that. For example, poll_osx_device_state() only handles Button and Misc. -Ken
On Tue, Jun 14, 2016 at 12:38 AM, Ken Thomases <ken(a)codeweavers.com> wrote:
On Jun 14, 2016, at 1:56 AM, DavidL <david.dljunk(a)gmail.com> wrote:
On Mon, Jun 13, 2016 at 2:46 PM, Ken Thomases <ken(a)codeweavers.com> wrote:
Is it intentional that Rudder and Brake both map to the same axis? Later, when enumerating elements, which comes first and thus "wins" the collision (if they ever appear on the same device) is arbitrary based on enumeration order. That OK?
Yeah I think that's fine as it's unlikely that the same joystick/gamepad/controller will have both a listed Rudder and a listed Brake axis as one is generally found on a device to control sim-planes and the other to control sim-cars. So this is a case where a controller might call its RZ axis GD_RZ, Brake, or Rudder depending on the device. Instead of GD_X or GD_Y a car controller might call it steering and accelerator. Instead of a slider, a joystick might list a throttle. The Linux version had some of these too and I thought it better than not just to check in case. I realized I did put the wrong link into the comment however. It should've been this one: https://msdn.microsoft.com/en-us/library/windows/hardware/ff538338(v=vs.85)....
OK.
By the way, that reminds me: with respect to your commit log, if there's anything in there, such as the v2, v3, etc. changes, that shouldn't actually be in the ultimate commit log, put it below the "---" line. The Signed-off-by line has to be above that line, though. So, depending on how you're doing things, you may need to use git-format-patch to create files, edit those files to put in the notes, and then use git-send-email to send those files. (There's also the git-notes functionality that you can use for this, but I haven't spent the time to figure out how to use it.)
It seems that treating kIOHIDElementTypeInput_Axis the same as
kIOHIDElementTypeInput_Misc is a separate issue from adding support for the Sim page inputs. Or is there an interrelationship that I'm not aware of? Is it the case that both Axis and Misc elements can be present with either GD or Sim usage pages?
Some of the debug logging statements have been changed to say "kIOHIDElementTypeInput_Axis/Misc" while others have been left as "kIOHIDElementTypeInput_Misc". Is that indicative of something or just an oversight?
Mostly it's because I wasn't sure how the Sim page axes would be listed - under Axis or under Misc. For instance, I was very surprised to see the Generic_Desktop axes listed under Misc instead of Axis. Do you know why that is?
No, I don't. Maybe it's the fact that all of the GD things are treated alike and, since some aren't axes, they can't go under Axis.
If the GD_X_Axis is under Misc, not Axis, what's under Axis?
I don't know. I suspect it just depends on how the device manufacturer designed the device and its HID reports.
The Mac HID documentation was even more confusing than the Microsoft documentation! (at least for me). The changes in the debug statements is because the general/default case could now be a Axis or Misc type, but the Generic_Desktop page axes are known Misc types.
Hmm? I don't follow how you know that those axes are Misc types (other than by the preexisting state of the code, which may not be reliable).
You're right, I was just going by the current code and that it seems to work for my joystick, but I don't actually know that these axes will be defined as type Misc all the time ... though maybe whoever wrote the original Mac dinput code (Aric?) knew they should generally/always be defined as Misc-type somehow? I haven't been able to find any resources on it. (I've changed all the Traces to Axis/Misc in the latest patch)
So I decided one but not the other. Not changing the Sim page axes debug statement on line 496 to Axis/Misc to reflect my lack of knowledge as to how they would be listed was indeed an oversight. However, we can go back to ignoring the reported Axes elements, but I noticed the dinput code didn't ignore them and neither did several other examples of OS X joystick programs I found on the web. So I thought, why not just have the axis-type fall through, since I'm not actually sure what falls under axis versus misc, and try to catch everything? That was my thinking.
I think it's good to handle kIOHIDElementTypeInput_Axis. I just think it's a separate issue from the Sim page and so deserves a separate patch.
For what it's worth, the handling in dinput doesn't seem like it could be right. The elements are added to the elements array but then nothing every handles them properly after that. For example, poll_osx_device_state() only handles Button and Misc.
Yeah I see what you're saying.
-Ken
participants (2)
-
DavidL -
Ken Thomases