Separation of feature reports across 3 devices in Win 8.1

[cboulay]

Can someone (@nitsch) please test this in Windows 7? Or in other Windows 8 @betonme ?
All of the relevant changes are in ifdef tags so they should only affect Windows.

With these changes the controller works well for me in Windows 8.1. I can use the psmovepair utility, magnetometer_calibration, and test_calibration (USB connection required for the first time).

[cboulay]

ugh. I was supposed to link this to #147 . Is it too late? Sorry, I'm pretty new to pull requests. Closes #147

[betonme]

I can test it with windows 8.1 within the next days

[nitsch]

I have test on 64-bit Windows 7. The psmoveapi test applications still seem to work fine with your patch. Could you add a little more detail to the comments in the code? Something that describes the "big picture", to provide a little context for this special treatment.

[cboulay]

I didn't know it would delete the pull request if I deleted the branch. I should have held on to it. Sorry. It was maybe incomplete because getting the move controller working on Windows is not terribly useful without the tracker. I finally got the tracker working reliably, but I had to unlink the PS3EYEDriver from its origin repository.

Maybe I could have maintained my own fork of PS3EYEDriver then worked off that? I don't know. Sorry, I'm new to pull requests and submodules.

[cboulay]

Oh hey I can restore and reopen it! I'll keep it open since it is a minimal fix for only the psmove controller (not the tracker). Then we can debate the merits of unlinking PS3EYEDriver another time.

[cboulay]

@betonme , did you get a chance to test this yet?

@nitsch mentioned in a forum post that the best Windows solution will likely be to use Windows HID API, which I presume will make more sense than the 3 different devices.

I'm not willing to tackle that problem, but I will bet on you to provide us with the most robust Windows solution. (Sorry, menotpunny).

[thp]

Indentation

[thp]

In general, this pull request has some indentation issues. Other than that looks good, but I haven't tested it on Windows.

[betonme]

I can compile it, but the pairing is never ending.

[cboulay]

@betonme Is that on Windows 8.1? Are you referring to the "Please unplug the USB cable and press the PS button." part? It's also never-ending for me, because there are still some changes to be made in Windows before it will pair. I suppose at this point we could check for Windows 8.1, cancel the pairing, then instruct the user to check the box in the bluetooth device services and edit the registry setting.

The better solution would be to make the device settings and registry setting changes in code, maybe restart the services, then use the Windows API for pairing (as in psmove-pair-win). That's well beyond this simple first-step.

As for the indentation, it looked OK on my screen. I'll try another editor.

[betonme]

Yes I'm using Windows 8.1 x64. And You're right I'm referring to the "Please unplug the USB cable and press the PS button." part. It would be nice to have all the hacks done automatically in psmovepair.

[cboulay]

I agree it would be nice.

This PR does not address the unreliability of connecting the controller in Windows 8.1 . I think that's a separate issue entirely and should probably be addressed separately.

This PR merely fixes how the API communicates with the controller once connected (either via USB or BT). Because it fixes USB communication, it makes it possible to write the host address to the controller, so the controller is now actually paired with the computer. (I bet that anyone who previously had any measure of success pairing in Windows 8.1 was using a dongle that they paired with on a different platform.) Because it can now get past the pairing phase, it exposes the connection problem.

I think the narrow scope of this PR still warrants adding a timeout during the attempt to connect to the controller using the psmovepair utility, and if it reaches timeout to give instructions to read the README.win64 section on bluetooth connection. I don't have time to work on that until later next week.

Thanks for your feedback!

[cboulay]

@thp , how would you like to handle the infinite connecting problem in Windows at this early stage? A timeout? A windows-only message to Ctrl+C? I know the long-term solution would be to figure out how to make all the necessary device setting + registry changes in code then restart the BT service (assuming that works), but for now, what would you like done?

[thp]

A generous timeout (30 seconds? not sure..) would work for me, maybe with a message as well that it's buggy on Windows, will time out in %d seconds and the user can press Ctrl+C to abort early.

[cboulay]

The latest patch includes a timeout during pairing. It builds, but I cannot test it beyond that. @betonme can you give it a shot?

[nitsch]

The latest patch includes a timeout during pairing.

What exactly is that timeout good for? Isn't mentioning Ctrl+C to abort
sufficient? For instance, I remember spending more than 30 seconds on
Windows 7 to get one particular Move controller connected.

Why not just display all the information prior to starting the loop and
let the user decide when to give up? It's much simpler.

[betonme]

@cboulay Hi, it builds and here is the print out of psmovepair.exe

Connected controllers: 1
[PSMOVE WARNING] Magnetometer in 00:11:22:33:44:55 not yet calibrated.
PSMove #1 connected via USB.

Unplug the controller.

Now press the controller's PS button. The red status LED
will start blinking. Whenever it goes off, press the
PS button again. Repeat this until the status LED finally
remains lit. This process will timeout in 30 seconds.
Press Ctrl+C to abort early.

A connection could not be established. This is not
unusual in Windows. Please refer to the README document
for your plastform for more information.
A connection could not be established. This is not
unusual in Windows. Please refer to the README document
for your plastform for more information.
A connection could not be established. This is not
unusual in Windows. Please refer to the README document

The message continues until I hit Ctrl+C

[cboulay]

I forgot to make it break the loop automatically, so it seems it would not have prevented @nitsch from connecting after more than 30 seconds. I clarified the text a little more. Thanks @betonme

[thp]

Typo: plastform -> platform

[thp]

Most of my comments here are about code style and indentation. I haven't tested it on Windows -- can anybody else report if it works and if it should be merged?

[cboulay]

I squashed and rebased against master. The diff looks a bit more complicated than necessary because rebasing ended up undoing/doing indentation fixes.

[thp]

Looks mostly good now, haven't tested it, though. Let's see if others report any issues in the coming days.

[cboulay]

Someone that is helping me with my UE4 plugin couldn't get it to work with Windows 8.0 but it worked fine with Windows 8.1. Based on his description of the behaviour and some very simple debugging, it seems that the problem occurred when attempting to open the found BT controller, so it might be related to this part of the code. However, it also would not have worked (reliably) without these changes, so I don't think we're introducing any regressions. Further, I don't intend to support Windows 8.0. The upgrade to 8.1 is free and 8.1 is significantly better in many aspects.

At some point it might be necessary to use platform-specific communication API (hidapi in Linux, OSX, and Windows' bluetooth API in Windows). My guess is that this would solve a lot of problems with pairing and communication.