add ready cb to bdm #668
add ready cb to bdm #668
Conversation
| @@ -47,6 +42,11 @@ void bdm_RegisterCallback(bdm_cb cb) | |||
| } | |||
| } | |||
|
|
|||
| void bdm_on_device_ready(int event) | |||
There was a problem hiding this comment.
The whole bdm_on_device_ready is not doing anything, should we keep this logic then?
There was a problem hiding this comment.
I know it seems pointless, it’s there so I can export the function to cdvdman in OPL; in the exported function I actually signal bdm io sema to start reads.
| @@ -15,7 +15,7 @@ | |||
| #include <usbhdfsd-common.h> | |||
|
|
|||
| // #define ASYNC | |||
| // #define DEBUG //comment out this line when not debugging | |||
There was a problem hiding this comment.
This line I suppose needs to keep commented
There was a problem hiding this comment.
Yes you’re right, this pr isn’t “complete” but more so just to get feedback on the overall changes
| @@ -705,6 +710,9 @@ static void usb_mass_release(mass_dev *dev) | |||
| dev->bulkEpO = -1; | |||
| dev->controlEp = -1; | |||
| dev->status = 0; | |||
|
|
|||
| if (devices_initialising > 0) // should handle all errors? | |||
| devices_initialising--; | |||
| } | |||
|
|
|||
| static int usb_mass_disconnect(int devId) | |||
There was a problem hiding this comment.
not sure, but is the devId now pointless as we anyway will go through all devIDs?
There was a problem hiding this comment.
No I don’t think so but this devices_initialising flag could probably done better with a dev->status flag rather than how I’m doing it currently, just waiting to hear if this method is reasonable or not before I spend more time on it
|
just curious what |
Yea callback, sorry lol this wasn’t really meant to be merged as is so I used shorthand |
|
I'm happy we are around these kinds of improvements, it makes the SDK more friendly and reactive. However, I'm not even sure what is the issue we are trying to fix, I suppose some kind of race condition when several USBs are connected. Can we create a dummy sample app located in I think that some of the logic we have in What do you think @KrahJohlito . Cheers |
Exactly, I think we need to find out the root cause of the issue first. The block device (BD) drivers I would like to keep as simple as possible. As soon as they call Any reactive events can be fired from BDM (the manager) if needed, but I know that's not possible in the case of OPL ingame, becouse OPL itself acts as a simplified BDM in that case. |
@rickgaiser @uyjulian @fjtrujy
So this is basically open for review more so than anything.. this coupled with this "fixes" the issue with OPL not launching from mass0 when dual USB is connected by not allowing reads in cdvdman until both devices are init see log after this change compared to before:
Not sure if this is the best way to go about it but its the only thing I could get to work, I tried multiple different things like adding a cb to scsi_connect instead of that workaround DelayThread.. a separate thread for ready setting etc etc basically a lot of different things with no success and this finally worked..
It does add a delay which increases boot times slightly which is not ideal I know but during tests it only seemed to iterate one cycle so really it should just be a second or two difference but a full five if only one device is connected so wait cycles could probably be reduced to 2 or 3.
Anyway that is why it is open for review, if there's improvements to be made here let me know or if its just outright the wrong way to go about it; let me know because there's not much point trying to refine it if its just the wrong way to go about solving the issue.
Thanks