x360: Add support of rumble

[rofferom]

Here is a draft of a rumble API for Xbox360 controllers.

The implementation is dirty (some .unwrap() must be removed), but it is a start point to discuss about the API and its implementation.

[CasualX]

Hey thanks for the PR. When I made the initial implementation I punted on the notification API because it was really difficult to figure out how to do this elegantly.

Your current code seems based on the same strategy as the official C API which I think is problematic.

First the creation of a separate thread to handle notifications is not nice in principle. It also requires extra thread safety for getting the results out of the notification callback back into your main processing logic.

I see two ways to handle this API better:

1. Polling based approach.

   Eg. video games are invoked in a loop, they constantly poll the controller anyway, they might as well also poll for notifications. It's done through a blocking call to DeviceIoControl. I think this can be made non-blocking but the buffer must remain live for the duration of the operation (can be fixed by using a heap indirection to keep a stable pointer to the receiving buffer). This buffer can live in the xbox struct.

2. Async support.

   Unfortunately I'm not too familiar with async in Rust or how exactly to integrate it with that and keeping in mind requirements like buffer lifetimes that must remain valid for the duration of the DeviceIoControl.

Finally there is the concern of dropping the xbox struct. The Rust code demands that the user can drop the xbox struct at any time in 2 different ways: Either implicitly through the Drop trait or explicitly through inherent drop method which returns the client connection. Your current implementation crashes and burns if you use that method instead.

I think I'm personally in favor of implementing 1. over the callback appraoch and using stuff like CancelIo. I haven't done that yet because I'm not super familiar with the semantics of these operations and whether vigembus cleanly supports all of this.

[CasualX]

I pushed a branch notification to attempt to implement the polling approach.

However I can't seem to get any notifications after setting state through rusty_xinput...

[rofferom]

I don't like the strategy I have implemented, I would prefer to have a single thread for all rumbles. But I wanted to make a simple prototype to open the discussion.

I get your point about integration of polling in game loop-like application. However, I'm not sure this matches my usecase (which is specific I think). Actually I don't have this kind of event loop. I have instead a network thread that receives some events that describe a gamepad action (Button press, Axis position), and I'd like to get rumble notifications as soon as possible to send them over the network quickly. That's why the threaded approach isn't a problem for me.

I think the usecase you describe and mince are not matching very well, I don't really know what kind of API to suggest.

[CasualX]

Ah I remember another issue I have with the callback based approach: knowing which target it belongs to if you have multiple. So you get a notification and then what? how do you know which target it belongs to? These things can be worked around but it's all so inelegant.

[CasualX]

Ok I solved all problems I could find, can you take a look at the 'notification' branch? It contains an example usage:

https://github.com/CasualX/vigem-client/blob/notification/examples/notification.rs

[rofferom]

Ah I remember another issue I have with the callback based approach: knowing which target it belongs to if you have multiple. So you get a notification and then what? how do you know which target it belongs to? These things can be worked around but it's all so inelegant.

My approach was to let the user define a trait implementation to make a custom mapping on its side. Actually I have just copied the SDL2 audio API to solve the issue

What other approches do you have in mind ?

[rofferom]

Ok I solved all problems I could find, can you take a look at the 'notification' branch? It contains an example usage:

https://github.com/CasualX/vigem-client/blob/notification/examples/notification.rs

I think the drawback of this approach is that the user has to manage the threads itself, but it's cool to be able to poll or get some kind of notification. Do you think it would be possible to have an API to let the library spawn the thread automatically to get the notification, and still expose the non-blocking poll API ?

It would be better to spawn a single thread to monitor all plugged gamepads notifications, but this would require a more complex communication to the notification thread to request it to update its list of gamepad to monitor. That's why I have reproduced the simple approach of the original C implementation to prototype the API.

[CasualX]

Ah I remember another issue I have with the callback based approach: knowing which target it belongs to if you have multiple. So you get a notification and then what? how do you know which target it belongs to? These things can be worked around but it's all so inelegant.

My approach was to let the user define a trait implementation to make a custom mapping on its side. Actually I have just copied the SDL2 audio API to solve the issue

What other approches do you have in mind ?

Depends on the use case, if all you need is simulate a single target then no complications are necessary, any callback is necessarily associated with the only target that your application creates. Regardless of the solution for multiple targets, I don't want to force complexity of Arc, Mutex etc... on the user of the API even if they end up only creating a single target.

If going by using a thread for each target to handle its notifications you're pretty much forced to use Arc, Mutex to wrap any shared state that you want to be able to be affected by the notifications.

Providing a generic API for this is just so complex and probably won't even solve the actual problem whereas providing the low level tools (and examples to show how to use them) is a better approach I think.

Ok I solved all problems I could find, can you take a look at the 'notification' branch? It contains an example usage: https://github.com/CasualX/vigem-client/blob/notification/examples/notification.rs

I think the drawback of this approach is that the user has to manage the threads itself, but it's cool to be able to poll or get some kind of notification. Do you think it would be possible to have an API to let the library spawn the thread automatically to get the notification, and still expose the non-blocking poll API ?

I can probably get away with making a helper function for the specific API as designed by the official client sdk without forcing every user of the API to pay for the costs.

In the meantime I'd appreciate if you could check if the implementation works. I noticed I'm getting a lot of spurious notifications with all fields set to 0. The notifications example prints Got 23 notifications! when only 20 notifications were expected...

It would be better to spawn a single thread to monitor all plugged gamepads notifications, but this would require a more complex communication to the notification thread to request it to update its list of gamepad to monitor. That's why I have reproduced the simple approach of the original C implementation to prototype the API.

Unfortunately once you start dabbling with 'handle multiple X in an event based fashion on a single thread' you're pretty much talking async. You want to be able to wait until any of the targets receive notification and then act on it. I consider this out of scope until I better understand the async mechanism in Rust as well as how exactly it interacts with Windows native resources (eg. how does GetOverlappedResult tie into Rust's async story?)

[CasualX]

I've updated notification branch, I added spawn_thread method on the request notification instance that handles the boilerplate.

[CasualX]

Hi I've merged my notification implementation, not yet published on crates io, but let me know if you find any issues

[rofferom]

In the meantime I'd appreciate if you could check if the implementation works. I noticed I'm getting a lot of spurious notifications with all fields set to 0. The notifications example prints Got 23 notifications! when only 20 notifications were expected...

I have noticed this kind of issues with the native implementation. I always get two events during unplug : Left Y and Right Y axis that are notified with a 0 value.

I'm going to update my implementation today with what you have done. Thank for you responsiveness (better than mine :nerd_face:).

[CasualX]

Np! No worries, sometimes I also take my time to respond to issues, if at all...