From 1fdf727e42ded2911fc9c0c87fca508d2ac2b68b Mon Sep 17 00:00:00 2001
From: KJ5HST <62bk8cwjjs@privaterelay.appleid.com>
Date: Wed, 17 Dec 2025 20:34:24 -0600
Subject: [PATCH] FTX-1: Fix code review issues and add missing function
 handlers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Critical fixes:
- Fix RIT/XIT double-negation parsing bug (atoi already handles sign)
- Add ±9999 Hz range validation for RIT/XIT offsets

High priority fixes:
- Add RIG_FUNC_TONE/TSQL handlers using CT command modes
- Add RIG_FUNC_RIT/XIT handlers for clarifier enable/disable
- Add SPA-1 guard to prevent TUNE on Field Head (no tuner available)

Medium priority fixes:
- Fix VFO handling bug in squelch (was using "? 0 : 0")
- Add error checking for power restoration during head detection
---
 rigs/yaesu/ftx1.c                |  8 +++++-
 rigs/yaesu/ftx1/ftx1_clarifier.c | 28 +++++++++++-------
 rigs/yaesu/ftx1/ftx1_func.c      | 49 ++++++++++++++++++++++++++++++++
 rigs/yaesu/ftx1/ftx1_tx.c        | 10 ++-----
 rigs/yaesu/ftx1/ftx1_vfo.c       |  8 ++++++
 5 files changed, 84 insertions(+), 19 deletions(-)

diff --git a/rigs/yaesu/ftx1.c b/rigs/yaesu/ftx1.c
index 0fb3b9810..fe0593735 100644
--- a/rigs/yaesu/ftx1.c
+++ b/rigs/yaesu/ftx1.c
@@ -255,7 +255,13 @@ static int ftx1_probe_field_head_power(RIG *rig)
 
     /* Restore original power setting */
     SNPRINTF(priv->cmd_str, sizeof(priv->cmd_str), "%s", original_power);
-    newcat_set_cmd(rig);
+    ret = newcat_set_cmd(rig);
+
+    if (ret != RIG_OK)
+    {
+        rig_debug(RIG_DEBUG_WARN, "%s: failed to restore original power setting\n",
+                  __func__);
+    }
 
     if (power_accepted)
     {
diff --git a/rigs/yaesu/ftx1/ftx1_clarifier.c b/rigs/yaesu/ftx1/ftx1_clarifier.c
index 9100c0a41..8121d3411 100644
--- a/rigs/yaesu/ftx1/ftx1_clarifier.c
+++ b/rigs/yaesu/ftx1/ftx1_clarifier.c
@@ -102,12 +102,8 @@ int ftx1_get_rit(RIG *rig, vfo_t vfo, shortfreq_t *rit)
     /* Check if RX clarifier (RIT) is enabled */
     if (rdata->rx_clarifier == '1')
     {
+        /* atoi() handles the sign prefix correctly - no need to negate again */
         *rit = atoi(rdata->clarifier);
-        /* Handle sign - clarifier field includes +/- prefix */
-        if (rdata->clarifier[0] == '-')
-        {
-            *rit = -*rit;
-        }
     }
     else
     {
@@ -137,6 +133,14 @@ int ftx1_set_rit(RIG *rig, vfo_t vfo, shortfreq_t rit)
 
     rig_debug(RIG_DEBUG_VERBOSE, "%s called with rit=%ld\n", __func__, (long)rit);
 
+    /* Validate range: FTX-1 supports ±9999 Hz */
+    if (rit < -9999 || rit > 9999)
+    {
+        rig_debug(RIG_DEBUG_ERR, "%s: RIT offset %ld out of range (±9999 Hz)\n",
+                  __func__, (long)rit);
+        return -RIG_EINVAL;
+    }
+
     if (rit == 0)
     {
         /* Clear RIT - RC0; */
@@ -196,12 +200,8 @@ int ftx1_get_xit(RIG *rig, vfo_t vfo, shortfreq_t *xit)
     /* Check if TX clarifier (XIT) is enabled */
     if (rdata->tx_clarifier == '1')
     {
+        /* atoi() handles the sign prefix correctly - no need to negate again */
         *xit = atoi(rdata->clarifier);
-        /* Handle sign - clarifier field includes +/- prefix */
-        if (rdata->clarifier[0] == '-')
-        {
-            *xit = -*xit;
-        }
     }
     else
     {
@@ -231,6 +231,14 @@ int ftx1_set_xit(RIG *rig, vfo_t vfo, shortfreq_t xit)
 
     rig_debug(RIG_DEBUG_VERBOSE, "%s called with xit=%ld\n", __func__, (long)xit);
 
+    /* Validate range: FTX-1 supports ±9999 Hz */
+    if (xit < -9999 || xit > 9999)
+    {
+        rig_debug(RIG_DEBUG_ERR, "%s: XIT offset %ld out of range (±9999 Hz)\n",
+                  __func__, (long)xit);
+        return -RIG_EINVAL;
+    }
+
     if (xit == 0)
     {
         /* Clear XIT - TC0; */
diff --git a/rigs/yaesu/ftx1/ftx1_func.c b/rigs/yaesu/ftx1/ftx1_func.c
index 7592c2c4f..bc07f3be1 100644
--- a/rigs/yaesu/ftx1/ftx1_func.c
+++ b/rigs/yaesu/ftx1/ftx1_func.c
@@ -48,6 +48,7 @@
 #include "misc.h"
 #include "yaesu.h"
 #include "newcat.h"
+#include "../ftx1.h"
 
 /* Extern helpers from ftx1_filter.c */
 extern int ftx1_set_anf_helper(RIG *rig, vfo_t vfo, int status);
@@ -153,6 +154,14 @@ extern int ftx1_set_dcs_code(RIG *rig, vfo_t vfo, tone_t code);
 extern int ftx1_get_dcs_code(RIG *rig, vfo_t vfo, tone_t *code);
 extern int ftx1_set_dcs_sql(RIG *rig, vfo_t vfo, tone_t code);
 extern int ftx1_get_dcs_sql(RIG *rig, vfo_t vfo, tone_t *code);
+extern int ftx1_set_ctcss_mode(RIG *rig, tone_t mode);
+extern int ftx1_get_ctcss_mode(RIG *rig, tone_t *mode);
+
+/* Extern helpers from ftx1_clarifier.c */
+extern int ftx1_set_rit(RIG *rig, vfo_t vfo, shortfreq_t rit);
+extern int ftx1_get_rit(RIG *rig, vfo_t vfo, shortfreq_t *rit);
+extern int ftx1_set_xit(RIG *rig, vfo_t vfo, shortfreq_t xit);
+extern int ftx1_get_xit(RIG *rig, vfo_t vfo, shortfreq_t *xit);
 
 /* Extern helpers from ftx1_info.c */
 extern int ftx1_set_trn(RIG *rig, int trn);
@@ -194,6 +203,18 @@ int ftx1_set_func(RIG *rig, vfo_t vfo, setting_t func, int status)
             return ftx1_set_breakin(rig, status ? 1 : 0);
         case RIG_FUNC_FBKIN:
             return ftx1_set_breakin(rig, status ? 2 : 0);
+        case RIG_FUNC_TONE:
+            return ftx1_set_ctcss_mode(rig, status ? FTX1_CTCSS_MODE_ENC : FTX1_CTCSS_MODE_OFF);
+        case RIG_FUNC_TSQL:
+            return ftx1_set_ctcss_mode(rig, status ? FTX1_CTCSS_MODE_TSQ : FTX1_CTCSS_MODE_OFF);
+        case RIG_FUNC_RIT:
+            /* FTX-1: Setting RIT to 0 disables it; to enable, use set_rit with offset */
+            if (!status) return ftx1_set_rit(rig, vfo, 0);
+            return RIG_OK;  /* Enable is no-op; must use set_rit with offset value */
+        case RIG_FUNC_XIT:
+            /* FTX-1: Setting XIT to 0 disables it; to enable, use set_xit with offset */
+            if (!status) return ftx1_set_xit(rig, vfo, 0);
+            return RIG_OK;  /* Enable is no-op; must use set_xit with offset value */
         /* Note: Contour (CO command) not exposed as RIG_FUNC_CONTOUR doesn't exist in Hamlib */
         default:
             return newcat_set_func(rig, vfo, func, status);
@@ -238,6 +259,34 @@ int ftx1_get_func(RIG *rig, vfo_t vfo, setting_t func, int *status)
             ret = ftx1_get_breakin(rig, &mode);
             if (ret == RIG_OK) *status = (mode == 2) ? 1 : 0;
             return ret;
+        case RIG_FUNC_TONE:
+            {
+                tone_t ctcss_mode;
+                ret = ftx1_get_ctcss_mode(rig, &ctcss_mode);
+                if (ret == RIG_OK) *status = (ctcss_mode == FTX1_CTCSS_MODE_ENC) ? 1 : 0;
+                return ret;
+            }
+        case RIG_FUNC_TSQL:
+            {
+                tone_t ctcss_mode;
+                ret = ftx1_get_ctcss_mode(rig, &ctcss_mode);
+                if (ret == RIG_OK) *status = (ctcss_mode == FTX1_CTCSS_MODE_TSQ) ? 1 : 0;
+                return ret;
+            }
+        case RIG_FUNC_RIT:
+            {
+                shortfreq_t rit_offset;
+                ret = ftx1_get_rit(rig, vfo, &rit_offset);
+                if (ret == RIG_OK) *status = (rit_offset != 0) ? 1 : 0;
+                return ret;
+            }
+        case RIG_FUNC_XIT:
+            {
+                shortfreq_t xit_offset;
+                ret = ftx1_get_xit(rig, vfo, &xit_offset);
+                if (ret == RIG_OK) *status = (xit_offset != 0) ? 1 : 0;
+                return ret;
+            }
         /* Note: Contour (CO command) not exposed as RIG_FUNC_CONTOUR doesn't exist in Hamlib */
         default:
             return newcat_get_func(rig, vfo, func, status);
diff --git a/rigs/yaesu/ftx1/ftx1_tx.c b/rigs/yaesu/ftx1/ftx1_tx.c
index feeab91e1..d9613bdce 100644
--- a/rigs/yaesu/ftx1/ftx1_tx.c
+++ b/rigs/yaesu/ftx1/ftx1_tx.c
@@ -331,15 +331,9 @@ int ftx1_get_breakin(RIG *rig, int *mode)
 int ftx1_set_squelch(RIG *rig, vfo_t vfo, float val)
 {
     struct newcat_priv_data *priv = STATE(rig)->priv;
-    int p1 = (vfo == RIG_VFO_SUB || vfo == RIG_VFO_B || vfo == RIG_VFO_CURR) ? 0 : 0;
+    int p1 = FTX1_VFO_TO_P1(vfo);
     int level = (int)(val * 100);
 
-    /* Handle VFO_CURR as MAIN */
-    if (vfo == RIG_VFO_SUB || vfo == RIG_VFO_B)
-    {
-        p1 = 1;
-    }
-
     if (level < 0) level = 0;
     if (level > 100) level = 100;
 
@@ -356,7 +350,7 @@ int ftx1_get_squelch(RIG *rig, vfo_t vfo, float *val)
     struct newcat_priv_data *priv = STATE(rig)->priv;
     int ret, p1, level;
 
-    p1 = (vfo == RIG_VFO_SUB || vfo == RIG_VFO_B) ? 1 : 0;
+    p1 = FTX1_VFO_TO_P1(vfo);
 
     rig_debug(RIG_DEBUG_VERBOSE, "%s: vfo=%s p1=%d\n", __func__,
               rig_strvfo(vfo), p1);
diff --git a/rigs/yaesu/ftx1/ftx1_vfo.c b/rigs/yaesu/ftx1/ftx1_vfo.c
index cb2d60e89..f01993a30 100644
--- a/rigs/yaesu/ftx1/ftx1_vfo.c
+++ b/rigs/yaesu/ftx1/ftx1_vfo.c
@@ -20,6 +20,7 @@
 #include "misc.h"
 #include "yaesu.h"
 #include "newcat.h"
+#include "../ftx1.h"
 
 /*
  * ftx1_set_vfo
@@ -233,6 +234,13 @@ int ftx1_vfo_op(RIG *rig, vfo_t vfo, vfo_op_t op)
         /* AC110: Tuner start (turns on tuner and starts tune) */
         /* Format: AC P1 P2 P3; P1=1 (on), P2=1 (start tune), P3=0 (MAIN) */
         /* Note: This causes transmission! */
+        /* Tuner is only available on SPA-1/Optima head */
+        if (!ftx1_has_spa1())
+        {
+            rig_debug(RIG_DEBUG_WARN, "%s: TUNE not available on Field Head\n",
+                      __func__);
+            return -RIG_ENAVAIL;
+        }
         SNPRINTF(priv->cmd_str, sizeof(priv->cmd_str), "AC110;");
         break;