[Fuego] [PATCH] Add support for full device paths (with slashes and colons) to sercp

Frank Rowand frowand.list at gmail.com
Thu Apr 13 20:50:10 UTC 2017


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 <tim.bird at sony.com>
> ---
>  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=<baud>           Serial port baud rate [115200]
>     -B, --basic                     Use basic (minimal) remote system commands
> +   -d, --device=<device path>      Use serial device specified
>     -h, --help                      Show help and exit
>     -M, --md5sum                    verify file transfer with md5sum
>     -m, --minicom=<name>            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=<device path>'
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 <seconds> to a small value will result
>        in errors or unstable behaviour.  <seconds> 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



More information about the Fuego mailing list