radio_io: say when radio_device is the wrong kind of port for the rig (#179)

radio_io/radio_io.c

@@ -34,6 +34,7 @@
 #endif
 
 #include "radio_io.h"
+#include "radio_port.h"
 #include "../common/hermes_log.h"
 
 #define RADIO_LOG_TAG "radio-io"
@@ -45,8 +46,35 @@
 #endif
 
 #ifdef HAVE_HAMLIB
-/* Configure the serial device path and speed through Hamlib's stable public
- * conf API (rig_token_lookup + rig_set_conf) rather than writing the internal
+/* Which sort of port does this rig speak?  Hamlib knows; the user should not
+ * have to guess.  See radio_port.h for why this matters (issue #179).
+ *
+ * Read through rig_get_caps_int() rather than rig->caps->port_type for the same
+ * reason the conf tokens are preferred below: it is an accessor keyed on the
+ * model number, so it does not depend on the layout of a struct that Hamlib 5
+ * is progressively closing off. */
+static radio_port_kind_t radio_io_port_kind(rig_model_t model)
+{
+    switch ((rig_port_t)rig_get_caps_int(model, RIG_CAPS_PORT_TYPE))
+    {
+    case RIG_PORT_SERIAL:
+        return RADIO_PORT_KIND_SERIAL;
+    case RIG_PORT_NETWORK:
+    case RIG_PORT_UDP_NETWORK:
+        return RADIO_PORT_KIND_NETWORK;
+    default:
+        return RADIO_PORT_KIND_OTHER;
+    }
+}
+
+static const char *radio_io_model_name(rig_model_t model)
+{
+    const char *name = rig_get_caps_cptr(model, RIG_CAPS_MODEL_NAME_CPTR);
+    return name ? name : "this rig";
+}
+
+/* Configure the device path and speed through Hamlib's stable public conf API
+ * (rig_token_lookup + rig_set_conf) rather than writing the internal
  * rig->state.rigport struct directly.
  *
  * Since Hamlib 4.6 the port struct is reached via the HAMLIB_RIGPORT macro,
@@ -55,7 +83,8 @@
  * rig_data_pointer undefined at link time), so depending on it is fragile.  The
  * conf tokens "rig_pathname" and "serial_speed" are stable across 4.6/4.7/5.x
  * and must be set before rig_open(). */
-static void radio_io_apply_serial_conf(RIG *radio, const char *device_path,
+static void radio_io_apply_serial_conf(RIG *radio, rig_model_t model,
+                                       const char *device_path,
                                        int serial_speed)
 {
     int rc;
@@ -70,6 +99,18 @@ static void radio_io_apply_serial_conf(RIG *radio, const char *device_path,
 
     if (serial_speed > 0)
     {
+        /* Only serial rigs have a baud rate.  Pushing serial_speed at a network
+         * rig makes Hamlib reject a token it does not own, and the resulting
+         * "rig_set_conf(serial_speed) failed: -1" reads like a fault when it is
+         * really just a setting that does not apply here. */
+        if (radio_io_port_kind(model) != RADIO_PORT_KIND_SERIAL)
+        {
+            HLOGI(RADIO_LOG_TAG,
+                  "radio_serial_speed=%d ignored: %s is not a serial rig",
+                  serial_speed, radio_io_model_name(model));
+            return;
+        }
+
         char rate[16];
         snprintf(rate, sizeof(rate), "%d", serial_speed);
         rc = rig_set_conf(radio, rig_token_lookup(radio, "serial_speed"), rate);
@@ -80,6 +121,19 @@ static void radio_io_apply_serial_conf(RIG *radio, const char *device_path,
                   serial_speed);
     }
 }
+
+/* Say plainly when radio_device does not match the kind of port the rig uses.
+ * Advisory only -- Hamlib still gets to try, because a shape heuristic must
+ * never veto a configuration that would have worked. */
+static void radio_io_warn_port_mismatch(rig_model_t model, const char *device_path)
+{
+    radio_port_verdict_t v = radio_port_check(radio_io_port_kind(model), device_path);
+    const char *advice = radio_port_advice(v);
+
+    if (advice)
+        HLOGW(RADIO_LOG_TAG, "radio_device='%s' looks wrong for %s -- %s",
+              device_path, radio_io_model_name(model), advice);
+}
 #endif
 
 /* Global mutex — protects radio state (g_radio_type, radio pointer,
@@ -186,15 +240,30 @@ int radio_io_init(int radio_type, const char *device_path, int hamlib_log_level,
         return -1;
     }
 
-    radio_io_apply_serial_conf(radio, device_path, serial_speed);
+    radio_io_apply_serial_conf(radio, radio_type, device_path, serial_speed);
+    radio_io_warn_port_mismatch(radio_type, device_path);
 
     HLOGD(RADIO_LOG_TAG, "Calling rig_open(device=%s)",
           device_path && device_path[0] ? device_path : "(default)");
     int ret = rig_open(radio);
     if (ret != RIG_OK)
     {
-        HLOGE(RADIO_LOG_TAG, "rig_open: error = %s %s",
-              device_path ? device_path : "(default)", rigerror(ret));
+        /* rigerror() returns Hamlib's debug ring buffer appended to the message,
+         * so at any log level above 0 the actual reason is buried in backend
+         * chatter.  rigerror2() is the plain string; keep the numeric code too,
+         * since that is what is greppable in a bug report. */
+        HLOGE(RADIO_LOG_TAG, "rig_open(%s) failed: %s (%d)",
+              device_path && device_path[0] ? device_path : "(default)",
+              rigerror2(ret), ret);
+
+        /* Repeat the advice next to the failure: the pre-flight warning above
+         * is easily lost in Hamlib's own debug output, and this line is the one
+         * the user will paste into a bug report. */
+        const char *advice =
+            radio_port_advice(radio_port_check(radio_io_port_kind(radio_type),
+                                               device_path));
+        if (advice)
+            HLOGE(RADIO_LOG_TAG, "%s", advice);
         rig_cleanup(radio);
         radio = NULL;
         g_radio_type = RADIO_TYPE_NONE;
@@ -229,17 +298,8 @@ int radio_io_init(int radio_type, const char *device_path, int hamlib_log_level,
      * member directly. Prefer the accessor so this survives the struct being
      * hidden. The hamlib version string goes on the same line because the field
      * only means anything once you know which hamlib produced it. */
-    {
-#if defined(HAMLIB_STATE)
-        const struct rig_state *rs = HAMLIB_STATE(radio);
-#elif defined(STATE)
-        const struct rig_state *rs = STATE(radio);
-#else
-        const struct rig_state *rs = &radio->state;
-#endif
-        HLOGI(RADIO_LOG_TAG, "hamlib runtime: %s",
-              hamlib_version2 ? hamlib_version2 : "version unknown");
-    }
+    HLOGI(RADIO_LOG_TAG, "hamlib runtime: %s",
+          hamlib_version2 ? hamlib_version2 : "version unknown");
     pthread_mutex_unlock(&g_radio_mutex);
     return 0;
 #else

radio_io/radio_port.h

@@ -0,0 +1,119 @@
+/* Does `radio_device` match the kind of port the chosen rig actually speaks?
+ *
+ * Copyright (C) 2026 Rhizomatica
+ *
+ * This is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3, or (at your option)
+ * any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * ---------------------------------------------------------------------------
+ *
+ * Not every rig Hamlib supports is reached over a serial cable.  A FlexRadio
+ * (SmartSDR, model 23005), rigctld (2), FLRig (4) and friends are network rigs:
+ * their `rig_pathname` is an address, and Hamlib appends its own default port
+ * if none is given.  Point one of those at a COM port and Hamlib dutifully
+ * builds "COM4:4992", tries to resolve it as a host, and fails with the rather
+ * unhelpful "Invalid parameter" -- reported as issue #179.
+ *
+ * The rig itself knows which it is (`caps->port_type`), so we can say so before
+ * the user has to guess.  These checks only ever produce a diagnostic: the
+ * device string is matched by shape, and a heuristic must never be allowed to
+ * refuse a configuration that would in fact have worked.  Hamlib still gets to
+ * make the final call.
+ *
+ * Kept free of Hamlib types so the shapes can be unit-tested on any host,
+ * including builds without Hamlib at all.
+ */
+
+#ifndef RADIO_PORT_H
+#define RADIO_PORT_H
+
+#include <stddef.h>
+
+typedef enum
+{
+    RADIO_PORT_KIND_OTHER = 0,  /* dummy, USB, parallel, ... -- no opinion */
+    RADIO_PORT_KIND_SERIAL,
+    RADIO_PORT_KIND_NETWORK,
+} radio_port_kind_t;
+
+typedef enum
+{
+    RADIO_PORT_OK = 0,
+    RADIO_PORT_WANTS_NETWORK,   /* network rig, but radio_device names a tty  */
+    RADIO_PORT_WANTS_SERIAL,    /* serial rig, but radio_device looks like an
+                                   address */
+} radio_port_verdict_t;
+
+/* "COM4", "com12", "\\.\COM12", "/dev/ttyUSB0", "/dev/cu.usbserial-1234" */
+static inline int radio_port_path_is_serial(const char *path)
+{
+    if (!path || !path[0])
+        return 0;
+
+    if (path[0] == '/')                       /* /dev/... (POSIX)            */
+        return 1;
+
+    if (path[0] == '\\')                      /* \\.\COMnn (Windows, >COM9)  */
+        return 1;
+
+    if ((path[0] == 'C' || path[0] == 'c') &&
+        (path[1] == 'O' || path[1] == 'o') &&
+        (path[2] == 'M' || path[2] == 'm') &&
+        path[3] >= '0' && path[3] <= '9')
+    {
+        for (const char *p = path + 4; *p; p++)
+            if (*p < '0' || *p > '9')
+                return 0;                     /* "COM4x" -- not a COM port   */
+        return 1;
+    }
+
+    return 0;
+}
+
+/* Judge a configured device string against the rig's port kind.  An empty
+ * device is always OK: Hamlib has its own defaults and the user may well be
+ * relying on them. */
+static inline radio_port_verdict_t radio_port_check(radio_port_kind_t kind,
+                                                    const char *path)
+{
+    if (!path || !path[0])
+        return RADIO_PORT_OK;
+
+    int looks_serial = radio_port_path_is_serial(path);
+
+    if (kind == RADIO_PORT_KIND_NETWORK && looks_serial)
+        return RADIO_PORT_WANTS_NETWORK;
+
+    if (kind == RADIO_PORT_KIND_SERIAL && !looks_serial)
+        return RADIO_PORT_WANTS_SERIAL;
+
+    return RADIO_PORT_OK;
+}
+
+/* One line the user can act on, or NULL when there is nothing to say. */
+static inline const char *radio_port_advice(radio_port_verdict_t v)
+{
+    switch (v)
+    {
+    case RADIO_PORT_WANTS_NETWORK:
+        return "this rig is reached over the NETWORK, not a serial port: set "
+               "radio_device to the radio's address (e.g. 192.168.1.50, or "
+               "192.168.1.50:4992 to override the port). radio_serial_speed "
+               "does not apply.";
+    case RADIO_PORT_WANTS_SERIAL:
+        return "this rig is reached over a SERIAL port: set radio_device to a "
+               "port name (e.g. COM4 on Windows, /dev/ttyUSB0 on Linux).";
+    case RADIO_PORT_OK:
+    default:
+        return NULL;
+    }
+}
+
+#endif /* RADIO_PORT_H */

radio_io/rigctl_parse.c

@@ -48,6 +48,7 @@ struct mod_entry
     char version[32];
     char status[32];
     char macro_name[32];
+    char port[8];
 };
 
 static struct mod_entry *g_models = NULL;
@@ -77,6 +78,20 @@ static int collect_model(const struct rig_caps *caps, void *data)
     snprintf(e->macro_name, sizeof(e->macro_name), "%s", caps->macro_name ? caps->macro_name : "");
     snprintf(e->status, sizeof(e->status), "%s", rig_strstatus(caps->status));
 
+    /* Which port this rig speaks is the one fact you need before setting
+     * radio_device, and it was previously invisible here (issue #179: a
+     * FlexRadio was configured with a COM port because the listing gave no
+     * hint it is a network rig). */
+    switch (caps->port_type)
+    {
+    case RIG_PORT_SERIAL:                 snprintf(e->port, sizeof(e->port), "serial"); break;
+    case RIG_PORT_NETWORK:
+    case RIG_PORT_UDP_NETWORK:            snprintf(e->port, sizeof(e->port), "net");    break;
+    case RIG_PORT_USB:                    snprintf(e->port, sizeof(e->port), "usb");    break;
+    case RIG_PORT_NONE:                   snprintf(e->port, sizeof(e->port), "-");      break;
+    default:                              snprintf(e->port, sizeof(e->port), "other");  break;
+    }
+
     return 1;
 }
 
@@ -184,7 +199,7 @@ void list_models(void)
     close(saved_stderr);
 #endif
 
-    printf(" Rig #  Mfg                    Model                   Version         Status      Macro\n");
+    printf(" Rig #  Mfg                    Model                   Version         Status      Port    Macro\n");
     status = rig_list_foreach(collect_model, NULL);
 
     if (status != RIG_OK)
@@ -198,12 +213,13 @@ void list_models(void)
     for (int i = 0; i < g_count; i++)
     {
         struct mod_entry *e = &g_models[i];
-        printf("%6d  %-23s%-24s%-16s%-12s%s\n",
+        printf("%6d  %-23s%-24s%-16s%-12s%-8s%s\n",
                e->id,
                e->mfg_name,
                e->model_name,
                e->version,
                e->status,
+               e->port,
                e->macro_name);
     }
 

tests/Makefile

@@ -46,7 +46,7 @@ TEST_BINS = test_ring_buffer test_arq_protocol test_arq_timing test_arq_fsm \
             test_tcp_interfaces test_resampler test_pcm24 test_arq_olla test_arq_sim \
             test_arq_conn test_cfg_utils test_arq_tnc test_channel_busy \
             test_freedv_harq test_sock_wire test_virtual_clock test_watterson \
-            test_ofdm_acq test_ui_status test_tx_pacing
+            test_ofdm_acq test_ui_status test_tx_pacing test_radio_port
 
 .PHONY: all test clean
 
@@ -174,6 +174,11 @@ test_ofdm_acq: modem/test_ofdm_acq.c $(UNITY_SRC) $(FREEDV_LIB)
 test_tx_pacing: modem/test_tx_pacing.c $(UNITY_SRC) ../modem/tx_pacing.h
 	$(CC) $(CFLAGS) -I../modem -o $@ modem/test_tx_pacing.c $(UNITY_SRC) $(LDFLAGS)
 
+# radio_device vs the rig's port type (pure header; no hamlib needed).
+# radio_port.h is a PREREQUISITE -- the logic under test lives in the header.
+test_radio_port: radio_io/test_radio_port.c $(UNITY_SRC) ../radio_io/radio_port.h
+	$(CC) $(CFLAGS) -I../radio_io -o $@ radio_io/test_radio_port.c $(UNITY_SRC) $(LDFLAGS)
+
 # UI status wire format (embedded struct vs remote JSON must not drift)
 test_ui_status: gui_interface/test_ui_status.c $(UNITY_SRC) ../gui_interface/ui_status.c
 	$(CC) $(CFLAGS) -I../gui_interface -I../datalink_arq -I../modem -I../common -o $@ $^ $(LDFLAGS)