<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Dan et al <br>
<br>
See below<br>
<br>
<div class="moz-cite-prefix">El 22/03/16 a las 21:48, Dan Smith via
chirp_devel escribió:<br>
</div>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">Hi Jim and Pavel,
Thanks for your work on this, I know it's in high demand and has been a
while coming. I have some concerns, detailed below:.
</pre>
<blockquote type="cite">
<pre wrap="">+def <b>_recv</b>(radio, amount):
+ # Get the header + basic sanitize
+ hdr = _rawrecv(radio, 4)
+ if len(hdr) != 4:
+ msg = "Short header for block: 0x%04x" % addr
+ LOG.error(msg)
+ raise errors.RadioError(msg)
+
+ # receive and validate the header
+ c, a, l = struct.unpack(">BHB", hdr)
+ if a != addr or l != BLOCK_SIZE or c != ord("X"):
+ msg = "Invalid answer for block 0x%04x:" % addr
+ LOG.error(msg)
+ LOG.debug("CMD: %s ADDR: %04x SIZE: %02x" % (c, a, l))
+ raise errors.RadioError(msg)
+
+ # Get the data
+ data = _rawrecv(radio, l)
</pre>
</blockquote>
<pre wrap="">
This kind of fine-grained reading is hard on performance -- it would be
better if you can read larger chunks of data, buffer it and split it up
as you find frames.</pre>
</blockquote>
<br>
Yes, good to know about the performance issue. Also how can I
measure that performance impact to compare for future improvements?<br>
<br>
I think you are mentioning the _recv() procedure instead the
_rawrecv()<br>
<br>
We originally wrote the driver that way but we found that the driver
was pretty unstable and not because of the driver it self but the
radios having and not consistent delays on the answer to PC
commands.<br>
<br>
Most of the time we got reads with not the amount of data we must
have and the driver failing, the other alternative was to use big
time.sleep() here and there and we have then a long delayed reads
and uploads with occasional resets of the radio. <br>
<br>
And that non consistent delays (pauses between write data from radio
to PC, mostly) are worst once we got different variants of the
*same* radios to test (pre-production, alphas, 1G, 2G) not to
mention other models.<br>
<br>
The only way we found to manage that was the _rawrecv() procedure
that you see now with precise time.sleep() in some places calculated
from a timing profile for every radios to found the sweet spot for
all of them.<br>
<br>
We developed yesterday a closed test with some users and that allow
us to tweak more precisely the constant to form the maxtime var in
the _rawrec procedure.<br>
<br>
Thinking about it now, maybe we can write now the _recv() procedure
with the "read big chunks" strategy, the key is the _rawrecv()
procedure that will allow us to do that, but we have a time to test
that on the field.
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<blockquote type="cite">
<pre wrap="">+def _do_magic(radio, status):
+ """Try to put the radio in program mode and get the ident string
+ it will make multiple tries"""
</pre>
</blockquote>
<pre wrap="">
I think maybe renaming this at some point would be good.</pre>
</blockquote>
<br>
From Aladdin story, this procedure manages the magic phrase to
"open" the radios to programming mode.<br>
<br>
Any suggestion?<br>
<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<blockquote type="cite">
<pre wrap="">+
+ # how many tries
+ tries = 5
+
+ # prep the data to show in the UI
+ status.cur = 0
+ status.msg = "Identifying the radio..."
+ status.max = len(radio._magic) * tries
+ radio.status_fn(status)
+ mc = 0
+
+ try:
+ # do the magic
+ for magic in radio._magic:
+ # we try a few times
+ for a in range(0, tries):
</pre>
</blockquote>
<pre wrap="">
This is super deeply nested. I think this should be broken up into
easier-to-understand pieces.</pre>
</blockquote>
<br>
Roger, I will do.<br>
<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ # Update the UI
+ status.cur = (mc * tries) + a
+ radio.status_fn(status)
+
+ # cleaning the serial buffer, try wrapped
+ try:
+ radio.pipe.flushInput()
+ except:
+ msg = "Error with a serial rx buffer flush at _do_magic"
+ LOG.error(msg)
+ raise errors.RadioError(msg)
+
+ # send the magic a byte at a time
+ for byte in magic:
+ ack = _rawrecv(radio, 1)
+ _send(radio, byte)
+
+ # A explicit time delay, with a longer one for the UV-5001
+ if "5001" in radio.MODEL:
+ time.sleep(0.5)
+ else:
+ time.sleep(0.1)
</pre>
</blockquote>
<pre wrap="">
I don't think this makes sense. Sleeping before a read (especially of
one byte) is kinda weird. Can't you just set your serial timeout to
cover both of these cases and then read the byte, handling the timeout
if it happens?</pre>
</blockquote>
<br>
Yes, you are right, it can be.<br>
<br>
But by the way the _rawrec() works we must have the serial timeout
to a minimum value, that's why have used a time.sleep() trick
instead.<br>
<br>
If we use direct serial reads from radio.pipe it can work with the
serial timeout, but will require more testing and time. <br>
<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ # wait between tries for different MAGICs to allow the radio to
+ # timeout, this is an experimental fature for the 5001 alpha that
+ # has the same ident as the MINI8900, raise it if it don't work
+ time.sleep(5)
</pre>
</blockquote>
<pre wrap="">
Yikes, five seconds? I guess it seems like we should be able to avoid
this with different model subclasses.</pre>
</blockquote>
<br>
Believe it or not it don't work below 3 secs for ones and 5 secs for
others, but at this point you don't know about who is who... so 5
secs. (this has a trick, keep reading)<br>
<br>
Even so, there is a UV-5001 G2v2 (marked as G22 in the code, and
yes, second revision of the second generation) that uses the magic
from the Waccom Mini 8900 instead the former one for the other
members of the UV-5001 designation. So we have to test both magic
words to open it, and the users is unaware of t<br>
he version of the radio.<br>
<br>
It's like the radios has a serial buffer that timeout after 3/5
secs, when you send a wrong magic you have to wait for the serial
buffer on the radio to timeout before sending the correct one,
otherwise the radio will ignore the correct magic....<br>
<br>
As a workaround we have arranged the order of the tries from the
most used units first to the least used units last (read the
comments at the end before the declaration of the specific models)
to get a higher chance of get it on the first tries; just for the
UV-5001 brand there is 5 radio variants in the market now.<br>
<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+def _do_ident(radio, status):
+ """Put the radio in PROGRAM mode & identify it"""
+ # set the serial discipline
+ radio.pipe.setBaudrate(9600)
+ radio.pipe.setParity("N")
+ radio.pipe.setTimeout(0.005)
</pre>
</blockquote>
<pre wrap="">
I feel like a timeout this low at 9600 baud is probably not very useful.
The inter-byte timing of 9600N81 is about 1ms and your timeout here is
5ms. Other drivers that communicate at this (and faster) speeds are
using timeouts on the order of 100-250ms.
Is there some detail you can offer here?</pre>
</blockquote>
<br>
Yes, I know that, let me explain:<br>
<br>
<b>Note:</b> Jim, please help me here if I make some mistake or
syntactic error that spoil the explanation please (sorry I'm native
Spanish and English is kind of self learned) <br>
<br>
The radios has an uneven (not constant time) delays in the response
of our queries starting to count from the end of the sent data, and
even on the segments of the replies (ack, pause, header, pause,
data...) even on the data stream the radio make pauses!<br>
<br>
This has give us some big head aches, if you use serial timeouts
then you get from time to time a shorter data block and the driver
fails, that's no reliable, for one radio it <u><i>could</i></u>
work, but for 10 variants of the same radio structure the thing get
nasty.<br>
<br>
No matter how you tweak the serial timeouts or use time.sleep(), to
much and the radio resets, to low and the radio give a short block,
when you test different generations of the same radios and different
models you get an very unstable driver, as all they have different
time requirements.<br>
<br>
At least the OEM has managed to get one software tuned for every few
specific models, but we want one driver for all of them.<br>
<br>
So to manage that we have to go back to the design table and I
designed the actual _rawrecv() procedure that reads the data a byte
at a time timing out on the desired amount of data on the buffer or
a master timeout related to how many bytes we want to read, and to
work that way we have to set the serial timeout as low as possible.
Implementing a serial timeout by software.<br>
<br>
This way and with the few pre-calculated time.sleep() distributed on
the _do_magic, _download & _upload (from statistics from real
data for all the radio models/variants), we have a very stable
driver for all models and with a single tune point for any needed
time tweak; the constant in this line on the _rawrecv() procedure:<br>
<br>
maxtime = amount * 0.020<br>
<br>
In fact that value in our tests and the final "release candidate"
was 0.008 sec tops for every character we have to read from the
radio.<br>
<br>
After a closed test we find that some FTDI chips on windows needs
0.015 and on Macs needs up to 0.020 to get the diver working
reliably.<br>
<br>
A resume after reading all this looking for typos: the _rawrecv()
procedure implements it's own kind of "software serial timeout" that
allow us to be correct and fast and works with all radio models from
Btech and the Waccom Mini 8900; but for that to works the real
serial timeout must be low enough to pass a single char.<br>
<br>
And this trick will allow us to fix things like this one just
reported here <a class="moz-txt-link-freetext" href="http://chirp.danplanet.com/issues/3507">http://chirp.danplanet.com/issues/3507</a> by just rising
the value of the constant on the mentioned line without loosing
performance (I'm pretty sure with 99% confidence)<br>
<br>
I hope I have explained it clearly.<br>
<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ # cleaning the serial buffer, try wrapped
+ try:
+ radio.pipe.flushInput()
+ except:
+ msg = "Error with a serial rx buffer flush at _do_ident"
+ LOG.error(msg)
</pre>
</blockquote>
<pre wrap="">
This should be a LOG.exception() right? Presumably this means something
very strange happened. Also, why do you think this is necessary?</pre>
</blockquote>
<br>
The radio.pipe.flushInput() and the flushOutput() makes the test bed
of Chirp fail unless it's try wrapped, note the end of the comment
on the start of the block of code. "try wrapped".<br>
<br>
The radio can send garbage if you wait to long listening on the
serial line, it's not the interface but the radio, I have serial
logs to prove that it does it with the OEM software, from where I
copied the trick of cleaning the serial buffer.<br>
<br>
If you don't make a periodic flushInput() before each block of reads
you get from time to time garbage in the next read block, and that
makes the driver fail.<br>
<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ # basic check for the ident
+ if len(ident) != 49:
+ msg = "Radio send a sort ident block, you need to increase maxtime."
</pre>
</blockquote>
<pre wrap="">
This goes to the UI, right? What is maxtime? Does it refer to the value
in _rawrecv()?</pre>
</blockquote>
<br>
Yes it's a debug message for the developer, not the user.<br>
<br>
Remember, I code here in Cuba, send the code by email to Jim in USA,
Jim test with the radios and fix what he can (help him from explicit
debug messages like this), he then pass logs, code & comments to
me bye email, I code... (loop)<br>
<br>
This was very useful then, must be changed now, will be changed.<br>
<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+class btech(chirp_common.CloneModeRadio, chirp_common.ExperimentalRadio):
</pre>
</blockquote>
<pre wrap="">
This should be:
class BTech(...
or something. Starting a class name with a lowercase letter is not
really in line with anyone's style.</pre>
</blockquote>
<br>
Good & sorry, I come from web development (ASP & PHP with
auto learned skills) and now into Python, I must learn some tricks
yet, thanks for guiding me with details like this. I will correct
it.<br>
<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ @classmethod
+ def get_prompts(cls):
+ rp = chirp_common.RadioPrompts()
+ rp.experimental = \
+ ('This driver is experimental and for personal use only.\n'
</pre>
</blockquote>
<pre wrap="">
You can't make this requirement of the user based on the license. Please
remove it.</pre>
</blockquote>
Right, will do it.<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ '\n'
+ 'Please keep a copy of your memories with the original software '
+ 'if you treasure them, this is the first release and may contain'
+ ' bugs.\n'
+ '\n'
+ 'You will miss the setting tab, we are working on it. Your '
+ 'success/failure story is appreciated, visit the Chirp\'s '
+ 'website and drop us a comment or just say THANKS if it works '
+ 'for you.\n'
</pre>
</blockquote>
<pre wrap="">
This is probably a little too verbose for the experimental warning.
</pre>
</blockquote>
Get it.<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+# Note:
+# the order in the lists in the _magic, IDENT and _fileid is important
+# we put the most common units first, the policy is as follows:
+
+# - First latest (newer) units, as they will be the most common
+# - Second the former latest version, and recursively...
+# - At the end the pre-production units (pp) as this will be unique
</pre>
</blockquote>
<pre wrap="">
This concerns me a bit. Is there any chance that the value you're using
for identification is actually just a mutable value and we'll be chasing
valid values forever?</pre>
</blockquote>
<br>
No, this value is NOT mutable.<br>
<br>
This radios has a REAL mem span of 0x4000 (you can safely read up to
there, and write too) but in the read the OEM software get just up
to 0x3200 (we get full span in chirp) and in the write just up to
0x3100.<br>
<br>
We respect the write boundary and this driver will never write
beyond 0x3100, see _upload() procedure comments<br>
<br>
Precisely in the area beyond 0x3200 there is some useful data like
the image fingerprint we use to identify each radio model, the
ranges for the bands, and other yet unknown data. (all this data
with some strange order conform the ident string the radio send at
the beginning)<br>
<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">I went ahead and applied this because I know that it's high-demand and
that the Jetstream work will likely depend on this for collaboration.
I've also put the test images into the tree.
I don't think this should go out to users with the experimental warning
the way it is, so I will fix that up myself.</pre>
</blockquote>
Roger, no problem on that and thanks for pushing it to the release
with so many "comments"<br>
<br>
If the JetStream is a clone of this (or vice versa) get support for
it is as fast as Jim can get it on it's hands to test it and we will
work on that direction ASAP.<br>
<br>
Sending the radio to me is complicated as custom laws are very
strict here (This is Cuba...), the only secure way of enter it is me
traveling aboard and carrying it back to Cuba as a Cuban Ham under
my ham license privileges. I hope POTUS visit ease the restrictions
here... some day.<br>
<br>
Also there are some other models that seems to be build on the same
base of this: QYT KT-8900, JT-6188, Tokmate 8900, Sainsonic GT-890,
Moonraker MT-270, Luiton LT-825U, Zastone MP-300, QYT KT UV-980,
etc...<br>
<br>
This radio base is apparently becoming in the Baofeng UV-5R for the
mobiles. <a class="moz-txt-link-freetext" href="http://chirp.danplanet.com/issues/2673">http://chirp.danplanet.com/issues/2673</a><br>
<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">I hope that there will be some further iteration on the timing stuff,
possibly with David's help from the Jetstream side.</pre>
</blockquote>
<br>
In deed, you will see more activity on this, we want to build also
the settings tabs for this and possibly for the others alike radios.<br>
<br>
Any help will be appreciated with this little beasts.<br>
<blockquote cite="mid:56F1F5FA.1060600@danplanet.com" type="cite">
<pre wrap="">
Thanks!
--Dan
</pre>
</blockquote>
<br>
Thanks to you for making Chirp open for us to participate, I'm
always open to learn and Chirp is a good one on this.<br>
<br>
I wait for your comment about this to improve the code.<br>
<br>
73 Pavel CO7WT.<br>
<br>
</body>
</html>