Hi, Michael,
On Tue, Nov 11, 2014 at 9:25 PM, Michael Albinus <[email protected]>
wrote:
> Ashi <[email protected]> writes:
>
> > Hi, Michael,
>
> Hi Zhongwei,
>
> > I've updated the patch:
> > - add tramp-adb-connect-if-not-connected feature
> > - remind user if unnecessary port number is provided in USB mode
> > - discover port number for user if port number is not provided in TCP
> mode
>
> Thanks. We're making progress!
>
> As usual, I've played with this. And as usual, I do nitpick :-)
>
Not at all! I real appreciate your review and helping! I've learned a lot
in it.
> +(defcustom tramp-adb-connect-if-not-connected nil
> > + "Try to run adb connect if provided device is not connected
> currently. It is
> > +in tramp-adb-get-host-for-execution."
>
> The Emacs coding guidelines do require the first line of a docstring to
> be exactly a sentence (or two sentences, if they fit). Everything else
> must start in the next line. See (info "(elisp) Documentation")
>
> > + :group 'tramp
> > + :version "24.4"
>
> Well, Emacs 24.4 has been released. Your patch cannot be merged to that
> anymore ... use 24.5.
>
Done.
>
> > + :type '(choice (const nil) (const t)))
>
> I would rather use :type 'boolean
>
Done.
>
> > +(defun tramp-adb-search-host-in-devices (host devices)
> > + "Returns host:port string by searching host in the devices list. If
> > +failed, return nil."
>
> Again, docstring conventions.
>
Done
>
> > + (if (null devices) nil
>
> (when devices
>
> > + (if (tramp-adb-execute-adb-command
> > + '["adb" nil "" "/" nil] "connect" exe-name)
>
> You create a dummy vector, because tramp-adb-execute-adb-command adds an
> additional argument when there is a host name. This looks like a hack;
> better would be to fix this in tramp-adb-execute-adb-command. For
> example, you could check whether (car args) is "shell". Or something
> like this.
>
Done.
>
> > + ;; If port is not provided, adb will connect to default
> port number
> > + ;; 5555, which must be appended to return to get a full
> host name.
> > + (if port
> > + exe-name
> > + (format "%s:%s" host "5555"))))
>
> For the time being it might be OK to hardwire 5555. However, it is
> always a good idea to make it configurable, for example via
> tramp-methods. Look, how a default port is configured in tramp-sh.el.
>
Done.
>
> > - (setq devices (mapcar 'cadr (tramp-adb-parse-device-names nil)))
> > + (append devices (mapcar 'cadr (tramp-adb-parse-device-names nil)))
>
> Did you forget "(setq devices"?
>
No, I've set devices in local variable initialization: (devices (list
exe-host)))
>
> Well, then I made some tests. I have the following settings:
>
> # adb devices
> List of devices attached
> 015d24bc55282409 device
> android-c05807abea645fdf.fritz.box:5555 device
>
> In Emacs, I type "C-x f /adb: TAB". The completion buffer looks then
>
> Possible completions are:
> adb:015d24bc55282409: adb:192.168.0.141#5555:
> adb:192.168.0.141:
> adb:[android-c05807abea645fdf.fritz.box:5555]:
>
> Hmm, the last entry isn't like it should be.
>
Yes, it is a bug. I haven't fixed it yet in this update. I'll update the
patch again when I solve this.
> When I type "C-x f /adb:android-c05807abea645fdf: RET" everything is
> OK. Now I try "C-x f /adb:192.168.0.141: RET" and I get an error
> message. Still OK.
>
> Now I set tramp-adb-connect-if-not-connected to t, and try again "C-x f
> /adb:192.168.0.141: RET", and I don't get a Lisp error. Only in the
> connection buffer, I see
>
> Process *tramp/adb 192.168.0.141* exited abnormally with code 1
> error: device offline
>
"device offline" is a error in adb itself, I guess we can't handle it in
tramp. When got this error, I usually restart my device and try again...
not a good solution though :/
>
> And when I check it from the shell, there is
>
> # adb devices
> List of devices attached
> 015d24bc55282409 device
> android-c05807abea645fdf.fritz.box:5555 device
> 192.168.0.141:5555 offline
>
> Is it intended like this?
>
> > Best regards,
> > Zhongwei
>
> Best regards, Michael.
>
Best regards,
Zhongwei
diff --git a/lisp/tramp-adb.el b/lisp/tramp-adb.el
index 1a7a34f..89c10c4 100644
--- a/lisp/tramp-adb.el
+++ b/lisp/tramp-adb.el
@@ -45,6 +45,13 @@
:version "24.4"
:type 'string)
+(defcustom tramp-adb-connect-if-not-connected nil
+ "Try to run adb connect if provided device is not connected currently.
+It is used in `tramp-adb-get-host-for-execution'."
+ :group 'tramp
+ :version "24.5"
+ :type 'boolean)
+
;;;###tramp-autoload
(defconst tramp-adb-method "adb"
"*When this method name is used, forward all calls to Android Debug Bridge.")
@@ -71,7 +78,8 @@
;;;###tramp-autoload
(add-to-list 'tramp-methods
`(,tramp-adb-method
- (tramp-tmpdir "/data/local/tmp")))
+ (tramp-tmpdir "/data/local/tmp")
+ (tramp-default-adb-port "5555")))
;;;###tramp-autoload
(add-to-list 'tramp-default-host-alist `(,tramp-adb-method nil ""))
@@ -991,11 +999,69 @@ PRESERVE-UID-GID and PRESERVE-EXTENDED-ATTRIBUTES are completely ignored."
(tramp-set-connection-property v "process-buffer" nil))))))
;; Helper functions.
+(defun tramp-adb-search-host-in-devices (host devices)
+ "Return host:port string by searching HOST in DEVICES.
+If failed, return nil."
+ (if (null devices) nil
+ (if (string-match (concat "^" host) (car devices))
+ (car devices)
+ (tramp-adb-search-host-in-devices host (cdr devices)))))
+
+(defun tramp-adb-get-host-for-execution (vec)
+ "Return full host name from VEC to be used in shell exceution.
+E.g. a host name \"192.168.1.1#5555\" returns \"192.168.1.1:5555\"
+ a host name \"R38273882DE\" returns \"R38273882DE\"."
+ (let* ((port (tramp-file-name-port vec))
+ (host (tramp-file-name-real-host vec))
+ (method (tramp-file-name-method vec))
+ (exe-name (if port
+ (format "%s:%s" host port)
+ host))
+ (devices (mapcar 'cadr (tramp-adb-parse-device-names nil))))
+ ;; Checking whether exe-name is in devices. Follow cases are checked:
+ ;; - yes, correct exe-name returns.
+ ;; - not, true device name is: xxxx
+ ;; user types: xxxx:port-number. Error returns with tips of correct name
+ ;; - not, true device name is: xxxx:port-number
+ ;; user types: xxxx. Tramp will auto-complete port-number for user
+ ;; - not, no device name is similar as user gives. if
+ ;; tramp-adb-connect-if-not-connected is t, try to connect that device,
+ ;; else return error
+ (cond ((member exe-name devices) exe-name)
+ ((member host devices) (tramp-error
+ vec 'file-error
+ "Port number is not needed for device %s"
+ host))
+ ((tramp-adb-search-host-in-devices host devices))
+ (tramp-adb-connect-if-not-connected
+ ;; Try to connect exe-name device.
+ (if (tramp-adb-execute-adb-command
+ vec "connect" exe-name)
+ (tramp-error vec 'file-error "Could not connect %s" exe-name)
+ ;; If port is not provided, adb will connect to default port
+ ;; number, which must be appended to return to get a full host
+ ;; name.
+ (if port
+ exe-name
+ (format "%s:%s"
+ host
+ (tramp-get-method-parameter
+ method 'tramp-default-adb-port)))))
+ (t (tramp-error
+ vec
+ 'file-error
+ "Could not find device %s"
+ exe-name)))))
(defun tramp-adb-execute-adb-command (vec &rest args)
"Returns nil on success error-output on failure."
- (when (> (length (tramp-file-name-host vec)) 0)
- (setq args (append (list "-s" (tramp-file-name-host vec)) args)))
+ (let ((host (tramp-file-name-host vec)))
+ (when (and (> (length host) 0)
+ ;; The -s switch is only available for ADB device commands.
+ (not (member
+ (car args)
+ (list "connect" "disconnect" "devices"))))
+ (setq args (append (list "-s" (tramp-adb-get-host-for-execution vec)) args))))
(with-temp-buffer
(prog1
(unless
@@ -1097,9 +1163,12 @@ connection if a previous connection has died for some reason."
(let* ((buf (tramp-get-connection-buffer vec))
(p (get-buffer-process buf))
(host (tramp-file-name-host vec))
+ (exe-host (tramp-adb-get-host-for-execution vec))
(user (tramp-file-name-user vec))
- devices)
-
+ ;; Adding the exe-host to devices ensures a new connected exe-host
+ ;; is in devices. Because tramp-adb-parse-device-names can't
+ ;; discover new connected device.
+ (devices (list exe-host)))
;; Maybe we know already that "su" is not supported. We cannot
;; use a connection property, because we have not checked yet
;; whether it is still the same device.
@@ -1110,10 +1179,10 @@ connection if a previous connection has died for some reason."
(and p (processp p) (memq (process-status p) '(run open)))
(save-match-data
(when (and p (processp p)) (delete-process p))
- (setq devices (mapcar 'cadr (tramp-adb-parse-device-names nil)))
+ (append devices (mapcar 'cadr (tramp-adb-parse-device-names nil)))
(if (not devices)
(tramp-error vec 'file-error "No device connected"))
- (if (and (> (length host) 0) (not (member host devices)))
+ (if (and (> (length host) 0) (not (member exe-host devices)))
(tramp-error vec 'file-error "Device %s not connected" host))
(if (and (> (length devices) 1) (zerop (length host)))
(tramp-error
@@ -1123,7 +1192,7 @@ connection if a previous connection has died for some reason."
(let* ((coding-system-for-read 'utf-8-dos) ;is this correct?
(process-connection-type tramp-process-connection-type)
(args (if (> (length host) 0)
- (list "-s" host "shell")
+ (list "-s" exe-host "shell")
(list "shell")))
(p (let ((default-directory
(tramp-compat-temporary-file-directory)))
_______________________________________________
Tramp-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/tramp-devel
