Synchronous motion plus init & OSX Refactoring

[lysannschlegel]

Includes all changes from #14 and #26.
Yes, there were several of conflicts, but I am confident I resolved them all correctly.

This branch also moves all platform-specific read and write code into wiiuse_os_read and wiiuse_os_write.
This ensures that the same interface is used for all wiiuse_io_read and wiiuse_io_write implementations. Especially inserting the SET_OUTPUT_REPORT byte on Mac and Linux is handled within these functions now, and not outside. The callers of these functions never see this first byte.

I tested with a Wiimote and Nunchuk with and without M+ on Mac and Windows, but I don't have a Linux box with Bluetooth available.

[rpavlik]

Seems to build and work on Ubuntu 10.04 x64 with GCC and a MotionPlus, and with MXE cross-compiling for windows and running on Windows 7.

On Linux, I sometimes get the warning [WARNING] Transmitting data packet when no request was made.

On Windows, with the accelerometers turned on in the example, I occasionally get three WARNING Packet ignored. This may indicate a problem (timeout is 10 ms). messages right in a row after reporting accel. numbers. Also, on connection, I get WARNING id 1 dropping report X, waiting for 0x21 twice, where X is first 0x22, then 0x30. (This is an original wiimote that was sold with a motion plus, I'm pairing it while the motion plus is attached)

Can't test with a nunchuk because @jscasallas still has mine, if I recall correctly.

With Clang I get these build errors, fixable by adding this line to the top of os_nix.c:

#include <sys/time.h>                   /* for struct timeval */

Not so sure about the warnings about "implicit declarations"...

/home/rpavlik/src/wiiuse/src/util.c:46:2: warning: implicit declaration of function 'usleep' is invalid in C99 [-Wimplicit-function-declaration]
        usleep(durationMilliseconds * 1000);
        ^

/home/rpavlik/src/wiiuse/src/os_nix.c:236:17: error: variable has incomplete type 'struct timeval'
        struct timeval tv;
                       ^
/home/rpavlik/src/wiiuse/src/os_nix.c:236:9: note: forward declaration of 'struct timeval'
        struct timeval tv;
               ^
/home/rpavlik/src/wiiuse/src/os_nix.c:237:2: error: use of undeclared identifier 'fd_set'
        fd_set fds;
        ^
/home/rpavlik/src/wiiuse/src/os_nix.c:250:2: warning: implicit declaration of function 'FD_ZERO' is invalid in C99 [-Wimplicit-function-declaration]
        FD_ZERO(&fds);
        ^
/home/rpavlik/src/wiiuse/src/os_nix.c:250:11: error: use of undeclared identifier 'fds'
        FD_ZERO(&fds);
                 ^
/home/rpavlik/src/wiiuse/src/os_nix.c:255:4: warning: implicit declaration of function 'FD_SET' is invalid in C99 [-Wimplicit-function-declaration]
                        FD_SET(wm[i]->in_sock, &fds);
                        ^
/home/rpavlik/src/wiiuse/src/os_nix.c:255:28: error: use of undeclared identifier 'fds'
                        FD_SET(wm[i]->in_sock, &fds);
                                                ^
/home/rpavlik/src/wiiuse/src/os_nix.c:269:6: warning: implicit declaration of function 'select' is invalid in C99 [-Wimplicit-function-declaration]
        if (select(highest_fd + 1, &fds, NULL, NULL, &tv) == -1) {
            ^
/home/rpavlik/src/wiiuse/src/os_nix.c:269:30: error: use of undeclared identifier 'fds'
        if (select(highest_fd + 1, &fds, NULL, NULL, &tv) == -1) {
                                    ^
/home/rpavlik/src/wiiuse/src/os_nix.c:281:7: warning: implicit declaration of function 'FD_ISSET' is invalid in C99 [-Wimplicit-function-declaration]
                if (FD_ISSET(wm[i]->in_sock, &fds)) {
                    ^
/home/rpavlik/src/wiiuse/src/os_nix.c:281:33: error: use of undeclared identifier 'fds'
                if (FD_ISSET(wm[i]->in_sock, &fds)) {
                                              ^
/home/rpavlik/src/wiiuse/src/os_nix.c:307:6: warning: unused variable 'i' [-Wunused-variable]
        int i;
            ^

[janoc]

Hello,

On 12/10/2012 09:00 PM, Ryan Pavlik wrote:

Seems to build and work on Ubuntu 10.04 x64 with GCC and a MotionPlus,
and with MXE cross-compiling for windows and running on Windows 7.

On Linux, I sometimes get the warning |[WARNING] Transmitting data
packet when no request was made.|

That looks like a spurious message. The function event_data_write()
where this comes from is called from only a single place: events.c:276
when the report type is WM_RPT_WRITE. According to the doc, it is an
unsolicited report intended to signal acknowledgements, failures or
function results of other output reports:

http://wiibrew.org/wiki/Wiimote#0x22:_Acknowledge_output_report.2C_return_function_result

On Windows, with the accelerometers turned on in the example, I
occasionally get three |WARNING Packet ignored. This may indicate a
problem (timeout is 10 ms).| messages right in a row after reporting
accel. numbers. Also, on connection, I get |WARNING id 1 dropping report
X, waiting for 0x21| twice, where X is first 0x22, then 0x30. (This is
an original wiimote that was sold with a motion plus, I'm pairing it
while the motion plus is attached)

I have seen this too, I strongly suspect this is very much due to the
bugs in the Windows BT stack.

0x22 is the unsolicited report described above, 0x30 is buttons report.

The message on connection is probably just too verbose, I suspect it
happens when some data (probably calibration or expansion ID) from the
Wiimote's memory is being read (waiting for report 0x21) but some other
report has arrived first and was dropped. I have seen that a lot when
debugging the Motion+ - you send a request to read some data from the
Wiimote and expect a reply. However, in the meantime you get several
unrelated reports, such as buttons or IR and only after some time the
correct report arrives. That was the main reason why the synchronous
init was required - if those other reports were processed before the
init was concluded, it was throwing off the state machine and performing
calculations with uninitialized data (such as the calibration).

Not so sure about the warnings about "implicit declarations"...

Looks like a missing header: #include <unistd.h>
I don't have Clang installed so I cannot test it, but usleep is declared
there.

Regards,

Jan

[rpavlik]

The catch is that I'm pretty sure #include <unistd.h> is there...

[janoc]

On 12/11/2012 01:25 AM, Ryan Pavlik wrote:

The catch is that I'm pretty sure #include is there...

Could it be that the #include relies on some "gcc-ism" that Clang
doesn't implement? Like #import or some preprocessor symbol not defined
by Clang?

It is system header, so it would not surprise me all that much.

Jan

[rpavlik]

You can pull my updates to your pull request from my osx-refactor_sync-mplus branch. Thanks so much for working on this!

Still get these warnings that make no sense to me given that I've got an #include <unistd.h> in there, but I'm not going to worry about it right now:

/home/rpavlik/src/wiiuse/src/util.c:46:2: warning: implicit declaration of function 'usleep' is invalid in C99 [-Wimplicit-function-declaration]
        usleep(durationMilliseconds * 1000);
        ^

/home/rpavlik/src/wiiuse/example-sdl/sdl.c:371:2: warning: implicit declaration of function 'usleep' is invalid in C99 [-Wimplicit-function-declaration]
        usleep(200000);
        ^
/home/rpavlik/src/wiiuse/example/example.c:454:2: warning: implicit declaration of function 'usleep' is invalid in C99 [-Wimplicit-function-declaration]
        usleep(200000);
        ^

[rpavlik]

OK, well, I've tested this (with my modifications - @lysannkessler please pull from my same-named branch and push it to this pull request) fairly well - see https://github.com/rpavlik/wiiuse/wiki/Compatibility for the nitty-gritty. If someone else could test, particularly with some non-official peripherals and/or a Mac, that would be great. (I'd really like to merge this and release 0.15!)

@lysannkessler - please also add yourself to the README and update the changelog how you see fit. Once it's been merged, we'll get @janoc to add to the changelog, etc.

[lysannschlegel]

Also, on connection, I get "WARNING id 1 dropping report X, waiting for 0x21" twice, where X is first 0x22, then 0x30. (This is an original wiimote that was sold with a motion plus, I'm pairing it while the motion plus is attached)

0x22 is the unsolicited report described above, 0x30 is buttons report.
The message on connection is probably just too verbose, I suspect it happens when some data (probably calibration or expansion ID) from the Wiimote's memory is being read (waiting for report 0x21) but some other report has arrived first and was dropped. I have seen that a lot when debugging the Motion+ - you send a request to read some data from the Wiimote and expect a reply. However, in the meantime you get several unrelated reports, such as buttons or IR and only after some time the correct report arrives. That was the main reason why the synchronous init was required - if those other reports were processed before the init was concluded, it was throwing off the state machine and performing calculations with uninitialized data (such as the calibration).

I added the warning in the case messages are dropped in wiiuse_wait_report, mainly for debugging purposes. Would you suggest to change the log too WIIUSE_DEBUG instead of WIIUSE_WARNING?

[rpavlik]

On Wed, Dec 12, 2012 at 1:34 PM, Lysann Schlegel
notifications@github.comwrote:

I added the warning in the case messages are dropped in
wiiuse_wait_report, mainly for debugging purposes. Would you suggest to
change the log too WIIUSE_DEBUG instead of WIIUSE_WARNING?

Probably a good idea, since it seems like occasional messages of this type
are harmless, and a debug build would be the approach for troubleshooting
anyway.

Ryan Pavlik
HCI Graduate Student
Virtual Reality Applications Center
Iowa State University

rpavlik@iastate.edu
http://academic.cleardefinition.com

[lysannschlegel]

Still get these usleep warnings that make no sense to me given that I've got an #include <unistd.h> in there, but I'm not going to worry about it right now.

I want add this to the discussion, but probably we don't want to bother about the warnings right now.

[rpavlik]

I've "moved" the issue about warnings to #36 to be tracked separately.

[lysannschlegel]

I pulled your changes and added myself to the list of contributors :)
The changelog looks fine to me.

[rpavlik]

Great, thanks!

[rpavlik]

@lysannkessler I've added you as a collaborator on this repo. You're still welcome to use pull requests (within this repo between branches, if you like, for easier collaboration), but at the same time also welcome to just commit if you wish (just as @janoc and @jscasallas can)