-
-
Notifications
You must be signed in to change notification settings - Fork 82
Add wireless controller syncing functionality to WPAD/LWBT #197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WHOA. ill have to look carefully at this, and test it fully but im looking forward to this! |
WOAH |
ok so, i made a checklist for this PR that i will post below. i do this because of the huge impact this change has on the community. we've been waiting for this change for a LONG time but i never got around to do it nor do i understand BT/Wiiuse enough haha. Anyway, i would like the following to be checked/done before merge:
this should be it. @Zarithya , it would be great if you could provide a small example program in the wii-examples repo that we can use/ship as an example but also use as a test to test the change :) we (the wii community) are extremely happy with the change and would love to see this merged :) |
wiiuse/io_wii.c
Outdated
if(wml->sock==NULL) | ||
wml->sock = bte_new(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can cause apps to crash and the cursors to go haywire when 2 or more Wiimotes are connected. Removing the null check seems to fix things though.
You'd be able to test for yourself if you can compile USB Loader GX, WiiFlow, Snes9xRX or any other app that can display more than 1 cursor like the system menu.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, not sure why that would be causing anything to go haywire... We don't want to create a new PCB if we already have one. And in my testing app, I have multiple cursors working just fine... What specific app are you testing with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested it with USB Loader GX and WiiFlow Lite.
Your code will work when there's 1 Wiimote connected, but if you connect a second then the cursor for player 1 will either freeze in place or it'll change to player 2 and both Wiimotes start fighting over where the cursor should be on the screen. This can eventually result in a DSI exception and the stack dump goes straight to these libogc changes.
With debug enabled like this I got the following.
if(wml->sock==NULL)
{
printf("wml->sock was null\n");
wml->sock = bte_new();
}
else
printf("err: %d / state: %d / bdaddr: %02x:%02x:%02x:%02x:%02x:%02x\n", wml->sock->err, wml->sock->state, wml->sock->bdaddr.addr[5], wml->sock->bdaddr.addr[4], wml->sock->bdaddr.addr[3], wml->sock->bdaddr.addr[2], wml->sock->bdaddr.addr[1], wml->sock->bdaddr.addr[0]);
Active Wiimote found in slot 1
wiiuse_register 0x80fa858c, bdaddr: 00:19:1d:6d:XX:XX
wml->sock was null
__wpad_linkkeynotCB
WPAD Assigning Slot 1 (used: 0x02)
__wpad_eventCB CONNECT
Active Wiimote found in slot 0
wiiuse_register 0x80fa8538, bdaddr: 00:25:a0:d3:XX:XX
err: 0 / state: 2 / bdaddr: 00:19:1d:6d:XX:XX
__wpad_linkkeynotCB
WPAD Assigning Slot 0 (used: 0x03)
__wpad_eventCB CONNECT
I'm pretty sure that second Wiimote shouldn't be trying to use the same sock as the first. It didn't originally and it didn't in an earlier version of your changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They definitely shouldn't be sharing a socket, but that code should create exactly one socket for each active slot: four slots for Wiimotes, then two for accessories like the Balance Board. It's meant to be that if a Wiimote disconnects and another one takes its slot, then the socket doesn't need to be destroyed, just redirected. It works in my code, but there's probably a race condition somewhere since I didn't know how to properly avoid those... Will look into this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, I made you a test app. If you check your invites you should find the link to the private repo.
It seems like the issue occurs after you disconnect and then reinitialise Wiimotes. And that's something that we'll typically do when we want to reload to a different IOS.
Also, Wiimotes won't reinit anymore after calling WPAD_Init();
printf("Wiimote LED should be on\n");
sleep(3);
// A lot of apps seem to loop through like this (including Priiloader)
for (int i = 0; i < WPAD_MAX_DEVICES; i++)
WPAD_Disconnect(i);
WPAD_Shutdown();
printf("Wiimote LED should be off\n");
sleep(3);
WPAD_Init();
printf("Wiimote LED should be on again, but it remains off\n");
sleep(3); If you skip calling Thank you so much for sharing this with the community though. This has been on our wishlists for a long time 😊 |
@wiidev The bug is actually with Wiimotes reconnecting if you don't call Of course, Wii games are able to disconnect Wiimotes and have them reconnect later, so I'm not sure how to properly replicate that. Maybe sniffing Bluetooth packets sent between the Wii menu and a Wiimote is in order... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, this is my first passthrough of the change. let this not demotivate you please. i love this and i hope you will see this through!
we would also love to see the test app ^^
@@ -40,6 +41,7 @@ distribution. | |||
static int __conf_inited = 0; | |||
static u8 __conf_buffer[0x4000] ATTRIBUTE_ALIGN(32); | |||
static char __conf_txt_buffer[0x101] ATTRIBUTE_ALIGN(32); | |||
static int __conf_buffer_dirty = FALSE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use actual bool
instead of the weird BOOL
stuff
if (fd < 0) | ||
return fd; | ||
|
||
ret = IOS_Write(fd, __conf_buffer, 0x4000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would add a define with the value of 0x4000
and call it something like ConfFileSize
. its just that the 0x4000
looks iffy at first, but upon closer inspection the file is always 0x4000
(or so wiibrew says).
so maybe the define + comment to explain would be good.
after that, maybe alter the reading too :D
if (ret < 0) | ||
return ret; | ||
|
||
/*ret = __CONF_WriteTxtBuffer(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in comments and not implemented, can be removed :)
@@ -174,6 +190,10 @@ s32 CONF_GetRegion(void); | |||
s32 CONF_GetArea(void); | |||
s32 CONF_GetVideo(void); | |||
|
|||
s32 CONF_SetPadDevices(const conf_pads *pads); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stupid minor change, but i'd put the Get & Set's together :)
@@ -181,7 +181,7 @@ typedef s8 err_t; | |||
* \hideinitializer | |||
*/ | |||
#define LOGGING 0 | |||
#define ERRORING 0 | |||
#define ERRORING 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe leave the error log off :p
default: | ||
printf("__wpad_eventCB(%d)\n", event); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment
return ERR_CONN; | ||
} | ||
|
||
static s8 __wpad_linkkeyreqCB(void *arg,struct bd_addr *pad_addr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put this in comments, its unused but useful if the bug you noticed would need fixing in the future
return result; | ||
} | ||
|
||
static s32 GetActiveSlot(struct bd_addr *pad_addr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused ? what is the use of this function?
same for GetRegisteredSlot and GetGuestSlot
@@ -138,12 +141,43 @@ int wiiuse_register(struct wiimote_listen_t *wml, struct bd_addr *bdaddr, struct | |||
return 0; | |||
} | |||
|
|||
int wiiuse_connect(struct wiimote_listen_t *wml, struct bd_addr *bdaddr, u8 *name, struct wiimote_t *(*assign_cb)(struct wiimote_listen_t *wml, u8 err)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a problem and weird. in both wiiuse upstream as our fork wiiuse_connect
's signature is int wiiuse_connect(struct wiimote_t** wm, int wiimotes)
. how does it compile this? can we make a connect work with that signature?
i ask this because switching to upstream means that wiiuse_connect will be the mentioned signature, and pass the info to a OS (in this case libogc) implementation of the connect function.
the wiimote_t should have all the info you pass here afaik. could you check?
wiiuse: https://github.com/wiiuse/wiiuse/blob/master/src/wiiuse.h#L730
current libogc fork : https://github.com/devkitPro/libogc/blob/master/gc/wiiuse/wiiuse.h#L594
aha, so the disconnect actually working as designed now haha |
ive logged in dolphin the following when in SM i boot a game:
then the game boots, as code was running @80003400 which is no longer SM:
this doesn't say much to me, but i suspect it only reset the interface, but not actually disconnect wiimotes.
after this, SM boots and is running code @80003400 so i think, but would love @Zarithya & @wiidev's input on this, that we fixed the disconnect bug, but that on the sdk its left in searching state because SDK doesn't actively disconnect them unless its shutting down the system, which i saw when shutting down in dolphin when in SM :
|
Updates on this? |
zari was at a con last weekend and has been recovering this week. we haven't forgotten about this aep :) |
This PR makes sweeping changes to how controllers are connected in homebrew apps, allowing for the pairing of new controllers without leaving a homebrew environment. This can be done either through pressing the red sync button on the front of the console (which is also exposed to homebrew authors via
WPAD_SetSyncButtonCallback
) for permanent pairing or by callingWPAD_Search
/WPAD_StopSearch
, which replicates the "Reconnect" button on the Home Menu and allows for guest controllers to be connected temporarily.I wrote this as a convenience so I don't have to keep exiting to the Wii menu every time I want to test a new controller (I refurbish a lot of Wii controllers) but I figured it could be of use to others writing homebrew. This is my first time doing anything like this (including Wii software and Bluetooth driver development) so please let me know if anything needs to be tweaked!
The SYSCONF writing functionality was borrowed from AnyRegion Changer, written by tona.