[Hamlib-developer] Proposed patch for tranceive hanging
Library to control radio transceivers and receivers
Brought to you by:
n0nb
|
From: Chuck H. <n2...@am...> - 2002-03-10 13:51:48
|
Short description:
Here's a patch to fix my tranceive problem a a bit more. (More like a re-write
of a small piece of the icom frame handling). Feel free to use it, ignore it,
use pieces of it, ask me to change and/or extend it, or whatever.
---
Long description:
I went to try to fix my problems with tranceive hanging in icom_decode, and
also saw the comment in read_icom_frame:
* However, an automate with a state model would be more efficient..
In doing this I was almost ready to write a new routine, then I noticed
read_string was close to what I wanted:
1. It didn't do any reads without a select (which solved my tranceive problem)
2. It could read until a given set of stop characters (as in: FI or COL)
3. It only printed out the complete frame with dump_hex rather than the start
followed by each additional character.
However, it didn't support one thing I needed:
1. Support for 00 as a character in the block.
So I copied it and created a new routine "read_block_until" (feel free to
change the name) for reading a block (that can include 00) up until a set of
stop characters (which may or may not include 00). Then I changed
read_icom_block and read_icom_frame to call this new routine.
(see docs at top of read_block_until for more information about it)
I tested it, and it seems to work fine here with my R7000.
Feel free use it, parts of it, change it, or whatever.
Since I didn't know what format you wanted it in, I attached a patch in
"diff -urN" format.
I also attached copies of the the new read_block_until and modified
read_icom_frame and read_icom_block to make them easier to read and/or do
something else with them.
(tried just sticking them in the message, but my email program decided to word
wrap them, which would have made it difficult to do anything but read them)
If you want anything different, let me know.
---
A few other things:
1. read_icom_block and read_icom_frame are very similar now. I didn't know
what you had planned for each of them.
2. I didn't add any additional checking for collisions or timeouts after
receiving at least one character. Timeouts before the first character are
handled as before.
Example collision/timeout processing:
rxlen = read_icom_block(
or
rxlen = read_icom_frame(
if(rxlen <= 0)
{
/* normal error handling including timeouts before the first character*/
/* should never be 0 */
} else {
select(rxbuffer[rxlen-1])
{
case FI: /* Normal frame */
case COL: /* Collision */
default: /* timeout after receiving at least one character */
}
If you would like me to work on this, let me know. Also, I didn't notice an
error message for a collision. Didn't know if you wanted a new one, or fit
it in one of the others.
Note: The docs I have say the radio sends the collision/jammer code 5 times,
waits for no more incoming data and then resends the frame. Just a word of
warning for anyone trying to implement collision recovery.
BTW With a bit a playing I did cause a collision. In the trace, I saw
multiple calls to icom_decode in a row:
icom_decode: tranceive cmd unsupported 0x05
sa_sigioaction: activity detected
icom: icom_decode called
RX 1 characters
0000 fc .
icom_decode: CI-V 0x8 called for 0xe0!
Now, this error comes from the fact that it's trying to look at parts of the
buffer that are not initialized. With my current xterm buffer size, I
couldn't scroll back far enough to see which routine got the collision first.
I'll come up with a better trace if need be.
3. Just a note to anyone who does want to try to implement resend on
error/collisions:
It would be nice if there was a way to turn it off.
A collision could easily be caused by someone turning the knob while the
software is trying to do something to the radio. In that case, my
software would prefer to know that the knob was turned rather than the
command was done correctly so it doesn't lose any knob turns. I understand
that many other pieces of software would prefer their commands to work and
don't want to have to worry about handling the errors.
(Well, the error handling in my software is fairly simple right now. :)
It just does a get after every set, and uses the last successful get as the
number to check the new calculated value against when deciding to an
update.
Also, I have implemented a semi-atomic update that does a get, checks to see
if the value is the same as the old value, and if it is, then it does a set
followed by another get. This helps catch any knob changes that it might
miss due to delays and such)
4. The old read_icom_frame didn't didn't worry about the max rxbuffer length.
This is a good thing to check just in case the radio/ci-v bus decided to
send lots of garbage. Since this new version does check, I looked at a
bunch of the callers.
Most of them call it with a buffer of size of 16, however icom_decode calls
it with a buffer size of 32. This should probably either be passed as an
argument to read_icom_frame, a #define'ed constant, or at least consistent.
For now, I set it to 16 in read_icom_frame. This will cause problems if
icom_decode_frame gets a frame between 17 and 32 characters (Not seen in
my testing)
5. Because I needed a character array with FI and COL in it in two different
routines and couldn't figure out how to use a #define to create it, I
defined it as a "static const char" in "icom/frame.c" (not inside of
either of the routines, but not EXTERN either).
If you have any better ideas of what you want to do with it, go ahead and
change it or let me know.
6. In testtrn I added a printf of the idle loop counter so as to better show
that it's working. Otherwise you can't tell if something is hung in
tranceive handing routines without waiting to see if the program exits
after 120 seconds. Feel free to ignore/change this if you don't want it.
7. read_block_until uses read rather than fread. I don't know if it was
changed to use fread in the past for a reason, but sharing the same
standard i/o file descriptor between "normal" and signal handler contexts
makes me nervous. Someone else may have a better idea of what is allowed.
You also could add buffering directly to read_block_until, but to do that
you would need:
a. A place to store characters read, but beyond the end of the current frame.
b. You might want to think a bit more about how you want to share a ci-v bus
between multiple items. With this, right now, non-tranceive stuff
probably works because it doesn't read anything it doesn't handle.
return i;
8. If you don't like memchr (I needed a routine to that treated 00 as a normal
char), it could easily be replaced by a for loop.
|