| Summary: | [Patch] Add accelerometer and gyroscope APIs | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Jonathan Newman <jonathannewman314> |
| Component: | joystick | Assignee: | Sam Lantinga <slouken> |
| Status: | WAITING --- | QA Contact: | Sam Lantinga <slouken> |
| Severity: | enhancement | ||
| Priority: | P2 | CC: | amaranth72, jonathannewman314, meyraud705 |
| Version: | HG 2.0 | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: |
Add accelerometer and gyroscope APIs
Add accelerometer and gyroscope APIs (with correct dynapi entries) PS4 hidapi implementation PS4 hidapi implementation update1 PS4 implementation update 2 |
||
Created attachment 3978 [details]
Add accelerometer and gyroscope APIs (with correct dynapi entries)
Guess who forgot to run gendynapi.pl? Added fixed patch.
Could this be integrated with the SDL_Sensor API? (In reply to Alex Szpakowski from comment #2) > Could this be integrated with the SDL_Sensor API? SDL_Sensor seems to be focused on built-in always-available sensors intended for phones, whereas my mental model of the accel+gyro in gamepads is as an additional high-precision analogue input. For that reason I didn't really consider using it. There's also the matter of Sensors having a separate Update function, whearas generally motion data is included in the normal updates from game controllers. If you have a strong preference then I can rework things to use SDL_Sensor, but I would prefer that sensors remain strongly associated with their corresponding Joysticks... as opposed to the user needing to separately open a Joystick and a Sensor, and maintain a mapping between them. It could result in a fairly cumbersome bridge between the _joystick and _sensor systems. I could instead just revise the APIs to be more consistent with Sensor, though. e.g. represent gyroscope data in radians per second rather than degrees per second, and accept a single float* param instead of separate x/y/z. Let me know how you'd like me to proceed. I think the idea with the Sensor APIs was to eventually extend them to work with Joysticks, similarly to how the haptic API can work with joysticks ( https://twitter.com/icculus/status/1032105900981530624 ). I agree that it would be ideal to obtain a joystick's sensor data without having to iterate through a separate list of sensors, I think the haptic API provides a similar concept for itself as well, so maybe you could look at how that works as a base. Created attachment 4247 [details] PS4 hidapi implementation I don't know if you are still working on this but I implemented motion control for dualshock4 hidapi driver. - API: Two functions to check for gyro and accelerometer: int SDL_JoystickHasAccelerometer(SDL_Joystick * joystick) int SDL_JoystickHasGyroscope(SDL_Joystick * joystick) Two functions to get the data: int SDL_JoystickGetGyroscope(SDL_Joystick * joystick, float *gyro_xyz); int SDL_JoystickGetAccelerometer(SDL_Joystick * joystick, float *accel_xyz); They take a pointer to an array of 3 floats unlike the original patch proposed. - Event Added a new event type SDL_JOYSENSORUPDATE that uses the same struct as SDL_SENSORUPDATE. - Joystick system Added fields to store gyro and acceleration data to _SDL_Joystick. The driver functions added by the original patch were not needed, instead I added SDL_PrivateJoystickSensors which work like others SDL_PrivateJoystick* functions. - HIDAPI PS4 I looked at the dualshock4 Linux driver to get the meaning of data from calibration report. https://elixir.bootlin.com/linux/v5.5.1/source/drivers/hid/hid-sony.c#L1631 Getting the data from gyro and accelerometer was straightforward as they already are in the PS4StatePacket_t struct. Some third-party joysticks compatible with the PS4 don't have a motion sensors. I don't know how to check the presence of sensors. Data from report is an array of Uint8, it cannot be casted to PS4StatePacket_t as it could cause misaligned read of the sensor data (Sint16). I use memcpy instead. - Misc. Added SDL_SENSORUPDATE and SDL_JOYSENSORUPDATE to SDL_LogEvent. Logging of these events are disabled by default as they spam a lot. Added SDL_JOYSENSORUPDATE to testjoystick.c. Rumble over Bluetooth only cause problem on Windows, so only disable it on Windows. (In reply to meyraud705 from comment #5) > Created attachment 4247 [details] > PS4 hidapi implementation > > I don't know if you are still working on this but I implemented motion > control for dualshock4 hidapi driver. Hey meyraud705, thank you for picking this up! I couldn't find time to get back to it. Your API is definitely better than my attempt- Fingers crossed this can be merged :) Created attachment 4329 [details]
PS4 hidapi implementation update1
Updated patch so it applies cleanly on last revision.
(In reply to meyraud705 from comment #5) > Rumble over Bluetooth only cause problem on Windows, so only disable it on > Windows. If I remember this correctly: In generic Bluetooth mode (not the official dongle), motion/touchpad/etc data isn't sent until rumble has been enabled. But enabling rumble also causes the pad to stop working with DirectInput applications until the pad is turned off and on. In my opinion, SDL should just put the pad into this full functionality mode without worrying about DirectInput. It's trivial to reverse by just restarting the controller, and I doubt there are many people who use the DS4 with DI anyway (it's mostly used with third-party wrappers like Steam Input/JoyShockMapper/DS4Windows, all of which enable rumble and break DI). Created attachment 4514 [details]
PS4 implementation update 2
I made some improvements:
- SDL_JoystickGetSensorRate()
When you integrate the angular velocity provided by the gyro you need the time between each sample.
Using the timestamp of the event does not work (timestamp represents time when event is pushed in the queue).
If you poll event 60 time per second and the controller report event at 240, then you get 4 events with the same timestamp.
Dualshock4 and Switch controller send data at a fixed rate so expose this to the user with SDL_JoystickGetSensorRate().
- SDL_HINT_JOYSTICK_ALLOW_BACKGROUND_SENSOR_EVENTS
Joystick does not send events when app is in the background unless SDL_HINT_JOYSTICK_ALLOW_BACKGROUND_EVENTS is set to 1.
This behavior is undesirable for sensor as you lose track of the orientation and most algorithm will need time to recover after app come back to foreground.
SDL_HINT_JOYSTICK_ALLOW_BACKGROUND_SENSOR_EVENTS is added and is set to 1 by default. It controls whether or not sensor events are sent in the background and works independently from SDL_HINT_JOYSTICK_ALLOW_BACKGROUND_EVENTS.
- Switch and Steam controller
Added implementation for Switch controller. It does not read calibration data, so accuracy will be low. I do not own a switch controller so this is not tested.
Added stub implementation for Steam controller and make the file compile in c89 mode.
- Trackpad
The trackpad on the Dualshock4 sends SDL_FINGER_DOWN/SDL_FINGERUP/SDL_FINGERMOTION event.
Added ref count to touch subsystem to be sure that it is not shutdown while joystick is still using it.
- Development note
This sensor implementation for joystick does not use the sensor API. Sensor data are stored in joystick struct and events are sent by joystick function. It only reuse the SDL_SensorEvent struct, rest of it is duplicate code.
From an user point of view, sensor datas are obtained from two event types and two set of functions, which add unneeded complexity.
The way the touch API works make it easy use it from anywhere and to extend the capability of a device. This make the implementation for the Dualshock 4 trackpad short and simple: create and delete a SDL_Touch, 4 lines of code to read data from controller and 2 functions to send data to SDL_Touch object.
From an user point of view, if touch is already working in their app then no additional work is required.
If you plan to write a new subsystem or rewrite an existing one, I think it would be better to make it works like the touch API.
Hey guys, I just saw this bug after I went ahead and added touchpad and accelerometer support. I looked through what you have here, and it looks pretty good. Is there anything that you'd like to see incorporated into the new API? As I said in my previous comment, when you integrate angular velocity you need the time between each sample. Timestamp of the event is not accurate enough. In my patch I added SDL_JoystickGetSensorRate(), but having an accurate timestamp will work too. I haven't used touchpad yet, but adding gesture support could be interesting. |
Created attachment 3977 [details] Add accelerometer and gyroscope APIs Adds `SDL_JoystickGetAccelerometer` and `SDL_JoystickGetGyroscope`, with appropriate dummy implementations for all drivers. I intend to implement these for HIDAPI_DriverPS4 soon.