Skip to content
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

OS X session termination fixes #231

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

sshimko
Copy link

@sshimko sshimko commented Sep 17, 2014

Note this builds on top of pull request #225 and includes the patch from that request.

No idea why, but session initialization and termination is behaving differently on OSX WRT empty bulk transfers. This fix has been tested on OS X Yosemite and Fedora 20.

Default parameter values in SendPacket and ReceivePacket
were leading to confusion in the ReceivePit function.
Empty transfer flags were passed in as timeouts.

Additionally, on the GS5, empty transfers
aren't required around the send ack packets
during Pit retrieval, and actually screw up
the state breaking the remainder of
the session.
The empty bulk transfer now appears
to behave differently on different OSes.

The pre-existing code work fine on Fedora 20,
however, PIT file transfers failed to
init due to a failure when requesting PIT
file size.
Like the previous commit, there
appears to be difering empty bulk
transfer behavior on differetnt OSes,

This fixes session termination on OSX.
Similar to previous patch, fix
empty transfers during session init on
OS X.
While not currently harmful, I mixed
up the timeout flags for recv.  Fix
that up before it becomes a problem.
Flash operations were failing on
OS X w/ timeouts after session initiation.

This patch does the usual, adjusts empty
bulk transfers.
@sshimko sshimko mentioned this pull request Oct 10, 2014
@sshimko
Copy link
Author

sshimko commented Oct 18, 2014

@Benjamin-Dobell this pull request needs more testing. I am seeing some unresolvable empty transfer conflicts between the GS5 and Note 3. They could be resolved with a couple device-specific lines in BridgeManager.cpp, but you have managed to avoid that, AFAICT.. Any thoughts on why this is now an issue? LOKE differences on the server side in different bootloaders? USB driver/libusb issues?

@Benjamin-Dobell
Copy link
Owner

@sshimko Yeah, sorry I really should respond more... I like to wait until I have answers 😉

I've tested all the current pull requests and they each have merit on some devices but unfortunately break support for other devices. I've gone to fairly great length to try avoid device specific (technically bootloader specific) functionality. Unfortunately detecting different devices isn't actually very straight forward either - all devices share just 3 USB device IDs. The only difference is the config strings they send (but they're quite broken and on several devices they even spell Samsung incorrectly!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants