From 7d5f540b9c367d2c3e7e6ef2f9b154e425c5fe44 Mon Sep 17 00:00:00 2001 From: Mikael Nousiainen Date: Mon, 20 Nov 2023 10:15:45 +0200 Subject: [PATCH] Fix bugs in dummy rig. Report errors (incl. timeouts) from icom_get_powerstat(), because the timeout reason is often something else than rig being powered off. Check for power status changes in rigctld/rigctl command loops. --- rigs/dummy/dummy.c | 35 +++++++++++++++++++++++++++++------ rigs/icom/icom.c | 28 ++++++++++++++-------------- tests/rigctl.c | 10 ++++++++-- tests/rigctl_parse.c | 1 + tests/rigctld.c | 11 +++++++++-- 5 files changed, 61 insertions(+), 24 deletions(-) diff --git a/rigs/dummy/dummy.c b/rigs/dummy/dummy.c index f1eeffb11..77bbb659e 100644 --- a/rigs/dummy/dummy.c +++ b/rigs/dummy/dummy.c @@ -533,6 +533,8 @@ static int dummy_set_mode(RIG *rig, vfo_t vfo, rmode_t mode, pbwidth_t width) vfo = vfo_fixup(rig, vfo, rig->state.cache.split); + if (vfo == RIG_VFO_CURR) { vfo = rig->state.current_vfo; } + if (width == RIG_PASSBAND_NOCHANGE) { switch (vfo) @@ -613,12 +615,34 @@ static int dummy_get_mode(RIG *rig, vfo_t vfo, rmode_t *mode, pbwidth_t *width) switch (vfo) { case RIG_VFO_MAIN: - case RIG_VFO_A: *mode = priv->vfo_a.mode; *width = priv->vfo_a.width; break; + case RIG_VFO_A: + *mode = priv->vfo_a.mode; *width = priv->vfo_a.width; + break; + + case RIG_VFO_MAIN_A: + *mode = priv->vfo_maina.mode; *width = priv->vfo_maina.width; + break; + + case RIG_VFO_MAIN_B: + *mode = priv->vfo_mainb.mode; *width = priv->vfo_mainb.width; + break; case RIG_VFO_SUB: - case RIG_VFO_B: *mode = priv->vfo_b.mode; *width = priv->vfo_b.width; break; + case RIG_VFO_B: + *mode = priv->vfo_b.mode; *width = priv->vfo_b.width; + break; - case RIG_VFO_C: *mode = priv->vfo_c.mode; *width = priv->vfo_c.width; break; + case RIG_VFO_SUB_A: + *mode = priv->vfo_suba.mode; *width = priv->vfo_suba.width; + break; + + case RIG_VFO_SUB_B: + *mode = priv->vfo_subb.mode; *width = priv->vfo_subb.width; + break; + + case RIG_VFO_C: + *mode = priv->vfo_c.mode; *width = priv->vfo_c.width; + break; } RETURNFUNC(RIG_OK); @@ -636,9 +660,6 @@ static int dummy_set_vfo(RIG *rig, vfo_t vfo) if (vfo == RIG_VFO_CURR) { vfo = rig->state.current_vfo; } - priv->last_vfo = priv->curr_vfo; - priv->curr_vfo = vfo; - switch (vfo) { case RIG_VFO_VFO: /* FIXME */ @@ -683,6 +704,8 @@ static int dummy_set_vfo(RIG *rig, vfo_t vfo) RETURNFUNC(-RIG_EINVAL); } + priv->last_vfo = priv->curr_vfo; + priv->curr_vfo = vfo; rig->state.current_vfo = vfo; RETURNFUNC(RIG_OK); diff --git a/rigs/icom/icom.c b/rigs/icom/icom.c index 6d34397fb..c54c87427 100644 --- a/rigs/icom/icom.c +++ b/rigs/icom/icom.c @@ -7799,26 +7799,27 @@ int icom_get_powerstat(RIG *rig, powerstat_t *status) freq_t freq; short retry_save = rig->state.rigport.retry; short timeout_retry_save = rig->state.rigport.timeout_retry; - HAMLIB_TRACE; + HAMLIB_TRACE; rig->state.rigport.retry = 0; rig->state.rigport.timeout_retry = 0; retval = rig_get_freq(rig, RIG_VFO_A, &freq); - if (retval == -RIG_ETIMEOUT) - { // then rig must be turned off - HAMLIB_TRACE; - rig_debug(RIG_DEBUG_WARN, "%s: get freq failed...assuming power is off\n", __func__); - rig->state.powerstat = RIG_POWER_OFF; - return RIG_OK; // returning RIG_OK here makes the rig->state reflect POWER_OFF + if (retval != RIG_OK) + { + rig_debug(RIG_DEBUG_WARN, "%s: get freq failed, assuming power is off\n", __func__); } - HAMLIB_TRACE; + HAMLIB_TRACE; rig->state.rigport.retry = retry_save; rig->state.rigport.timeout_retry = timeout_retry_save; + // Assume power is OFF if get_freq fails *status = retval == RIG_OK ? RIG_POWER_ON : RIG_POWER_OFF; + + // Modify rig_state powerstat directly to reflect power ON/OFF status, but return the result of rig_get_freq, + // because the error could indicate other connectivity issues too rig->state.powerstat = *status; return retval; } @@ -7827,14 +7828,13 @@ int icom_get_powerstat(RIG *rig, powerstat_t *status) retval = icom_transaction(rig, C_SET_PWR, -1, NULL, 0, ackbuf, &ack_len); - if (retval == -RIG_ETIMEOUT) - { // then rig must be turned off - rig_debug(RIG_DEBUG_WARN, "%s: get powerstat failed...assuming power is off\n", __func__); - rig->state.powerstat = RIG_POWER_OFF; - return RIG_OK; // returning RIG_OK here makes the rig->state reflect POWER_OFF - } if (retval != RIG_OK) { + // Assume power is OFF if getting power status fails + // Modify rig_state powerstat directly to reflect power ON/OFF status, but return the result of rig_get_freq, + // because the error could indicate other connectivity issues too + rig_debug(RIG_DEBUG_WARN, "%s: get powerstat failed, assuming power is off\n", __func__); + rig->state.powerstat = RIG_POWER_OFF; RETURNFUNC(retval); } diff --git a/tests/rigctl.c b/tests/rigctl.c index 73e5380ad..619f92213 100644 --- a/tests/rigctl.c +++ b/tests/rigctl.c @@ -227,6 +227,7 @@ int main(int argc, char *argv[]) char rigstartup[1024]; char vbuf[1024]; rig_powerstat = RIG_POWER_ON; // defaults to power on + struct timespec powerstat_check_time; #if HAVE_SIGACTION struct sigaction act; #endif @@ -814,6 +815,8 @@ int main(int argc, char *argv[]) #endif #endif + elapsed_ms(&powerstat_check_time, HAMLIB_ELAPSED_SET); + do { if (!rig_opened) @@ -829,10 +832,11 @@ int main(int argc, char *argv[]) // If we get a timeout, the rig might be powered off // Update our power status in case power gets turned off - if (retcode == -RIG_ETIMEOUT && my_rig->caps->get_powerstat) + // Check power status if rig is powered off, but not more often than once per second + if (my_rig->caps->get_powerstat && (retcode == -RIG_ETIMEOUT || + (retcode == -RIG_EPOWER && elapsed_ms(&powerstat_check_time, HAMLIB_ELAPSED_GET) >= 1000))) { powerstat_t powerstat; - rig_get_powerstat(my_rig, &powerstat); rig_powerstat = powerstat; @@ -840,6 +844,8 @@ int main(int argc, char *argv[]) { retcode = -RIG_EPOWER; } + + elapsed_ms(&powerstat_check_time, HAMLIB_ELAPSED_SET); } // if we get a hard error we try to reopen the rig again diff --git a/tests/rigctl_parse.c b/tests/rigctl_parse.c index c47a87fe6..8d52718f0 100644 --- a/tests/rigctl_parse.c +++ b/tests/rigctl_parse.c @@ -1759,6 +1759,7 @@ readline_repeat: && cmd_entry->cmd != 0x87 // set_powerstat && cmd_entry->cmd != 0x88 // get_powerstat && cmd_entry->cmd != 0xa5 // client_version + && cmd_entry->cmd != 0xf2 // set_vfo_opt && my_rig->caps->rig_model != RIG_MODEL_POWERSDR) // some rigs can do stuff when powered off { diff --git a/tests/rigctld.c b/tests/rigctld.c index 2bce5b814..c0e571301 100644 --- a/tests/rigctld.c +++ b/tests/rigctld.c @@ -1192,6 +1192,7 @@ void *handle_socket(void *arg) char send_cmd_term = '\r'; /* send_cmd termination char */ int ext_resp = 0; rig_powerstat = RIG_POWER_ON; // defaults to power on + struct timespec powerstat_check_time; fsockin = get_fsockin(handle_data_arg); @@ -1257,6 +1258,8 @@ void *handle_socket(void *arg) my_rig->state.powerstat = rig_powerstat; } + elapsed_ms(&powerstat_check_time, HAMLIB_ELAPSED_SET); + do { mutex_rigctld(1); @@ -1273,7 +1276,6 @@ void *handle_socket(void *arg) if (rig_opened) // only do this if rig is open { - powerstat_t powerstat; rig_debug(RIG_DEBUG_TRACE, "%s: doing rigctl_parse vfo_mode=%d, secure=%d\n", __func__, handle_data_arg->vfo_mode, handle_data_arg->use_password); @@ -1286,8 +1288,11 @@ void *handle_socket(void *arg) // If we get a timeout, the rig might be powered off // Update our power status in case power gets turned off - if (retcode == -RIG_ETIMEOUT && my_rig->caps->get_powerstat) + // Check power status if rig is powered off, but not more often than once per second + if (my_rig->caps->get_powerstat && (retcode == -RIG_ETIMEOUT || + (retcode == -RIG_EPOWER && elapsed_ms(&powerstat_check_time, HAMLIB_ELAPSED_GET) >= 1000))) { + powerstat_t powerstat; rig_get_powerstat(my_rig, &powerstat); rig_powerstat = powerstat; @@ -1295,6 +1300,8 @@ void *handle_socket(void *arg) { retcode = -RIG_EPOWER; } + + elapsed_ms(&powerstat_check_time, HAMLIB_ELAPSED_SET); } } else