Re: [dinput] initial support for BSD's usbhid joysticks
Christoph Frick wrote:
From 7618ceb6941c0dffac9c48c1fa1aeb7e821673a2 Mon Sep 17 00:00:00 2001 From: Christoph Frick <frick(a)sc-networks.com> Date: Sat, 26 Apr 2008 02:50:01 +0200 Subject: [PATCH] Initial support for BSD's usbhid joysticks
Please consider this a tracer bullet. I had only time to test it with a 4-axis/4-button wheel in LFS in FreeBSD 7.0. I will take further tests using a flight stick and other games soon. But for now i just want to see, what others have to say about it. ---
Please use 4 spaces indentation, no tabs.
+ snprintf(buf, MAX_PATH, FMT_UHIDDEVS, i); + buf[MAX_PATH - 1] = 0;
snprintf always writes terminating '\0' character at the end of the string. You don't need to explicitly do that yourself.
+ if ((fd = open(buf, O_RDWR | O_NDELAY, 0)) == -1) { + TRACE("Unable to open %s read/write: %s\n", buf, strerror(errno)); + continue; + } Do you really have to open it read/write? Shouldn't read only be enough for not FF joystick?
The only way I can think of to remove code duplication is to create base joystick class. There were no reason for that with only 2 joystick drivers. With third one should really do it. Vitaliy.
snprintf always writes terminating '\0' character at the end of the string.
Only if the buffer contains enough space for the terminating null.
You don't need to explicitly do that yourself.
That's probably true in this case. --Juan
Juan Lang wrote:
snprintf always writes terminating '\0' character at the end of the string.
Only if the buffer contains enough space for the terminating null.
No (in case buffer at least 1 byte long). From man page: The functions snprintf() and vsnprintf() write at most size bytes (including the trailing null byte ('\0')) to str. This means that terminating zero is always written to the buffer (if it's at least 1 char long). I agree that above part of the man page is not exactly clear. But you can find a better explanation on the web, which spell out what is actually written. This is not the infamous strncpy that does what you said. Vitaliy.
On Sat, Apr 26, 2008 at 08:57:28AM -0600, Vitaliy Margolen wrote:
Please use 4 spaces indentation, no tabs.
what happened to original author decides? ;) i will run the uncopied parts through indent once i find the -unreadable flag for it...
+ snprintf(buf, MAX_PATH, FMT_UHIDDEVS, i); + buf[MAX_PATH - 1] = 0; snprintf always writes terminating '\0' character at the end of the string. You don't need to explicitly do that yourself.
ack
+ if ((fd = open(buf, O_RDWR | O_NDELAY, 0)) == -1) { + TRACE("Unable to open %s read/write: %s\n", buf, strerror(errno)); + continue; + } Do you really have to open it read/write? Shouldn't read only be enough for not FF joystick?
i tested beforehand with usbhidctl - and it needed r/w for just getting the state there. so i assumed it is needed. i will just try it out.
The only way I can think of to remove code duplication is to create base joystick class. There were no reason for that with only 2 joystick drivers. With third one should really do it.
Four with an potential xinput implementation. Some of the functions had subtle changes; but i guess they could be unified. if there where no FF the interface would be quite small: - find them - (un)acquire - poll I can not think of stuff, that get/setProperty would really need a special implementation for. A common JoyDev struct could hold all vital data; we could keep a private area for the "driver" around. But i guess I can go with the "release" before this changes. We can cope with the dups later on. so my next steps are: - fix above mentioned problems - test with a TM cougar (6 axes, 28 buttons) - test more games (GPL, NR2003 - especially ones, that needed patches in the linux parts some time ago: IL2 and RBR) i head for a "release" on monday - if there are no show stoppers ahead, of course. BTW: no complains about the autoconf additions? -- cu
participants (3)
-
Christoph Frick -
Juan Lang -
Vitaliy Margolen