Fix power on/off bugs: rig_open() should succeed even if rig is powered off, auto power-on should not be automatically enabled (prevents turning rig off), fix Kenwood and Yaesu power on/off sequences for all rigs

rigs/kenwood/kenwood.c

@@ -321,15 +321,6 @@ transaction_write:
         /* flush anything in the read buffer before command is sent */
         rig_flush(&rs->rigport);
 
-        // PS command may need to wake up serial port
-        if (priv->ps_cmd_wakeup_data)
-        {
-            if (strncmp(cmd, "PS", 2) == 0)
-            {
-                write_block(&rs->rigport, (unsigned char *) ";;;;", 4);
-            }
-        }
-
         retval = write_block(&rs->rigport, (unsigned char *) cmd, len);
 
         free(cmd);
@@ -544,19 +535,8 @@ transaction_read:
      */
     if (datasize)
     {
-        char *ps_cmd;
-
-        if (priv->ps_cmd_wakeup_data)
-        {
-            ps_cmd = ";;;;PS";
-        }
-        else
-        {
-            ps_cmd = "PS";
-        }
-
         // we ignore the special PS command
-        if (cmdstr && strcmp(cmdstr, ps_cmd) != 0 && (buffer[0] != cmdstr[0]
+        if (cmdstr && strcmp(cmdstr, "PS") != 0 && (buffer[0] != cmdstr[0]
                 || (cmdstr[1] && buffer[1] != cmdstr[1])))
         {
             /*
@@ -5051,6 +5031,7 @@ int kenwood_get_trn(RIG *rig, int *trn)
 int kenwood_set_powerstat(RIG *rig, powerstat_t status)
 {
     int retval;
+    struct rig_state *state = &rig->state;
     struct kenwood_priv_data *priv = rig->state.priv;
 
     if ((priv->is_k3 || priv->is_k3s) && status == RIG_POWER_ON)
@@ -5065,6 +5046,14 @@ int kenwood_set_powerstat(RIG *rig, powerstat_t status)
 
     rig_debug(RIG_DEBUG_VERBOSE, "%s called status=%d\n", __func__, status);
 
+    if (status == RIG_POWER_ON)
+    {
+        // When powering on a Kenwood rig needs dummy bytes to wake it up,
+        // then wait at least 200ms and within 2 seconds issue the power-on command again
+        write_block(&state->rigport, (unsigned char *) "PS1;", 4);
+        hl_usleep(500000);
+    }
+
     rig->state.rigport.retry = 0;
 
     retval = kenwood_transaction(rig,
@@ -5108,7 +5097,8 @@ int kenwood_set_powerstat(RIG *rig, powerstat_t status)
 int kenwood_get_powerstat(RIG *rig, powerstat_t *status)
 {
     char pwrbuf[6];
-    int retval;
+    int result;
+    struct rig_state *state = &rig->state;
     struct kenwood_priv_data *priv = rig->state.priv;
 
     ENTERFUNC;
@@ -5124,22 +5114,61 @@ int kenwood_get_powerstat(RIG *rig, powerstat_t *status)
         RETURNFUNC(-RIG_EINVAL);
     }
 
-    char *ps_cmd;
+    // The first PS command has two purposes:
+    // 1. to detect that the rig is turned on/off when it responds with PS1/PS0 immediately
+    // 2. to act as dummy wake-up data for a rig that is turned off
 
-    if (priv->ps_cmd_wakeup_data)
+    // Timeout needs to be set temporarily to a low value,
+    // so that the second command can be sent in 2 seconds, which is what Kenwood rigs expect.
+    short retry_save;
+    short timeout_retry_save;
+    int timeout_save;
+
+    retry_save = state->rigport.retry;
+    timeout_retry_save = state->rigport.timeout_retry;
+    timeout_save = state->rigport.timeout;
+
+    state->rigport.retry = 0;
+    state->rigport.timeout_retry = 0;
+    state->rigport.timeout = 500;
+
+    result = kenwood_safe_transaction(rig, "PS", pwrbuf, 6, 3);
+
+    state->rigport.retry = retry_save;
+    state->rigport.timeout_retry = timeout_retry_save;
+    state->rigport.timeout = timeout_save;
+
+    // Rig may respond here already
+    if (result == RIG_OK)
     {
-        ps_cmd = ";;;;PS";
+        char ps = pwrbuf[2];
-    }
+
-    else
+        switch (ps)
-    {
+        {
-        ps_cmd = "PS";
+        case '1':
+            *status = RIG_POWER_ON;
+            RETURNFUNC(RIG_OK);
+
+        case '0':
+            *status = RIG_POWER_OFF;
+            RETURNFUNC(RIG_OK);
+
+        default:
+            // fall through to retry command
+            break;
+        }
     }
 
-    retval = kenwood_safe_transaction(rig, ps_cmd, pwrbuf, 6, 3);
+    // Kenwood rigs in powered-off state require the PS command to be sent
+    // after waiting for at least 200ms and within 2 seconds after dummy data
+    hl_usleep(500000);
+    // Discard any unsolicited data
+    rig_flush(&rig->state.rigport);
 
-    if (retval != RIG_OK)
+    result = kenwood_safe_transaction(rig, "PS", pwrbuf, 6, 3);
+    if (result != RIG_OK)
     {
-        RETURNFUNC(retval);
+        RETURNFUNC(result);
     }
 
     *status = pwrbuf[2] == '0' ? RIG_POWER_OFF : RIG_POWER_ON;

rigs/kenwood/kenwood.h

@@ -175,7 +175,6 @@ struct kenwood_priv_data
     rmode_t modeB;
     int datamodeA; // datamode status from get_mode or set_mode
     int datamodeB; // datamode status from get_mode or set_mode
-    int ps_cmd_wakeup_data; // PS command requires wakeup characters (;)
     int question_mark_response_means_rejected; /* the question mark response has multiple meanings */
     int save_k2_ext_lvl; // so we can restore to original
     int save_k3_ext_lvl; // so we can restore to original -- for future use if needed

rigs/kenwood/ts480.c

@@ -1211,7 +1211,6 @@ int ts480_init(RIG *rig)
     priv->ag_format = 2;
     priv->micgain_min = 0;
     priv->micgain_max = 100;
-    priv->ps_cmd_wakeup_data = 1;
 
     RETURNFUNC(RIG_OK);
 }

rigs/yaesu/newcat.c

@@ -3538,7 +3538,6 @@ int newcat_set_powerstat(RIG *rig, powerstat_t status)
     int retval;
     int i = 0;
     int retry_save;
-    char ps;
 
     ENTERFUNC;
 
@@ -3554,9 +3553,8 @@ int newcat_set_powerstat(RIG *rig, powerstat_t status)
     switch (status)
     {
     case RIG_POWER_ON:
-        ps = '1';
+        // When powering on a Yaesu rig needs dummy bytes to wake it up,
-        // when powering on need a dummy byte to wake it up
+        // then wait from 1 to 2 seconds and issue the power-on command again
-        // then sleep  from 1 to 2 seconds so we'll do 1.5 secs
         write_block(&state->rigport, (unsigned char *) "PS1;", 4);
         hl_usleep(1200000);
         break;
@@ -3567,10 +3565,11 @@ int newcat_set_powerstat(RIG *rig, powerstat_t status)
         RETURNFUNC(retval);
 
     default:
-        RETURNFUNC(-RIG_ENAVAIL);
+        RETURNFUNC(-RIG_EINVAL);
     }
 
-    SNPRINTF(priv->cmd_str, sizeof(priv->cmd_str), "PS%c%c", ps, cat_term);
+    // Power on may require a second command
+    SNPRINTF(priv->cmd_str, sizeof(priv->cmd_str), "PS1%c", cat_term);
 
     retval = write_block(&state->rigport, (unsigned char *) priv->cmd_str,
                          strlen(priv->cmd_str));
@@ -3620,9 +3619,9 @@ int newcat_set_powerstat(RIG *rig, powerstat_t status)
  */
 int newcat_get_powerstat(RIG *rig, powerstat_t *status)
 {
-    struct newcat_priv_data *priv = (struct newcat_priv_data *)rig->state.priv;
+    struct rig_state *state = (struct rig_state *) &rig->state;
-    //struct rig_state *state = &rig->state;
+    struct newcat_priv_data *priv = (struct newcat_priv_data *) rig->state.priv;
-    int err;
+    int result;
     char ps;
     char command[] = "PS";
 
@@ -3635,25 +3634,61 @@ int newcat_get_powerstat(RIG *rig, powerstat_t *status)
         RETURNFUNC(-RIG_ENAVAIL);
     }
 
-    // when not powered on need a dummy byte to wake it up
+    // The first PS command has two purposes:
-    // then sleep  from 1 to 2 seconds so we'll do 1.5 secs
+    // 1. to detect that the rig is turned on when it responds with PS1 immediately
-//    write_block(&state->rigport, (unsigned char *) "PS;", 3);
+    // 2. to act as dummy wake-up data for a rig that is turned off
     SNPRINTF(priv->cmd_str, sizeof(priv->cmd_str), "%s%c", command, cat_term);
-    newcat_get_cmd(rig); // don't care about the return
-    if (priv->ret_data[2] == '1')
-    {
-        *status = 1;
-        RETURNFUNC(RIG_OK);
 
+    // Timeout needs to be set temporarily to a low value,
+    // so that the second command can be sent in 2 seconds, which is what Yaesu rigs expect.
+    short retry_save;
+    short timeout_retry_save;
+    int timeout_save;
+
+    retry_save = state->rigport.retry;
+    timeout_retry_save = state->rigport.timeout_retry;
+    timeout_save = state->rigport.timeout;
+
+    state->rigport.retry = 0;
+    state->rigport.timeout_retry = 0;
+    state->rigport.timeout = 500;
+
+    result = newcat_get_cmd(rig);
+
+    state->rigport.retry = retry_save;
+    state->rigport.timeout_retry = timeout_retry_save;
+    state->rigport.timeout = timeout_save;
+
+    // Rig may respond here already
+    if (result == RIG_OK)
+    {
+        ps = priv->ret_data[2];
+
+        switch (ps)
+        {
+        case '1':
+            *status = RIG_POWER_ON;
+            RETURNFUNC(RIG_OK);
+
+        case '0':
+            *status = RIG_POWER_OFF;
+            RETURNFUNC(RIG_OK);
+
+        default:
+            // fall through to retry command
+            break;
+        }
     }
-    hl_usleep(1200000); // then we must be waking up
-    rig_flush(&rig->state.rigport);  /* discard any unsolicited data */
 
+    // Yeasu rigs in powered-off state require the PS command to be sent between 1 and 2 seconds after dummy data
+    hl_usleep(1100000);
+    // Discard any unsolicited data
+    rig_flush(&rig->state.rigport);
 
-    /* Get Power status */
+    result = newcat_get_cmd(rig);
-    if (RIG_OK != (err = newcat_get_cmd(rig)))
+    if (result != RIG_OK)
     {
-        RETURNFUNC(err);
+        RETURNFUNC(result);
     }
 
     ps = priv->ret_data[2];
@@ -3669,7 +3704,7 @@ int newcat_get_powerstat(RIG *rig, powerstat_t *status)
         break;
 
     default:
-        RETURNFUNC(-RIG_ENAVAIL);
+        RETURNFUNC(-RIG_EPROTO);
     }
 
     RETURNFUNC(RIG_OK);

src/rig.c

@@ -1304,14 +1304,13 @@ int HAMLIB_API rig_open(RIG *rig)
             powerstat_t powerflag;
             status = rig_get_powerstat(rig, &powerflag);
 
-            if (status == RIG_OK && powerflag == RIG_POWER_OFF
+            if (status == RIG_OK && (powerflag == RIG_POWER_OFF || powerflag == RIG_POWER_STANDBY)
                     && rig->state.auto_power_on == 0)
             {
+                // rig_open() should succeed even if the rig is powered off, so simply log power status
                 rig_debug(RIG_DEBUG_ERR,
                           "%s: rig power is off, use --set-conf=auto_power_on=1 if power on is wanted\n",
                           __func__);
-
-                RETURNFUNC2(-RIG_EPOWER);
             }
 
             // don't need auto_power_on if power is already on
@@ -1319,14 +1318,10 @@ int HAMLIB_API rig_open(RIG *rig)
 
             if (status == -RIG_ETIMEOUT)
             {
-                rig_debug(RIG_DEBUG_ERR, "%s: Some rigs cannot get_powerstat while off\n",
+                // rig_open() should succeed even if get_powerstat() fails,
-                          __func__);
+                // as many rigs cannot get power status while powered off
+                rig_debug(RIG_DEBUG_ERR, "%s: Some rigs cannot get_powerstat while off\n", __func__);
                 rig_debug(RIG_DEBUG_ERR, "%s: Known rigs: K3, K3S\n", __func__);
-                rig_debug(RIG_DEBUG_ERR, "%s: Rigs that should but don't work: TS480\n",
-                          __func__);
-                // A TS-480 user was showing ;;;;PS; not working so we'll just show the error message for now
-                // https://github.com/Hamlib/Hamlib/issues/1226
-                //RETURNFUNC2 (-RIG_EPOWER);
             }
         }
 
@@ -6154,8 +6149,8 @@ int HAMLIB_API rig_set_powerstat(RIG *rig, powerstat_t status)
 
     HAMLIB_TRACE;
     retcode = rig->caps->set_powerstat(rig, status);
-    rig_flush(&rig->state.rigport); // if anything is queued up flush it
+    // if anything is queued up flush it
-    rig->state.auto_power_on = 1; // ensure we auto power on in the future
+    rig_flush_force(&rig->state.rigport, 1);
     RETURNFUNC(retcode);
 }
 
@@ -6201,17 +6196,12 @@ int HAMLIB_API rig_get_powerstat(RIG *rig, powerstat_t *status)
     HAMLIB_TRACE;
     retcode = rig->caps->get_powerstat(rig, status);
 
-    if (retcode == RIG_EIO)
+    if (retcode != RIG_OK)
     {
-        rig_debug(RIG_DEBUG_ERR, "%s: hard error, reopening rig\n", __func__);
+        // if failed, assume power is on
-        rig_close(rig);
+        *status = RIG_POWER_ON;
-        rig_open(rig);
     }
 
-    if (retcode != RIG_OK) { *status = RIG_POWER_ON; } // if failed assume power is on
-
-    if (*status == RIG_POWER_OFF && rig->state.auto_power_on) { rig->caps->set_powerstat(rig, RIG_POWER_ON); }
-
     RETURNFUNC(retcode);
 }
 

tests/rigctl_parse.c

@@ -1732,8 +1732,7 @@ readline_repeat:
     else
     {
         // Allow only certain commands when the rig is powered off
-        if (retcode == RIG_OK && (rig_powerstat == RIG_POWER_OFF
+        if ((rig_powerstat == RIG_POWER_OFF || rig_powerstat == RIG_POWER_STANDBY)
-                                  || rig_powerstat == RIG_POWER_STANDBY)
                 && cmd_entry->cmd != '1' // dump_caps
                 && cmd_entry->cmd != '3' // dump_conf
                 && cmd_entry->cmd != 0x8f // dump_state
@@ -4722,8 +4721,11 @@ declare_proto_rig(set_powerstat)
     CHKSCN1ARG(sscanf(arg1, "%d", &stat));
 
     retval = rig_set_powerstat(rig, (powerstat_t) stat);
-    rig->state.powerstat = stat;
+    if (retval == RIG_OK)
-    rig_powerstat = stat; // update our global so others can see powerstat
+    {
+        rig->state.powerstat = stat;
+        rig_powerstat = stat; // update our global so others can see powerstat
+    }
     fflush(fin);
     RETURNFUNC2(retval);
 }
@@ -4751,6 +4753,7 @@ declare_proto_rig(get_powerstat)
 
     fprintf(fout, "%d\n", stat);
     rig->state.powerstat = stat;
+    rig_powerstat = stat; // update our global so others can see powerstat
 
     RETURNFUNC2(status);
 }

tests/rigctld.c

@@ -1285,7 +1285,10 @@ void *handle_socket(void *arg)
                 rig_get_powerstat(my_rig, &powerstat);
                 rig_powerstat = powerstat;
 
-                if (powerstat == RIG_POWER_OFF) { retcode = -RIG_EPOWER; }
+                if (powerstat == RIG_POWER_OFF || powerstat == RIG_POWER_STANDBY)
+                {
+                    retcode = -RIG_EPOWER;
+                }
             }
         }
         else