Hi Dan,

 

> > +debug = True
>
> You don't want to leave this on for all chirp users.

 

you are right, this was a leftover from debugging, I will deactivate it again.

 

> These being module-level means they'll live for the lifetime of the
> running process.

 

That´s intentional: Once the driver detects that there are troubles with the timing, its best to memorize that (instead of again failing for each up- and download attempt).
This is my first time hacking in python - so if there is a better solution for achieving that, let me know.

 

> 10ms inter-byte latency is crazy high. I also don't like making this
> different on different platforms, even if we're doing it in response to
> a short read.

 

I agree that this is not the best solution, but it´s worth to mention:
- as Pavel Milanes found out, the inter-byte delay is exactly what the OEM-Software is doing ( http://chirp.danplanet.com/issues/3993#note-24 ), so obviously even the manufacturer knows that their radios are not able to handle 9600 baud (and has no better solution to deal with that error)
- we are not doing it different on different platforms (this was the first approach), but changed to: detect "short read" or "invalid-header"-errors, and slow down only then. This avoids the artificial delay with radios that are fast enough to handle pyserial on linux, and on the other hand fix the error also on windows-versions. (Where it may happen as well, but only very rarely).
- The 10ms are indeed quite long. In a earlier version of this patch only 5ms were used, and this was tested by Jim Unroe and other users on 5 different radios ( http://chirp.danplanet.com/issues/3993#note-22 ) and reported as OK. However, Pavel argued to use 10ms, to be safe. (The comment in the sources is his ;-) )
I just retested on my setup (Ubuntu 16.4 and KT-8900R), that 2ms is sufficient in most cases (1ms is BTW too short). So from my point of view we could start with 3ms, and finetune that in case that any new errors are reported.

 

>
> I would rather we try things like writing eight bytes followed by a
> delay or something like that.

 

Adding the delay only every X bytes was what I tried first, however it did not work.

 

> This will spam everyone's logs with one line for each byte of the memory
> map. That's not reasonable, IMHO.

 

It is not that evil as you might think:
It happens only if debug == True, and even in that case it is not per byte, but per TX_BLOCK_SIZE.
However, I share  your concerns. I would propose adding the "delayed"-Information in the same line as the hex-trace (which was already there ...):
        # DEBUG
        if debug is True:
            if NEEDS_DELAY:
                LOG.debug("==> (%d) bytes (DELAYED):\n\n%s" %
                      (len(data), util.hexprint(data)))
            else:
                LOG.debug("==> (%d) bytes:\n\n%s" %
                      (len(data), util.hexprint(data)))

 

> Stray whitespace damage here.

My I ask what exactly do you mean? (did I already mention that I am completely new to python and this project? ;-) )

 

> These hunks appear unrelated to the rest of the change?

 

Oops ... This must be a fix by Pavel for some other issue (I dont even know which one), which I accidentally took over, when including his proposals changes ... I will remove it from this patch.

What is the correct process to add the fixed patch? Should I mail it as a reply to this thread? Or should I create a new patch using mercurial? (which will lead to a new thread in the mailing-list?)

 

73,
Michael