On 24/11/2014 23:23, Michael Black wrote:
Hi Mike,
Yes there are two functions which have no '\r'.
Set Transmit On/Off *T which gives a 1-byte return (that will be PTT via
CAT)
Set CWChar to Transmit *C2K which gives a 1-byte return (this one doesn't
matter for WSJT-X)
I'm not sure that's correct, those are both set commands not queries, if
I am reading the docs correctly they have no reply at all.
The problem I ran into was that the logic in omnivii.c was expecting the
count of all bytes to be returned and it was getting one less. Is that
going to be the only one that assumes that? Or perhaps all the alpha level
rigs should be looked at to ensure no others make that mistake?
I suspect the ten-tec back end is the main (only?) offender.
So instead of passing in expected length we should all be passing in
expected+1?
correct, it is clearly stated in the read_string() docs.
I'll revert iofunc.c to put the terminator in and make omnivii.c comply with
that.
I have the attached patch pending in my local git repo, I can't remember
why I haven't submitted it upstream. It is related to correct processing
of time outs in the Windows serial port code. Perhaps you can give it a
try. It fixes a problem where time outs are only correctly processed on
the first received character i.e. a problem when a shorter than expected
reply is received.
Mike W9MDB
73
Bill
G4WJS.
-----Original Message-----
From: Bill Somerville [mailto:[email protected]]
Sent: Monday, November 24, 2014 5:10 PM
To: [email protected]
Subject: Re: [wsjt-devel] Rig control problem with RigBlaster
On 24/11/2014 22:51, Michael Black wrote:
Hi Mike,
You referred to me to whatever version this is which I assume is the
hamlib3 custom one you've referred to for WSJT-X?
git clone git://git.code.sf.net/u/bsomervi/hamlib src
OK, some comments below.
Mike W9MDB
-----Original Message-----
From: Bill Somerville [mailto:[email protected]]
Sent: Monday, November 24, 2014 4:38 PM
To: [email protected]
Subject: Re: [wsjt-devel] Rig control problem with RigBlaster
On 24/11/2014 22:27, Michael Black wrote:
Hi Mike,
The string length logic seems a bit inconsistent on hamlib.
It looks to me like all rigs that use hamlib serial com should be
having problems.
I checked some other rigs and it appears they should have similar
problems with truncated responses.
If you read_string() with a correct expected length for the response
you get back length-1. Maybe this doesn't cause problems if most
rigs return a 0x0d or such on the end of their responses.
If you ask for more bytes than you expect you get a timeout and an
extra byte.
read_string() is clearly documented as taking the receive buffer length in
the rxmax argument, i.e. including room for the terminating '\0' char that
will be added to the end.
It's fairly obvious that you don't want to hit a timeout on every
command...so the "expected response length" should be the right argument.
The Omni VII has some commands which are binary and exact length is
returned plus the implementation for several commands was wrong and
I'm
fixing those.
Most protocols have a specific terminating character, as does the ten-tec
protocol, you will not get a time out if the terminating character is seen,
i.e. the '\r' char.
Looks to me that the whole problem stems from tentec_transaction() in
tentec.c not using the message termination character set arguments to
read_string(). Are there some ten-tec protocols that don;'t use the '\r'
terminator character?
I've got basic operation working now and am working on split mode.
In read_string() it does this incorrect logic which ends up return
one less byte than requested
while (total_count < rxmax-1) {
Presumably to make room for this
rxbuffer[total_count] = '\000';
I changed that and commented out the insertion of the terminator
while (total_count < rxmax) {
// rxbuffer[total_count] = '\000';
// maybe this should be rxbuffer[total_count+1]='\000' but I'm leary
of this since I'm sure many calling functions may not expect to have
to allow for an extra byte Either string_read should add the
terminating zero and the calling functions ensure they have length+1
bytes available....
Or...the calling functions should be responsible for ensuring a
terminating zero.
read_string() is clearly documented as requiring a buffer pointer and a
length including the space for the terminating '\0' char that will be added
to make the reply a valid C string. If you make that change you will break
many other back ends.
With that change it behaves for the Omni VII with the corrected
command set that I've implemented getting back the correct length for all
commands.
And I think the extra byte on timeout is fixed with this in iofunc.c
by adding a rd_count > 0 check instead of always incrementing total_count
rd_count = port_read(p, &rxbuffer[total_count], 1);
if (rd_count > 0) {
++total_count;
}
These changes are very familiar. What version of hamlib are you basing
your
changes on?
This area is definitely a bit dubious, I have made some fixes there but
it may still be broken.
The select further up is where the time out happens, total_count should
be incremented there because the read just above is guaranteed to have
read a character. If you are seeing the read being called after a time
out then something is broken in the select code.
Mike W9MDB
73
Bill
G4WJS.
73
Bill
G4WJS.
----------------------------------------------------------------------------
--
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
wsjt-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wsjt-devel
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
wsjt-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wsjt-devel
>From c967cb85394ada2faab20b76ffa4aec39940d210 Mon Sep 17 00:00:00 2001
From: Bill Somerville <[email protected]>
Date: Mon, 14 Oct 2013 22:58:18 +0100
Subject: [PATCH] Fix serial i/o on Windows.
ReadFile function called with incorrect argument according to MSDN
docs. Doesn't seem to change behaviour so probably not required.
read_string from cummunications port doesn't handle timeout on
anything but the first character read.
---
lib/termios.c | 12 ++++++------
src/iofunc.c | 24 +++++++++++-------------
2 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/lib/termios.c b/lib/termios.c
index 7c3e311..79419d8 100644
--- a/lib/termios.c
+++ b/lib/termios.c
@@ -1434,15 +1434,15 @@ int win32_serial_read( int fd, void *vb, int size )
index->rol.Offset = index->rol.OffsetHigh = 0;
ResetEvent( index->rol.hEvent );
- err = ReadFile( index->hComm, dest + total, size, &nBytes,
&index->rol );
+ err = ReadFile( index->hComm, dest + total, size, NULL,
&index->rol );
#ifdef DEBUG_VERBOSE
/* warning Roy Rogers! */
sprintf(message, " ========== ReadFile = %i %s\n",
( int ) nBytes, (char *) dest + total );
report( message );
#endif /* DEBUG_VERBOSE */
- size -= nBytes;
- total += nBytes;
+/* size -= nBytes; */
+/* total += nBytes; */
if ( !err )
{
@@ -1607,15 +1607,15 @@ int win32_serial_read( int fd, void *vb, int size )
index->rol.Offset = index->rol.OffsetHigh = 0;
ResetEvent( index->rol.hEvent );
- err = ReadFile( index->hComm, dest + total, size, &nBytes,
&index->rol );
+ err = ReadFile( index->hComm, dest + total, size, NULL,
&index->rol );
#ifdef DEBUG_VERBOSE
/* warning Roy Rogers! */
sprintf(message, " ========== ReadFile = %i %s\n",
( int ) nBytes, (char *) dest + total );
report( message );
#endif /* DEBUG_VERBOSE */
- size -= nBytes;
- total += nBytes;
+/* size -= nBytes; */
+/* total += nBytes; */
if ( !err )
{
diff --git a/src/iofunc.c b/src/iofunc.c
index b229b3a..492a1ba 100644
--- a/src/iofunc.c
+++ b/src/iofunc.c
@@ -504,8 +504,17 @@ int HAMLIB_API read_string(hamlib_port_t *p, char
*rxbuffer, size_t rxmax, const
efds = rfds;
retval = port_select(p, p->fd+1, &rfds, NULL, &efds, &tv);
- if (retval == 0) /* Timed out */
- break;
+ if (retval == 0) {
+ /* Record timeout time and caculate elapsed time */
+ gettimeofday(&end_time, NULL);
+ timersub(&end_time, &start_time, &elapsed_time);
+
+ dump_hex((unsigned char *) rxbuffer, total_count);
+ rig_debug(RIG_DEBUG_WARN, "%s(): Timed out %d.%d seconds after
%d chars\n",
+ __func__, elapsed_time.tv_sec, elapsed_time.tv_usec,
total_count);
+
+ return -RIG_ETIMEOUT;
+ }
if (retval < 0) {
dump_hex((unsigned char *) rxbuffer, total_count);
@@ -543,17 +552,6 @@ int HAMLIB_API read_string(hamlib_port_t *p, char
*rxbuffer, size_t rxmax, const
*/
rxbuffer[total_count] = '\000';
- if (total_count == 0) {
- /* Record timeout time and caculate elapsed time */
- gettimeofday(&end_time, NULL);
- timersub(&end_time, &start_time, &elapsed_time);
-
- rig_debug(RIG_DEBUG_WARN, "%s(): Timed out %d.%d seconds without reading a
character.\n",
- __func__, elapsed_time.tv_sec, elapsed_time.tv_usec);
-
- return -RIG_ETIMEOUT;
- }
-
rig_debug(RIG_DEBUG_TRACE,"%s(): RX %d characters\n", __func__, total_count);
dump_hex((unsigned char *) rxbuffer, total_count);
--
1.8.3.msysgit.0
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
wsjt-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wsjt-devel
