Patch to make BJoystick non blocking on read

[caz_haiku]

This patch allows BJoystick to be used in a non blocking way, BJoystick should not block and wait in it's Update function. Also a patch for BJoystick's GetDeviceName function, it is legal to call GetDeviceName without first using CountDevices. Also this patch has modification for JoystickProtocolHandler to allow the non blocking read. Excellent work from mmlr for implementing the BJoystick in usb_hid, does this guy ever go to sleep :-). If these changes are not acceptable, then however BJoystick is implemented it should not block in it's update function for compatibility.

[caz_haiku]

BJoystickProtocolHandler patch

[mmlr]

Those changes couldn't really work reliably with how the usb_hid framework is implemented. You'd effectively cancel the ongoing waits before actually waiting for a report. It might have worked in some cases, but the timeframe in which a report would need to arrive would be rather narrow and the results would at best scheduling dependent.

I've implemented a robust version of a non-blocking JoystickProtocolHandler in hrev41865 instead. It should already solve most of this ticket. I'll look at the BJoystick part next.

[mmlr]

I've cleaned up _BJoystickTweaker in hrev41868 while looking over it.

But I still don't agree with your BJoystick changes (as before with the other patch). The ScanDevices() method is private, so you can't force scanning from your application. Hence for the application to be able to pick up plugged in devices (appart from creating a new BJoystick object) the scanning needs to occur on demand (i.e. on CountDevices() and GetDeviceName()). With your changes it would only happen the first time however. This wasn't really an issue with the fixed gameports back then (as the gameport would be there in any case, regardless of there actually being a device attached), but with on-demand pluggable devices like USB and on-demand device entry creation, this has to be taken into account. That currently creates the highly suboptimal situation of scanning over and over, but I'll fix this later on.

I've fixed it differently in hrev41869 by using CountDevices() for the index check instead of using the count of the potentially empty devices list.

[mmlr]

Reconsidering this, having CountDevices() not stable is probably a bad idea, as it can lead to problematic situations depending on the usage pattern. Instead I'll add a public Rescan() method so that one can update the state consistently without recreating the BJoystick object.

[mmlr]

Implemented as mentioned above in hrev41870.

[caz_haiku]

Michael, Good work on this, i'll test it later this evening. The original BJoystick by Be did work in a way similar to my changes but it doesn't matter about that as long as it works as expected. The following code was legal previously which i used in BeS9x, and without the modifications no joysticks were opened. Anyway you have fixed it so it works as expected, will test later.

GameStick::GameStick(input_interface *buffer) : jbuffer(buffer)
{
	for(int i = 0; i < MAX_JOYSTICKS; i++)
		sticks[i] = 0;
	
	BJoystick stick;
	char devName[B_OS_NAME_LENGTH];
	for(int i = 0; i < stick.CountDevices() && i < MAX_JOYSTICKS; i++)
	{
		sticks[i] = new Joystick(i, buffer);
		if((sticks[i]->GetDeviceName(i, devName) != B_OK) || (sticks[i]->Open(devName, true) < B_OK))
		{
			delete sticks[i];
			sticks[i] = 0;
		} else
			sticks[i]->InitNames();
	}
	InitKeyNames();
}

[caz_haiku]

Just tested things and as expected it works great :-0. I'm very happy about this and it means a lot that you have done this work. Regarding the following comment you have in Joystick.cpp

		// Allocate the extended_joystick structures to hold the info for each
		// "stick". Note that the whole num_sticks thing seems a bit bogus, as
		// all sticks would be required to have exactly the same attributes,
		// i.e. axis, hat and button counts, since there is only one global
		// joystick_info for the whole device. What's implemented here is a
		// "best guess", using the read position in Update() to select the
		// stick for which an extended_joystick structure shall be returned.

The num_sticks thing was to do with old standard gameport Sidewinder gamepads which you could daisy chain up to 4 controllers connected to each other, so indeed they did have the same attributes.

Also the old legacy axes values horizontal and vertical should be the same as the enhanced ones IIRC.

[mmlr]

Replying to caz_haiku:

Just tested things and as expected it works great :-0.

Cool.

The num_sticks thing was to do with old standard gameport Sidewinder gamepads which you could daisy chain up to 4 controllers connected to each other, so indeed they did have the same attributes.

Yes I know, it's documented in the newsletter article IIRC. Still it's a questionable decision API design wise since just providing seperate devices for each stick would've been less complex from an app developer side of things as you wouldn't have to take into account devices and sticks, even though you're going to handle their input the same way in both cases anyway.

The "best guess" was directed at the implementation of BJoystick::Update() however, as I don't know (and can't test) how addressing the different sticks was implemented in the original BJoystick. Therefore I've just implemented it by using the stick number as position to the read call which may be the right thing, or maybe not...

Also the old legacy axes values horizontal and vertical should be the same as the enhanced ones IIRC.

Indeed, I was mislead by the legacy joystick structure data type and thought there was a need for conversion, which there isn't. I've fixed that in hrev41875.