[chirp_devel] [PATCH] [FT-70] New Model #5329 Increased second block size

Wed Mar 21 16:25:34 PDT 2018

**** Thanks for the feedback!

On 21 March 2018 at 23:06, Dan Smith via chirp_devel <
chirp_devel at intrepid.danplanet.com> wrote:

> > The FT70 has a built in USB interface The Chirp driver is only intended
> to be used with this, no baud rate is set on it. Using a higher baud rate
> made a slight difference in speed, But initially thought this might now be
> causing other issues, so changed back to 38400.
>
> That is very surprising to me. Every other USB serial device I know of
> honors the baud rate as expected. Unless the radio is really mirroring
> whatever it can tell the USB serial bridge is set to, but that would be the
> first such device I've ever encountered.
>

****  The USB interface is built into the radio, I would need to research
further but the baud rate does not effect the radio side of the connection.
It enumerates as /dev/cu.usbmodem1D141 basically a USB interface provided
by the radio microprocessor. I don't know if any/many other Chirp supported
radios have a built in USB interface.

**** Thanks for the feedback! Agreed we need to look at revising the read
in yaesu_clone I think this issue effects the FT1D as well

> > At 38400 or 15200 baud blocks were being returned with no data in them.
>
> So the problem is with _reading_ from the radio, not writing to it? In
> almost all cases of flaky "it works on my system but not his" cases, it was
> writing to the radio that was the problem. This is usually because the
> radio has subtle timing requirements (or a small buffer with no flow
> control). So, odd thing #2.
>

**** It seems to write fine, although one to triple check.

>
> > Investigated further with Fred and Mark
> > We are losing blocks at the beginning in the Chirp yaesu_clone method.
> >
> > [2018-03-19 21:35:41,483] chirp.drivers.yaesu_clone - DEBUG: Read 0/65920
> > [2018-03-19 21:35:41,733] chirp.drivers.yaesu_clone - DEBUG: Read 0/65920
> > [2018-03-19 21:35:41,986] chirp.drivers.yaesu_clone - DEBUG: Read 0/65920
> > [2018-03-19 21:35:42,236] chirp.drivers.yaesu_clone - DEBUG: Read 0/65920
> >
> > I thought the rest of the download was working, hence the puzzle of how
> are we short at the end. Which did not make sense if we were only losing
> blocks at the beginning...
>
> These aren't blocks, they're chunks, and the yaesu_clone code assumes that
> each chunk of the whole block can and will be read. Sounds like that code
> should at _least_ assert that it read the length expected, because if it
> doesn't, it will silently return less data than the block requested. In
> reality, it should probably be smarter and plan to read the total number of
> bytes regardless of how many trips it takes, and have a overall timeout to
> catch the case where we don't hear anything from the radio.
>
> > After increasing the size last night it worked and then we realised that
> blocks must be being returned with no data during the rest of the download.
> Slight confusion of the debug info as that shows the running total - not
> the bytes read for each block - errors will show as an unchanged value not
> as zero... Loaded it into a spreadsheet this morning and behold we are
> losing a dozen or so blocks...
> >
> > Two examples
> >
> > [2018-03-19 21:18:16,013] chirp.drivers.yaesu_clone - DEBUG: Read
> 49152/65920
> > [2018-03-19 21:18:16,263] chirp.drivers.yaesu_clone - DEBUG: Read
> 49152/65920
> >
> > [2018-03-19 21:18:16,489] chirp.drivers.yaesu_clone - DEBUG: Read
> 53248/65920
> > [2018-03-19 21:18:16,739] chirp.drivers.yaesu_clone - DEBUG: Read
> 53248/65920
> >
> > etc. Note the read figure has not changed, so as you said that block
> read did not return any data.
>
> Yeah, so what I'm saying about changing the behavior of _chunk_read()
> above would apply I think.
>
> > I submitted a patch to increase the size - for the time being - but it
> seems that yaesu_clone needs looking at.
>
> Okay, so I had this backwards because I thought you were talking about
> uploading not downloading. So I understand why that being larger makes a
> difference, but it'd be wrong for anyone that doesn't stall at the
> beginning. I guess the same stupidness in _chunk_read() makes that not
> completely hang for those people, but...not the right solution :)
>
> > chirp sets the time out to .25 secs I changed it to 2 secs and seems to
> work.
>
> Yes, and some drivers have to change this because of their radios
> (although it's always on write AFAIK). You could do that, but I think
> making the change I described above to _chunk_read() would be better as it
> would allow you to keep a low timeout (which is desirable) and also not
> return a short block if you happen to hit a stall.
>
> Something like this completely untested change:
>
> diff -r 61ba9c815170 chirp/drivers/yaesu_clone.py
> --- a/chirp/drivers/yaesu_clone.py      Thu Mar 15 23:41:07 2018 +0000
> +++ b/chirp/drivers/yaesu_clone.py      Wed Mar 21 16:05:01 2018 -0700
> @@ -43,14 +43,24 @@
>
>
>  def _chunk_read(pipe, count, status_fn):
> +    timer = time.time()
>      block = 32
>      data = ""
> -    for _i in range(0, count, block):
> -        data += pipe.read(block)
> -        if data:
> +    while len(data) < count:
> +        # Don't read past the end of our block if we're not on a 32-byte
> +        # boundary
> +        chunk_size = min(block, count - len(data))
> +        chunk = pipe.read(chunk_size)
> +        if chunk:
> +            timer = time.time()
> +            data += chunk
>              if data[0] == chr(CMD_ACK):
>                  data = data[1:]  # Chew an echo'd ack if using a 2-pin
> cable
>                  # LOG.debug("Chewed an ack")
> +        if time.time() - timer > 2:
> +            # It's been two seconds since we last saw data from the radio,
> +            # so it's time to give up.
> +            raise errors.RadioError("Timed out reading from radio")
>          status = chirp_common.Status()
>          status.msg = "Cloning from radio"
>          status.max = count
>
> --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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20180321/22b88d0c/attachment-0001.html 