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 :-) > +(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. > + :type '(choice (const nil) (const t))) I would rather use :type 'boolean > +(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. > + (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. > + ;; 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. > - (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"? 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. 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 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. _______________________________________________ Tramp-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/tramp-devel
