mirror
/
Hamlib
kopia lustrzana https://github.com/Hamlib/Hamlib
1
0
Forkuj 0
Kod Zgłoszenia Projekty Wydania Wiki Aktywność

Allow icom_rig_open() to succeed even if the rig is powered off. Fix issues with async I/O error code propagation. Allow rigctld daemon to start even if rig is powered off. Add mutex locks around rig_open()/rig_close() calls as multiple client connections could end up calling them concurrently, which could lead to all kinds of issues as the functions are not meant to be thread-safe. This also blocks execution of rigctl commands during rig open/close.

pull/943/head
Mikael Nousiainen 2022-01-22 23:19:07 +02:00
rodzic be992b8a36
commit 1f428c8d95
4 zmienionych plików z 93 dodań i 56 usunięć
Pokaż statystyki Ściągnij plik aktualizacji Ściągnij plik porównania Expand all files Collapse all files

31
rigs/icom/icom.c
Wyświetl plik

@ -929,7 +929,7 @@ icom_rig_open(RIG *rig)
struct rig_state *rs = &rig->state; struct rig_state *rs = &rig->state;
struct icom_priv_data *priv = (struct icom_priv_data *) rs->priv; struct icom_priv_data *priv = (struct icom_priv_data *) rs->priv;
int retry_flag = 1; int retry_flag = 1;
int retry_save = rs->rigport.retry; short retry_save = rs->rigport.retry;
ENTERFUNC; ENTERFUNC;
@ -969,7 +969,7 @@ retry_open:
retval = abs(rig_set_powerstat(rig, 1)); retval = abs(rig_set_powerstat(rig, 1));
// this is only a fatal error if powerstat is implemented // this is only a fatal error if powerstat is implemented
// if not iplemented than we're at an error here // if not implemented than we're at an error here
if (retval != RIG_OK && retval != RIG_ENIMPL && retval != RIG_ENAVAIL) if (retval != RIG_OK && retval != RIG_ENIMPL && retval != RIG_ENAVAIL)
{ {
rig_debug(RIG_DEBUG_WARN, "%s: unexpected retval here: %s\n", rig_debug(RIG_DEBUG_WARN, "%s: unexpected retval here: %s\n",
@ -978,17 +978,18 @@ retry_open:
rig_debug(RIG_DEBUG_WARN, "%s: rig_set_powerstat failed: =%s\n", __func__, rig_debug(RIG_DEBUG_WARN, "%s: rig_set_powerstat failed: =%s\n", __func__,
rigerror(retval)); rigerror(retval));
rs->rigport.retry = retry_save; rs->rigport.retry = retry_save;
RETURNFUNC(retval);
} }
// Now that we're powered up let's try again if (retval == RIG_OK)
retval_echo = icom_get_usb_echo_off(rig);
if (retval_echo != 0 && retval_echo != 1)
{ {
rig_debug(RIG_DEBUG_ERR, "%s: Unable to determine USB echo status\n", __func__); // Now that we're powered up let's try again
rs->rigport.retry = retry_save; retval_echo = icom_get_usb_echo_off(rig);
RETURNFUNC(retval);
if (retval_echo != 0 && retval_echo != 1)
{
rig_debug(RIG_DEBUG_ERR, "%s: Unable to determine USB echo status\n", __func__);
rs->rigport.retry = retry_save;
}
} }
} }
else if (retval != RIG_OK) else if (retval != RIG_OK)
@ -1005,11 +1006,14 @@ retry_open:
} }
rs->rigport.retry = retry_save; rs->rigport.retry = retry_save;
RETURNFUNC(retval);
} }
rig->state.current_vfo = icom_current_vfo(rig); priv->poweron = (retval == RIG_OK) ? 1 : 0;
priv->poweron = 1;
if (priv->poweron)
{
rig->state.current_vfo = icom_current_vfo(rig);
}
if (rig->caps->has_get_func & RIG_FUNC_SATMODE) if (rig->caps->has_get_func & RIG_FUNC_SATMODE)
{ {
@ -7834,6 +7838,7 @@ int icom_set_powerstat(RIG *rig, powerstat_t status)
if (retval == RIG_OK) if (retval == RIG_OK)
{ {
rig->state.current_vfo = icom_current_vfo(rig);
RETURNFUNC(retval); RETURNFUNC(retval);
} }
else else

65
src/iofunc.c
Wyświetl plik

@ -437,7 +437,7 @@ extern int is_uh_radio_fd(int fd);
static int port_read_sync_data_error_code(hamlib_port_t *p) static int port_read_sync_data_error_code(hamlib_port_t *p)
{ {
ssize_t total_bytes_read = 0; ssize_t total_bytes_read = 0;
unsigned char data; signed char data;
int result; int result;
do { do {
@ -810,13 +810,14 @@ static ssize_t port_read_generic(hamlib_port_t *p, void *buf, size_t count, int
#define port_select(p,n,r,w,e,t,d) select((n),(r),(w),(e),(t)) #define port_select(p,n,r,w,e,t,d) select((n),(r),(w),(e),(t))
//! @endcond //! @endcond
static int flush_and_read_last_byte(hamlib_port_t *p, int fd, int direct) static int port_read_sync_data_error_code(hamlib_port_t *p, int fd, int direct)
{ {
fd_set rfds, efds; fd_set rfds, efds;
ssize_t total_bytes_read = 0;
ssize_t bytes_read; ssize_t bytes_read;
struct timeval tv_timeout; struct timeval tv_timeout;
int result; int result;
char data; signed char data;
do { do {
tv_timeout.tv_sec = 0; tv_timeout.tv_sec = 0;
@ -829,21 +830,32 @@ static int flush_and_read_last_byte(hamlib_port_t *p, int fd, int direct)
result = port_select(p, fd + 1, &rfds, NULL, &efds, &tv_timeout, direct); result = port_select(p, fd + 1, &rfds, NULL, &efds, &tv_timeout, direct);
if (result < 0) if (result < 0)
{ {
rig_debug(RIG_DEBUG_VERBOSE, "%s(): select() timeout, direct=%d\n", __func__, direct);
return -RIG_ETIMEOUT; return -RIG_ETIMEOUT;
} }
if (result == 0) if (result == 0)
{ {
if (total_bytes_read > 0) {
rig_debug(RIG_DEBUG_VERBOSE, "%s(): returning error code %d, direct=%d\n", __func__, (int) data, direct);
return data;
}
rig_debug(RIG_DEBUG_ERR, "%s(): no error code available\n", __func__);
return -RIG_EIO; return -RIG_EIO;
} }
if (FD_ISSET(fd, &efds)) if (FD_ISSET(fd, &efds))
{ {
rig_debug(RIG_DEBUG_ERR, "%s(): select() indicated error\n", __func__);
return -RIG_EIO; return -RIG_EIO;
} }
bytes_read = read(fd, &data, 1); bytes_read = read(fd, &data, 1);
total_bytes_read += bytes_read;
} while (bytes_read > 0); } while (bytes_read > 0);
rig_debug(RIG_DEBUG_VERBOSE, "%s(): returning error code %d\n", __func__, data);
return data; return data;
} }
@ -886,28 +898,30 @@ static int port_wait_for_data(hamlib_port_t *p, int direct)
else if (result < 0) else if (result < 0)
{ {
rig_debug(RIG_DEBUG_ERR, rig_debug(RIG_DEBUG_ERR,
"%s(): select() error: %s\n", "%s(): select() error, direct=%d: %s\n",
__func__, __func__,
direct,
strerror(errno)); strerror(errno));
return -RIG_EIO; return -RIG_EIO;
} }
if (FD_ISSET(fd, &efds)) if (FD_ISSET(fd, &efds))
{ {
rig_debug(RIG_DEBUG_ERR, "%s(): fd error\n", __func__); rig_debug(RIG_DEBUG_ERR, "%s(): fd error, direct=%d\n", __func__, direct);
return -RIG_EIO; return -RIG_EIO;
} }
if (!direct) if (!direct)
{ {
if (FD_ISSET(errorfd, &efds)) if (FD_ISSET(errorfd, &efds))
{ {
rig_debug(RIG_DEBUG_ERR, "%s(): fd error from sync error pipe\n", __func__); rig_debug(RIG_DEBUG_ERR, "%s(): fd error from sync error pipe, direct=%d\n", __func__, direct);
return -RIG_EIO; return -RIG_EIO;
} }
if (FD_ISSET(errorfd, &rfds)) if (FD_ISSET(errorfd, &rfds))
{ {
return flush_and_read_last_byte(p, errorfd, 0); rig_debug(RIG_DEBUG_VERBOSE, "%s(): attempting to read error code, direct=%d\n", __func__, direct);
return port_read_sync_data_error_code(p, errorfd, 0);
} }
} }
@ -1078,7 +1092,7 @@ static int read_block_generic(hamlib_port_t *p, unsigned char *rxbuffer, size_t
struct timeval start_time, end_time, elapsed_time; struct timeval start_time, end_time, elapsed_time;
int total_count = 0; int total_count = 0;
rig_debug(RIG_DEBUG_VERBOSE, "%s called\n", __func__); rig_debug(RIG_DEBUG_VERBOSE, "%s called, direct=%d\n", __func__, direct);
#ifdef ASYNC_BUG #ifdef ASYNC_BUG
if (!p->async && !direct) if (!p->async && !direct)
@ -1111,11 +1125,12 @@ static int read_block_generic(hamlib_port_t *p, unsigned char *rxbuffer, size_t
} }
rig_debug(RIG_DEBUG_WARN, rig_debug(RIG_DEBUG_WARN,
"%s(): Timed out %d.%d seconds after %d chars\n", "%s(): Timed out %d.%d seconds after %d chars, direct=%d\n",
__func__, __func__,
(int)elapsed_time.tv_sec, (int)elapsed_time.tv_sec,
(int)elapsed_time.tv_usec, (int)elapsed_time.tv_usec,
total_count); total_count,
direct);
return -RIG_ETIMEOUT; return -RIG_ETIMEOUT;
} }
@ -1126,7 +1141,7 @@ static int read_block_generic(hamlib_port_t *p, unsigned char *rxbuffer, size_t
{ {
dump_hex((unsigned char *) rxbuffer, total_count); dump_hex((unsigned char *) rxbuffer, total_count);
} }
rig_debug(RIG_DEBUG_ERR, "%s(): I/O error after %d chars: %d\n", __func__, total_count, result); rig_debug(RIG_DEBUG_ERR, "%s(): I/O error after %d chars, direct=%d: %d\n", __func__, total_count, direct, result);
return result; return result;
} }
@ -1138,7 +1153,7 @@ static int read_block_generic(hamlib_port_t *p, unsigned char *rxbuffer, size_t
if (rd_count < 0) if (rd_count < 0)
{ {
rig_debug(RIG_DEBUG_ERR, "%s(): read failed - %s\n", __func__, strerror(errno)); rig_debug(RIG_DEBUG_ERR, "%s(): read failed, direct=%d - %s\n", __func__, direct, strerror(errno));
return -RIG_EIO; return -RIG_EIO;
} }
@ -1148,7 +1163,7 @@ static int read_block_generic(hamlib_port_t *p, unsigned char *rxbuffer, size_t
if (direct) if (direct)
{ {
rig_debug(RIG_DEBUG_TRACE, "%s(): RX %d bytes\n", __func__, total_count); rig_debug(RIG_DEBUG_TRACE, "%s(): RX %d bytes, direct=%d\n", __func__, total_count, direct);
dump_hex((unsigned char *) rxbuffer, total_count); dump_hex((unsigned char *) rxbuffer, total_count);
} }
@ -1229,7 +1244,7 @@ static int read_string_generic(hamlib_port_t *p,
return -RIG_EINTERNAL; return -RIG_EINTERNAL;
} }
rig_debug(RIG_DEBUG_TRACE, "%s called, rxmax=%d\n", __func__, (int)rxmax); rig_debug(RIG_DEBUG_TRACE, "%s called, rxmax=%d direct=%d\n", __func__, (int)rxmax, direct);
if (!p || !rxbuffer) if (!p || !rxbuffer)
{ {
@ -1272,11 +1287,12 @@ static int read_string_generic(hamlib_port_t *p,
if (!flush_flag) if (!flush_flag)
{ {
rig_debug(RIG_DEBUG_WARN, rig_debug(RIG_DEBUG_WARN,
"%s(): Timed out %d.%03d seconds after %d chars\n", "%s(): Timed out %d.%03d seconds after %d chars, direct=%d\n",
__func__, __func__,
(int)elapsed_time.tv_sec, (int)elapsed_time.tv_sec,
(int)elapsed_time.tv_usec / 1000, (int)elapsed_time.tv_usec / 1000,
total_count); total_count,
direct);
} }
return -RIG_ETIMEOUT; return -RIG_ETIMEOUT;
@ -1291,7 +1307,7 @@ static int read_string_generic(hamlib_port_t *p,
{ {
dump_hex(rxbuffer, total_count); dump_hex(rxbuffer, total_count);
} }
rig_debug(RIG_DEBUG_ERR, "%s(): I/O error after %d chars: %d\n", __func__, total_count, result); rig_debug(RIG_DEBUG_ERR, "%s(): I/O error after %d chars, direct=%d: %d\n", __func__, total_count, direct, result);
return result; return result;
} }
@ -1306,7 +1322,7 @@ static int read_string_generic(hamlib_port_t *p,
if (errno == EAGAIN) if (errno == EAGAIN)
{ {
hl_usleep(5 * 1000); hl_usleep(5 * 1000);
rig_debug(RIG_DEBUG_WARN, "%s: port_read is busy?\n", __func__); rig_debug(RIG_DEBUG_WARN, "%s: port_read is busy? direct=%d\n", __func__, direct);
} }
} }
while (++i < 10 && errno == EBUSY); // 50ms should be enough while (++i < 10 && errno == EBUSY); // 50ms should be enough
@ -1318,7 +1334,7 @@ static int read_string_generic(hamlib_port_t *p,
{ {
dump_hex((unsigned char *) rxbuffer, total_count); dump_hex((unsigned char *) rxbuffer, total_count);
} }
rig_debug(RIG_DEBUG_ERR, "%s(): read failed - %s\n", __func__, strerror(errno)); rig_debug(RIG_DEBUG_ERR, "%s(): read failed, direct=%d - %s\n", __func__, direct, strerror(errno));
return -RIG_EIO; return -RIG_EIO;
} }
@ -1348,14 +1364,15 @@ static int read_string_generic(hamlib_port_t *p,
*/ */
rxbuffer[total_count] = '\000'; rxbuffer[total_count] = '\000';
if (direct) // if (direct)
{ // {
rig_debug(RIG_DEBUG_TRACE, rig_debug(RIG_DEBUG_TRACE,
"%s(): RX %d characters\n", "%s(): RX %d characters, direct=%d\n",
__func__, __func__,
total_count); total_count,
direct);
dump_hex((unsigned char *) rxbuffer, total_count); dump_hex((unsigned char *) rxbuffer, total_count);
} // }
return total_count; /* return bytes count read */ return total_count; /* return bytes count read */
} }

17
src/rig.c
Wyświetl plik

@ -1070,10 +1070,12 @@ int HAMLIB_API rig_open(RIG *rig)
if (status != RIG_OK) if (status != RIG_OK)
{ {
remove_opened_rig(rig);
#ifdef ASYNC_BUG #ifdef ASYNC_BUG
async_data_handler_stop(rig); async_data_handler_stop(rig);
#endif #endif
port_close(&rs->rigport, rs->rigport.type.rig); port_close(&rs->rigport, rs->rigport.type.rig);
rs->comm_state = 0;
RETURNFUNC(status); RETURNFUNC(status);
} }
} }
@ -6992,15 +6994,16 @@ void *async_data_handler(void *arg)
if (result < 0) if (result < 0)
{ {
// TODO: it may be necessary to have mutex locking on transaction_active flag // Timeouts occur always if there is nothing to receive, so they are not really errors in this case
if (rs->transaction_active)
{
unsigned char data = (unsigned char) result;
write_block_sync_error(&rs->rigport, &data, 1);
}
if (result != -RIG_ETIMEOUT) if (result != -RIG_ETIMEOUT)
{ {
// TODO: it may be necessary to have mutex locking on transaction_active flag
if (rs->transaction_active)
{
unsigned char data = (unsigned char) result;
write_block_sync_error(&rs->rigport, &data, 1);
}
// TODO: error handling -> store errors in rig state -> to be exposed in async snapshot packets // TODO: error handling -> store errors in rig state -> to be exposed in async snapshot packets
rig_debug(RIG_DEBUG_ERR, "%s: read_frame_direct() failed, result=%d\n", rig_debug(RIG_DEBUG_ERR, "%s: read_frame_direct() failed, result=%d\n",
__func__, result); __func__, result);

36
tests/rigctld.c
Wyświetl plik

@ -135,6 +135,7 @@ static unsigned client_count;
#endif #endif
static RIG *my_rig; /* handle to rig (instance) */ static RIG *my_rig; /* handle to rig (instance) */
static volatile int rig_opened = 0;
static int verbose; static int verbose;
#ifdef HAVE_SIG_ATOMIC_T #ifdef HAVE_SIG_ATOMIC_T
@ -687,14 +688,15 @@ int main(int argc, char *argv[])
exit(0); exit(0);
} }
/* open and close rig connection to check early for issues */ /* attempt to open rig to check early for issues */
retcode = rig_open(my_rig); retcode = rig_open(my_rig);
rig_opened = retcode == RIG_OK ? 1 : 0;
if (retcode != RIG_OK) if (retcode != RIG_OK)
{ {
fprintf(stderr, "rig_open: error = %s %s %s \n", rigerror(retcode), rig_file, fprintf(stderr, "rig_open: error = %s %s %s \n", rigerror(retcode), rig_file,
strerror(errno)); strerror(errno));
exit(2); // continue even if opening the rig fails, because it may be powered off
} }
if (verbose > RIG_DEBUG_ERR) if (verbose > RIG_DEBUG_ERR)
@ -1010,8 +1012,6 @@ int main(int argc, char *argv[])
} }
while (retcode == 0 && !ctrl_c); while (retcode == 0 && !ctrl_c);
network_multicast_publisher_stop(my_rig);
#ifdef HAVE_PTHREAD #ifdef HAVE_PTHREAD
/* allow threads to finish current action */ /* allow threads to finish current action */
mutex_rigctld(1); mutex_rigctld(1);
@ -1026,6 +1026,9 @@ int main(int argc, char *argv[])
#else #else
rig_close(my_rig); /* close port */ rig_close(my_rig); /* close port */
#endif #endif
network_multicast_publisher_stop(my_rig);
rig_cleanup(my_rig); /* if you care about memory */ rig_cleanup(my_rig); /* if you care about memory */
#ifdef __MINGW32__ #ifdef __MINGW32__
@ -1132,10 +1135,9 @@ void *handle_socket(void *arg)
#endif #endif
int rig_opened = 1; // our rig is already open
do do
{ {
mutex_rigctld(1);
if (!rig_opened) if (!rig_opened)
{ {
retcode = rig_open(my_rig); retcode = rig_open(my_rig);
@ -1143,6 +1145,7 @@ void *handle_socket(void *arg)
rig_debug(RIG_DEBUG_ERR, "%s: rig_open reopened retcode=%d\n", __func__, rig_debug(RIG_DEBUG_ERR, "%s: rig_open reopened retcode=%d\n", __func__,
retcode); retcode);
} }
mutex_rigctld(0);
if (rig_opened) // only do this if rig is open if (rig_opened) // only do this if rig is open
{ {
@ -1168,15 +1171,24 @@ void *handle_socket(void *arg)
do do
{ {
mutex_rigctld(1);
retcode = rig_close(my_rig); retcode = rig_close(my_rig);
hl_usleep(1000 * 1000); rig_opened = 0;
mutex_rigctld(0);
rig_debug(RIG_DEBUG_ERR, "%s: rig_close retcode=%d\n", __func__, retcode); rig_debug(RIG_DEBUG_ERR, "%s: rig_close retcode=%d\n", __func__, retcode);
retcode = rig_open(my_rig);
rig_opened = retcode == RIG_OK ? 1 : 0; hl_usleep(1000 * 1000);
rig_debug(RIG_DEBUG_ERR, "%s: rig_open retcode=%d, opened=%d\n", __func__,
retcode, rig_opened); mutex_rigctld(1);
if (!rig_opened) {
retcode = rig_open(my_rig);
rig_opened = retcode == RIG_OK ? 1 : 0;
rig_debug(RIG_DEBUG_ERR, "%s: rig_open retcode=%d, opened=%d\n", __func__,
retcode, rig_opened);
}
mutex_rigctld(0);
} }
while (retry-- > 0 && retcode != RIG_OK); while (!rig_opened && retry-- > 0 && retcode != RIG_OK);
} }
} }
while (retcode == RIG_OK || RIG_IS_SOFT_ERRCODE(-retcode)); while (retcode == RIG_OK || RIG_IS_SOFT_ERRCODE(-retcode));