Skip to content

Implement Android debugging with unix sockets redirection #1373

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

Merged
merged 1 commit into from
Jan 6, 2016

Conversation

teobugslayer
Copy link
Contributor

This is cleaned-up version of #1227
To test it, you need this runtime.

This pull request is releated to the changes here.

The android debugger now uses named sockets instead of ports hence the code rewrite in adndroid-debug-service to accommodate for that.
Contact @blagoev directly in order to merge this successfully into the respected main branches.

@teobugslayer teobugslayer self-assigned this Jan 5, 2016
@teobugslayer teobugslayer added this to the 1.6.0 (Under consideration) milestone Jan 5, 2016
@teobugslayer teobugslayer force-pushed the totev/android-debugger-unixsockets branch 2 times, most recently from b0971a1 to a289bee Compare January 6, 2016 07:19
let forwardsResult = this.device.adb.executeCommand(["forward", "--list"]).wait();

//matches 123a188909e6czzc tcp:40001 localabstract:org.nativescript.testUnixSockets-debug
let regexp = new RegExp(`(${deviceId} tcp:)([\\d])+(?= localabstract:${packageName}-debug)`, "g");
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that this RegExp could be rewritten as follows:

new RegExp(`(?:${deviceId} tcp:)([\\d]+)(?= localabstract:${packageName}-debug)`, "g");

The benefits would be that you could execute regexp.exec(forwardsResult) on the line below instead of forwardsResult.match(regexp). That way the result port would simply be match[1] instead of parseInt(match[0].substring(match[0].length - 5));

Example:

let deviceId = "123a188909e6czzc";
let packageName = "org.nativescript.testUnixSockets";
let forwardsResult = "123a188909e6czzc tcp:40001 localabstract:org.nativescript.testUnixSockets-debug";

let regexp = new RegExp(`(?:${deviceId} tcp:)([\\d]+)(?= localabstract:${packageName}-debug)`, "g");
let match = regexp.exec(forwardsResult);
console.log(match);
// output
/* [ '123a188909e6czzc tcp:40001',
  '40001',
  index: 0,
  input: '123a188909e6czzc tcp:40001 localabstract:org.nativescript.testUnixSockets-debug' ]
*/

Copy link
Contributor

Choose a reason for hiding this comment

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

good one.
the best way should have been with a negative lookbehind... Not sure if exec is the optimal way thus I have done this with substring, which is ugly.

I leave it to you guys to decide.

@Mitko-Kerezov
Copy link
Contributor

Overall great work!
👍 after addressing comments

@teobugslayer teobugslayer force-pushed the totev/android-debugger-unixsockets branch from a289bee to e743e9c Compare January 6, 2016 09:19
let regexp = new RegExp(`(?:${deviceId} tcp:)([\\d]+)(?= localabstract:${packageName}-debug)`, "g");
let match = regexp.exec(forwardsResult);
if (match) {
port = parseInt(match[0].substring(match[0].length - 5));
Copy link
Contributor

Choose a reason for hiding this comment

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

port=match[1];
Because the regex is now exec-ed instead of match-ed

@teobugslayer teobugslayer force-pushed the totev/android-debugger-unixsockets branch from e743e9c to 182660a Compare January 6, 2016 09:37
teobugslayer added a commit that referenced this pull request Jan 6, 2016
…xsockets

Implement Android debugging with unix sockets redirection
@teobugslayer teobugslayer merged commit e46408f into master Jan 6, 2016
@teobugslayer teobugslayer deleted the totev/android-debugger-unixsockets branch January 6, 2016 10:31
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.

3 participants