From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-transfer-encoding; bh=jVzdMAPw7P+jI+j1EtZRqXR661aLU+34Z2eu21tQ0iU=; b=mpU+2aby2IBJLzRwelOCkRkYnLa6mwKKvV9XNmEXUXYkcol9cMd5NdYCjkWmkCbRmo AcgFGEP/PZKgQY0ugDWGfJxbuN3glFdRrHGMzDv+b4uHU8I+/TM2ftRCqqsxuxS9ph6S Q6zxokiB/C9aoXHLKAi1RGh5i8s2Na+uKvYSyrTTY4xrZlEmhoPZwUY6U3bqsgyLQop4 1/wANDPD7/cAiSIIytdMKjcCsAj0uBMGRQAxiM91e90LfyxeXHXmBaIos8sLKOkwy4Nj es+qv7C3P9cu9V3XLmkoLzOqixY+g9+3jXaz4SFQHsDhF4xbP39R2YvP9rQJdyXBYUqn Xi7g== References: From: Frank Rowand Message-ID: <58EFE482.7020508@gmail.com> Date: Thu, 13 Apr 2017 13:50:10 -0700 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Fuego] [PATCH] Add support for full device paths (with slashes and colons) to sercp List-Id: Mailing list for the Fuego test framework List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Bird, Timothy" , "monstr@monstr.eu" , "fuego@lists.linuxfoundation.org" On 04/13/17 11:35, Bird, Timothy wrote: > Add a new argument (-d, --device) to support a full device path. > Due to file argument parsing constraints, sersh cannot handle > certain serial paths used as the host for the source or destination. > > This feature makes it possible to use a full path with slashes > and colons to specify the serial port for the operation. Use something > like: > $ sercp -d /dev/serial/by-path/pci-0000:00:00.0-usb-0:3:1.4 file1 serial:/tmp > > The reason for this is that in some cases, using the full path is more > convenient than the single-word device. For example, the full path can > uniquely identify a serial port on a USB port, that might change device > names as USB ports are connected and disconnected. Yes to the concept. Some implementation details need adjustment. All of my code below is untested. > Signed-off-by: Tim Bird > --- > serio | 31 +++++++++++++++++++++++++------ > 1 file changed, 25 insertions(+), 6 deletions(-) > > diff --git a/serio b/serio > index b1ed793..f1e2730 100755 > --- a/serio > +++ b/serio > @@ -537,6 +537,7 @@ def usage_sercp(): > print ''' > -b, --baudrate= Serial port baud rate [115200] > -B, --basic Use basic (minimal) remote system commands > + -d, --device= Use serial device specified > -h, --help Show help and exit > -M, --md5sum verify file transfer with md5sum > -m, --minicom= Name of the minicom config file to use > @@ -555,6 +556,10 @@ Notes: > a "/" before the ":". This means that "host1" and "host2" may not > contain a "/". Due to this constraint "host1" and "host2" are device > names relative to "/dev/". > + - if a device path is specified, then it should be a fully-qualified path > + (not relative to '/dev/'), and you should use "serial" in the source > + or destination path as the host. + In this case the hostname is just a placeholder. If multiple source + files are specified the hostname must be the same for each of the + source files. > +ex: sercp -d /dev/serial/by-path/pci-0000:00:00.0-usb-0:3:1.4 file1 serial:/tmp > - All source files ("file1 ...") must be on the same host. > - The source files ("file1 ...") and the destination location ("file2") > must be on different hosts. Local copies and copies between remote > @@ -625,7 +630,8 @@ def usage_sersh(): > > Notes: > - user is ignored > - - host is a serial device name, relative to "/dev/". > + - host is a serial device name, relative to "/dev/", or a full path > + to the serial device, starting with "/". The format of host should not change for sersh. I want host to be consistent for sercp and sersh, so the parsing of host for sersh should be changed to not allow an embedded slash. This means that the '-d, --device=' also needs to be added to sersh option parsing and the same additions as were made to usage_sercp() should be added to usage_sersh(). > - --timeout is a big stick. Setting to a small value will result > in errors or unstable behaviour. must be long enough to allow > the entire command to be echoed. > @@ -652,7 +658,7 @@ Return: > > def main(): > > - def parse_arg_for_files(args): > + def parse_arg_for_files(args, device_path): > host = None > files = [] > > @@ -689,7 +695,14 @@ def main(): > host_start = 0 > new_host = arg[host_start:host_end] > if not '/' in new_host: > - new_host = '/dev/' + new_host That existing code has a test that is not needed. The paragraph above these two lines guarantees that there is not a '/' in new_host, so the 'if not '/' in new_host:' is redundant and should be removed. Line 692 should always prefix new_host with '/dev/'. The following 8 lines should not be added. Just use the existing code to find the host name and ensure that it is the same for each source file. The test should be on device_path, not on the name of new_host (see my next chunk of code, just before the return). > + if new_host=="serial": > + if device_path: > + new_host = device_path > + else: > + print "ERROR: must specific device path (with -d) with host 'serial'" > + sys.exit(EX_USAGE) > + else: > + new_host = '/dev/' + new_host > if host is None: > host = new_host > elif host != new_host: Then just before the return statement, adjust the value of host: + if host is not None and device_path is not None: + host = device_path return host, files > @@ -717,6 +730,7 @@ def main(): > create_link_path = None > create_links = None > destination = None > + device_path = None > timeout = None > minicom = None > port = None > @@ -763,10 +777,11 @@ def main(): > > try: > opts, args = getopt.getopt(sys.argv[1:], > - 'b:BhMm:rT:v', > + 'b:Bd:hMm:rT:v', > [ > 'baudrate=', > 'basic', > + 'device=', > 'help', > 'md5sum', > 'minicom=', > @@ -786,6 +801,8 @@ def main(): > baudrate = arg > elif opt in ('-B', '--basic'): > basic = 1 > + elif opt in ('-d', '--device'): > + device_path = arg > elif opt in ('-h', '--help'): > usage_sercp() > sys.exit(EX_OK) > @@ -885,8 +902,10 @@ def main(): > print 'ERROR: file1 and file2 required' > sys.exit(EX_USAGE) > No need to line wrap the following, there are many longer lines already. > - src_host, source = parse_arg_for_files(args[0:len(args) - 1]) > - dst_host, dst = parse_arg_for_files(args[len(args) - 1:]) > + src_host, source = parse_arg_for_files(args[0:len(args) - 1], > + device_path) > + dst_host, dst = parse_arg_for_files(args[len(args) - 1:], > + device_path) > > if src_host: > if dst_host: > Then the following change will be needed for sersh at line 940 so that sersh will follow the same host name rules: if '/' in host: - port = host + print 'ERROR: invalid host name' + sys.exit(EX_USAGE) + elif host is not None and device_path is not None: + port = device_path else: port = '/dev/' + host