I have tested it with the "newer" versions that had the upload/download issues as well as an ​"older" version that was previously working with chirp.

What is the review process for patches? Should I submit a new patch with the corrections that you request?

Thank you.

2015-03-09 21:16 GMT-04:00 Dan Smith <dsmith@danplanet.com>:
Hi Mathieu,

It came through this time :)

> # HG changeset patch
> # User Mathieu Rozon <mathrozon@gmail.com>
> # Date 1425778153 18000
> #      Sat Mar 07 20:29:13 2015 -0500
> # Node ID cfdd6f6577f1c8d420376c35bb684a9552042179
> # Parent  d84dfd6c29d61268de11584f5d427e60936131d9
> Fix downloading/uploading issue for UV-5R. Fixes #2317
>
> diff -r d84dfd6c29d6 -r cfdd6f6577f1 chirp/drivers/uv5r.py
> --- a/chirp/drivers/uv5r.py   Tue Mar 03 22:25:38 2015 -0800
> +++ b/chirp/drivers/uv5r.py   Sat Mar 07 20:29:13 2015 -0500
> @@ -438,17 +438,22 @@
>      ack = serial.read(1)
>      if ack != "\x06":
>          raise errors.RadioError("Radio refused clone")
> -
> +

You're adding whitespace here, please remove this.

>      return ident
>
>
> -def _read_block(radio, start, size):
> +def _read_block(radio, start, size, firstCommand):

Since this is not needed every time, you should default this to False
for easier calling. Like this:

  def _read_block(radio, start, size, first_command=False):
      ...

>      msg = struct.pack(">BHB", ord("S"), start, size)
>      radio.pipe.write(msg)
>
> +    if(firstCommand == False):

Please don't use camelCase identifiers and observe proper spacing; the
parentheses are unnecessary. Also, the pythonic way to do this would be:

  if not first_command:
      ...

> +        ack = radio.pipe.read(1)
> +        if ack != "\x06":
> +            raise errors.RadioError("Radio refused to send second block 0x%04x" % start)
> +
>      answer = radio.pipe.read(4)
>      if len(answer) != 4:
> -        raise errors.RadioError("Radio refused to send block 0x%04x" % start)
> +        raise errors.RadioError("Radio refused to send first block 0x%04x" % start)

I don't think you should make this change, as you could hit this error
on a non-first block.

>
>      cmd, addr, length = struct.unpack(">BHB", answer)
>      if cmd != ord("X") or addr != start or length != size:
> @@ -464,28 +469,25 @@
>          raise errors.RadioError("Radio sent incomplete block 0x%04x" % start)
>
>      radio.pipe.write("\x06")
> -
> -    ack = radio.pipe.read(1)
> -    if ack != "\x06":
> -        raise errors.RadioError("Radio refused to send block 0x%04x" % start)
> +    time.sleep(0.005)
>
>      return chunk
>
>
>  def _get_radio_firmware_version(radio):
>      if radio.MODEL == "BJ-UV55":
> -        block = _read_block(radio, 0x1FF0, 0x40)
> +        block = _read_block(radio, 0x1FF0, 0x40, True)
>          version = block[0:6]
>      else:
> -        block1 = _read_block(radio, 0x1EC0, 0x40)
> -        block2 = _read_block(radio, 0x1F00, 0x40)
> +        block1 = _read_block(radio, 0x1EC0, 0x40, True)
> +        block2 = _read_block(radio, 0x1F00, 0x40, False)

If you make the change I suggested above about the defaulted parameter,
then you won't need to pass False here.

I'm worried that this may not work for other UV5R variants, other than
the ones mentioned in the bug.

Jim, can you look the changes here over and see if you think it'll be a
problem for the other versions of this radio?

Thanks!

--Dan


_______________________________________________
chirp_devel mailing list
chirp_devel@intrepid.danplanet.com
http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers



--
<---- Mathieu Rozon ---->
   mathrozon@gmail.com
     (514)475-5010