Fix issues found during testing

rigs/icom/ic7300.c

@@ -2185,34 +2185,19 @@ int ic9700_get_clock(RIG *rig, int *year, int *month, int *day, int *hour,
 
 int ic9700_set_vfo(RIG *rig, vfo_t vfo)
 {
-    ENTERFUNC;
     unsigned char ackbuf[MAXFRAMELEN];
-    int ack_len = sizeof(ackbuf), retval = -RIG_EINTERNAL;
+    int ack_len = sizeof(ackbuf);
+    int retval;
+
+    ENTERFUNC;
 
     rig_debug(RIG_DEBUG_VERBOSE, "%s: vfo=%s\n", __func__, rig_strvfo(vfo));
+
     if (vfo == RIG_VFO_A)
     {
-        retval = icom_transaction(rig, 0x07, 0x00, NULL, 0, ackbuf, &ack_len);
+        retval = icom_transaction(rig, C_SET_VFO, S_VFOA, NULL, 0, ackbuf, &ack_len);
     }
     else if (vfo == RIG_VFO_B)
-    {
-        retval = icom_transaction(rig, 0x07, 0x01, NULL, 0, ackbuf, &ack_len);
-    }
-    else if (vfo == RIG_VFO_MAIN || vfo == RIG_VFO_MAIN_A || vfo == RIG_VFO_MAIN_B)
-    {
-        retval = icom_transaction(rig, 0x07, 0xd0, NULL, 0, ackbuf, &ack_len);
-        if (retval != RIG_OK)
-        {
-            rig_debug(RIG_DEBUG_ERR, "%s: %s\n", __func__, rigerror(retval));
-            return -retval;
-        }
-        if (vfo == RIG_VFO_MAIN_A || vfo == RIG_VFO_MAIN_B)
-        {
-            int subcmd =  vfo == RIG_VFO_MAIN_A ? 0x00: 0x01;
-            retval = icom_transaction(rig, 0x07, subcmd, NULL, 0, ackbuf, &ack_len);
-        }
-    }
-    else if (vfo == RIG_VFO_SUB || vfo == RIG_VFO_SUB_A || vfo == RIG_VFO_SUB_B)
     {
         if (rig->state.cache.satmode)
         {
@@ -2220,25 +2205,66 @@ int ic9700_set_vfo(RIG *rig, vfo_t vfo)
             // we return RIG_OK anyways as this should just be a bad request
             return RIG_OK;
         }
-        // first switch to sub
+
-        retval = icom_transaction(rig, 0x07, 0xd1, NULL, 0, ackbuf, &ack_len);
+        retval = icom_transaction(rig, C_SET_VFO, S_VFOB, NULL, 0, ackbuf, &ack_len);
+    }
+    else if (vfo == RIG_VFO_MAIN || vfo == RIG_VFO_MAIN_A || vfo == RIG_VFO_MAIN_B)
+    {
+        // First switch to Main receiver
+        retval = icom_transaction(rig, C_SET_VFO, S_MAIN, NULL, 0, ackbuf, &ack_len);
         if (retval != RIG_OK)
         {
             rig_debug(RIG_DEBUG_ERR, "%s: %s\n", __func__, rigerror(retval));
-            return -retval;
+            return retval;
         }
+
+        if (rig->state.cache.satmode && vfo == RIG_VFO_MAIN_B)
+        {
+            rig_debug(RIG_DEBUG_WARN, "%s: cannot switch to VFOB when in satmode\n", __func__);
+            // we return RIG_OK anyways as this should just be a bad request
+            return RIG_OK;
+        }
+
+        if (vfo == RIG_VFO_MAIN_A || vfo == RIG_VFO_MAIN_B)
+        {
+            int subcmd = vfo == RIG_VFO_MAIN_A ? S_VFOA : S_VFOB;
+            retval = icom_transaction(rig, C_SET_VFO, subcmd, NULL, 0, ackbuf, &ack_len);
+        }
+    }
+    else if (vfo == RIG_VFO_SUB || vfo == RIG_VFO_SUB_A || vfo == RIG_VFO_SUB_B)
+    {
+        // First switch to Sub receiver
+        retval = icom_transaction(rig, C_SET_VFO, S_SUB, NULL, 0, ackbuf, &ack_len);
+        if (retval != RIG_OK)
+        {
+            rig_debug(RIG_DEBUG_ERR, "%s: %s\n", __func__, rigerror(retval));
+            return retval;
+        }
+
+        if (rig->state.cache.satmode && vfo == RIG_VFO_SUB_B)
+        {
+            rig_debug(RIG_DEBUG_WARN, "%s: cannot switch to VFOB when in satmode\n", __func__);
+            // we return RIG_OK anyways as this should just be a bad request
+            return RIG_OK;
+        }
+
         if (vfo == RIG_VFO_SUB_A || vfo == RIG_VFO_SUB_B)
         {
             HAMLIB_TRACE;
-            int subcmd =  vfo == RIG_VFO_SUB_A ? 0x00: 0x01;
+            int subcmd = vfo == RIG_VFO_SUB_A ? S_VFOA : S_VFOB;
-            retval = icom_transaction(rig, 0x07, subcmd, NULL, 0, ackbuf, &ack_len);
+            retval = icom_transaction(rig, C_SET_VFO, subcmd, NULL, 0, ackbuf, &ack_len);
         }
     }
+    else
+    {
+        // Unsupported VFO
+        RETURNFUNC(-RIG_EINVAL);
+    }
 
     if (retval != RIG_OK)
     {
         rig_debug(RIG_DEBUG_ERR, "%s: %s\n", __func__, rigerror(retval));
-        return -retval;
+        return retval;
     }
 
     RETURNFUNC(retval);

rigs/icom/icom.c

@@ -1386,6 +1386,7 @@ int icom_set_freq(RIG *rig, vfo_t vfo, freq_t freq)
     struct icom_priv_data *priv = (struct icom_priv_data *) rs->priv;
     unsigned char freqbuf[MAXFRAMELEN], ackbuf[MAXFRAMELEN];
     int freq_len, ack_len = sizeof(ackbuf), retval;
+    int check_ack = 0;
     int cmd, subcmd;
     freq_t curr_freq;
 
@@ -1454,6 +1455,8 @@ int icom_set_freq(RIG *rig, vfo_t vfo, freq_t freq)
             retval = icom_transaction(rig, cmd, subcmd, freqbuf, freq_len, ackbuf,
                                       &ack_len);
         }
+
+        check_ack = 1;
     }
 
     // pause for transceive message and we'll flush it
@@ -1499,6 +1502,8 @@ int icom_set_freq(RIG *rig, vfo_t vfo, freq_t freq)
 
                     RETURNFUNC2(retval);
                 }
+
+                check_ack = 1;
             }
         }
 
@@ -1510,7 +1515,7 @@ int icom_set_freq(RIG *rig, vfo_t vfo, freq_t freq)
         }
     }
 
-    if ((retval = icom_check_ack(ack_len, ackbuf)) != RIG_OK)
+    if (check_ack && (retval = icom_check_ack(ack_len, ackbuf)) != RIG_OK)
     {
         RETURNFUNC2(retval);
     }

src/rig.c

@@ -21,17 +21,6 @@
  *
  */
 
-/**
- * TEST: Remove use of private state cache for VFOs/frequencies/modes in Icom backends and migrate to using the rig_state cache
- * TEST: Ignore VFO targeting in all set/get split freq/mode commands and use the TX VFO set by \set_split_vfo command instead
- * TEST: If split is not enabled, make \get_split_freq return 0 Hz frequency and get_split_mode return NONE mode with 0 Hz filter to indicate split is OFF
- * TEST: set_split_freq and set_split_mode turn split ON if it is not enabled yet
- * TODO: Make sure Icom set_freq/get_freq/set_mode/get_mode + set_split_freq/get_split_freq/set_split_mode/get_split_mode all use the 0x25 and 0x26 commands in a consistent way to avoid VFO swapping (on both the selected/unselected type rigs and Main/Sub VFO rigs)
- *
- * TODO: Test targeted set_freq/get_freq/set_mode/get_mode + split commands
- * TODO: Test latest WSJT-X in split mode
- */
-
 /**
  * \addtogroup rig
  * @{