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

[TIMOB-24900] Support Android O changes #14

Merged
merged 6 commits into from
Aug 10, 2017

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Aug 7, 2017

  • Increase 100ms delay to 200ms before sending command
  • Implement waitForResponse for trackDevices as the initial response may not contain any response data
  • ps now requires the -A parameter to list all process results. Implement backwards-compatible adb.ps method
  • Fix getPid to use new adb.ps method
  • Remove -no-boot-anim as this flag prevents the Android O emulator from booting
  • Increase -partition-size to 512

JIRA Ticket

Copy link
Contributor

@cb1kenobi cb1kenobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't working. When I connect a device and run the adb test, sometimes it works, but sometimes it exits if there is no device found.

$ node tests/test-adb.js
[1] CONNECTED
[1] SENDING host:track-devices
[1] RECEIVED 4 BYTES (state=1) (cmd=host:track-devices)
[1] BUFFER LENGTH = 4
[1] RESULT OKAY
[1] DONE, SETTING STATE TO WAIT_FOR_RESPONSE
[1] RECEIVED 20 BYTES (state=4) (cmd=host:track-devices)
[1] BUFFER LENGTH = 20
[1] DONE, RECEIVED RESPONSE
[1] SOCKET CLOSED BY SERVER 20
[2] CONNECTED
[2] SENDING host:transport:001006de0b00
[2] RECEIVED 4 BYTES (state=1) (cmd=host:transport:001006de0b00)
[2] BUFFER LENGTH = 4
[2] RESULT FAIL
[2] ERROR!
[2] RECEIVED 35 BYTES (state=0) (cmd=host:transport:001006de0b00)
[2] SOCKET CLOSED BY SERVER null
[3] CONNECTED
[3] SENDING host:transport:192.168.56.101:5555
[3] RECEIVED 4 BYTES (state=1) (cmd=host:transport:192.168.56.101:5555)
[3] BUFFER LENGTH = 4
[3] RESULT FAIL
[3] ERROR!
[3] RECEIVED 42 BYTES (state=0) (cmd=host:transport:192.168.56.101:5555)
[3] SOCKET CLOSED BY SERVER null
[4] CONNECTED
[4] SENDING host:transport:192.168.56.101:5555
[4] RECEIVED 46 BYTES (state=1) (cmd=host:transport:192.168.56.101:5555)
[4] BUFFER LENGTH = 46
[4] RESULT FAIL
[4] ERROR! device '192.168.56.101:5555' not found
Devices:
[ { id: '001006de0b00', state: 'device', emulator: false } ]

[4] SOCKET CLOSED BY SERVER null

I tried testing it before your changes and it also isn't working as expected. It doesn't find the connected device the first time, but it also doesn't exit and continues to listen for devices.

I'm starting to think maybe we should switch to something like adbkit.

@garymathews
Copy link
Contributor Author

@cb1kenobi Updated PR

lib/adb.js Outdated
}
}
}.bind(this);
this.shell(deviceId, 'ps -A', output);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ps -A does nothing on my device.

chris@yojimbo (tmp):~/appc/node-titanium-sdk$ adb shell
shell@flo:/ $ ps -A
USER      PID   PPID  VSIZE  RSS   WCHAN            PC  NAME
shell@flo:/ $
shell@flo:/ $ ps
USER      PID   PPID  VSIZE  RSS   WCHAN            PC  NAME
root      1     0     2216   820   sys_epoll_ 00000000 S /init
root      2     0     0      0       kthreadd 00000000 S kthreadd
root      3     2     0      0     run_ksofti 00000000 S ksoftirqd/0
root      6     2     0      0     cpu_stoppe 00000000 S migration/0
root      16    2     0      0     rescuer_th 00000000 S khelper
root      17    2     0      0     rescuer_th 00000000 S suspend_sys_syn
root      18    2     0      0     rescuer_th 00000000 S suspend
root      23    2     0      0     irq_thread 00000000 S irq/203-msmdata
<snip>

I see you are checking if the output prints an error, but running ps -A on my device does not display an error. You basically need to check for the bad pid '-A' OR if it just returned the header. Maybe even run ps by default and fallback to ps -A if needbe.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, what Android OS are you running?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android 6.0.1, which is the latest version I can get on my 2013 Nexus 7.

@garymathews
Copy link
Contributor Author

@cb1kenobi Updated PR

lib/adb.js Outdated
@@ -580,7 +580,8 @@ ADB.prototype.ps = function ps(deviceId, callback) {
callback(err);
} else {
// old ps, does not support '-A' parameter
if (data.toString().startsWith('bad pid \'-A\'')) {
var dataStr = data.toString();
if (dataStr.startsWith('bad pid \'-A\'') || dataStr.endsWith('NAME\r\n')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that Android is Linux based, are you sure there's a \r character? Might be safer to do var dataStr = data.toString().trim(); and then do the endsWith().

lib/adb.js Outdated
* @param {ADB~psCallback} callback - A callback that is fired once ps is executed
*/
ADB.prototype.ps = function ps(deviceId, callback) {
var output = function (err, data) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

output is a bad variable name since it is a function and not a buffer/string with "output".

lib/adb.js Outdated
@@ -575,20 +575,20 @@ ADB.prototype.installApp = function installApp(deviceId, apkFile, opts, callback
* @param {ADB~psCallback} callback - A callback that is fired once ps is executed
*/
ADB.prototype.ps = function ps(deviceId, callback) {
var output = function (err, data) {
var outputCallback = function (err, data) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will work.

@cb1kenobi cb1kenobi merged commit 2cf616b into tidev:master Aug 10, 2017
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.

None yet

2 participants