Page MenuHomePhabricator

Move ADB communications to AdbClient class - to make it accessible by other components.

Authored by ovyalov on Mar 22 2015, 7:27 PM.



Move ADB communications to AdbClient class - in order to make it accessible by other LLDB components.
I'm planning to use device serial number for Android devices instead of host name within module cache - will integrate AdbClient with PlatformAndroid with a next CL.

Diff Detail

Event Timeline

ovyalov updated this revision to Diff 22445.Mar 22 2015, 7:27 PM
ovyalov retitled this revision from to Move ADB communications to AdbClient class - to make it accessible by other components..
ovyalov updated this object.
ovyalov edited the test plan for this revision. (Show Details)
ovyalov added reviewers: vharron, tberghammer.
ovyalov added a subscriber: Unknown Object (MLST).
tberghammer edited edge metadata.Mar 23 2015, 7:30 AM

Thanks for moving it out to a new class.

Please check that the adb port forward is set up correctly (not left over from a previous session) and also check that all of the port forwards are cleaned up after lldb exits (I suggest a manual test as full test suit might leak some adb forwards in case of a failure).


I think it isn't an error in this scenario if no device was found


What is the goal of this function? I think it will return m_device_id on success and if m_device_id is not set then it will fail.


Do we need a virtual destructor?


Please return an error if not exactly one device is found and remove the assert.

I think we should add asserts to check for bugs in lldb while having no connected device can be caused by almost anything.

If we found more then 1 device then (randomly) choosing one will cause some flakiness in our tests so I think it is better to report a failure here. I plan to add some way to specify the selected device from platform-android.

ovyalov added inline comments.Mar 23 2015, 11:32 AM



Good point - removed it.


Removed it at all.


Makes sense - fixed.

ovyalov updated this revision to Diff 22495.Mar 23 2015, 11:33 AM
ovyalov edited edge metadata.

Fixed accordingly to suggestions.
Please take another look.

Thank you.

tberghammer accepted this revision.Mar 23 2015, 11:56 AM
tberghammer edited edge metadata.

LGTM, but please add back the xcode project files and the Makefile changes (included in #1, removed from #2)

This revision is now accepted and ready to land.Mar 23 2015, 11:56 AM
ovyalov updated this revision to Diff 22500.Mar 23 2015, 12:07 PM
ovyalov edited edge metadata.

Added missed lldb.xcodeproj/project.pbxpro - thanks for catching this!

ovyalov closed this revision.Mar 23 2015, 2:07 PM




ovyalov (Author)