[chirp_devel] Fwd: Fix downloading/uploading issue for UV-5R. Fixes #2317

[Mathieu Rozon]

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 at danplanet.com>:

> Hi Mathieu,
>
> It came through this time :)
>
> > # HG changeset patch
> > # User Mathieu Rozon <mathrozon at 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 at intrepid.danplanet.com
> http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
> Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
>



-- 
<---- Mathieu Rozon ---->
   mathrozon at gmail.com <mathrozon at gmail.som>
     (514)475-5010
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20150309/f8c82c68/attachment-0001.html