[chirp_devel] Fwd: Fix downloading/uploading issue for UV-5R. Fixes #2317
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]
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
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 block0x%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 block0x%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]
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
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.
Ah, good, thanks for that. I'd still like Jim to weigh in as he pretty much "owns" that driver.
What is the review process for patches? Should I submit a new patch with the corrections that you request?
Yep, you can just revise your patch and re-send it. Some details that might help are here:
http://chirp.danplanet.com/projects/chirp/wiki/DevelopersProcess
If you have already committed your change (i.e. not using mq), you can install mq and do "hg qimport -r tip" to place it under mq's control going forward.
Thanks Mathieu!
--Dan
On Mon, Mar 9, 2015 at 9:46 PM, Mathieu Rozon mathrozon@gmail.com wrote:
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.
Hi Mathieu,
Thanks for working on this. I've been scratching my head about it for a few days now. I, just a few minutes ago, finally got my loaner radio to read. I was heading in a similar direction as you were but you are clearly further along and have nicer code once you make Dan's changes.
I will test you updated patch with my radios tomorrow.
Thanks again, Jim KC9HI
On Mon, Mar 9, 2015 at 11:19 PM, Jim Unroe rock.unroe@gmail.com wrote:
On Mon, Mar 9, 2015 at 9:46 PM, Mathieu Rozon mathrozon@gmail.com wrote:
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.
Hi Mathieu,
I spent some time testing this today. I was able to read my 3 year old UV-5R (BFB231) radio, my UV-82 radio, my BF-F8HP radio and my loaner UV-5R that has the problem that the patch is for. That is the good news.
The bad news is that neither my UV-5R (BFB231) or BF-F8HP will upload. The error returned is "Radio refused to accept block 0x0020". The UV-82 and the loaner UV-5R seemed to upload OK.
Jim
participants (3)
-
Dan Smith -
Jim Unroe -
Mathieu Rozon