From 3a67356bc1c3b4bc5d5f3697c1a7cb7242555b79 Mon Sep 17 00:00:00 2001
From: Mikael Nousiainen <mikael.nousiainen@iki.fi>
Date: Mon, 31 May 2021 16:22:22 +0300
Subject: [PATCH] Add some comments and improved debugging on SIGIO async data
 handling

---
 rigs/icom/frame.c |  1 +
 rigs/icom/icom.c  |  5 +----
 src/event.c       | 29 ++++++++++++++++++++++++-----
 tests/rigctld.c   | 20 +++++++++++++++-----
 4 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/rigs/icom/frame.c b/rigs/icom/frame.c
index 967535c2b..65997f093 100644
--- a/rigs/icom/frame.c
+++ b/rigs/icom/frame.c
@@ -346,6 +346,7 @@ read_another_frame:
         RETURNFUNC(-RIG_EPROTO);
     }
 
+    // TODO: Does ctrlid (detected by icom_is_async_frame) vary (seeing some code above using 0x80 for non-full-duplex)?
     if (icom_is_async_frame(rig, frm_len, buf))
     {
         int elapsed_ms;
diff --git a/rigs/icom/icom.c b/rigs/icom/icom.c
index 8f09cc214..1375f9afe 100644
--- a/rigs/icom/icom.c
+++ b/rigs/icom/icom.c
@@ -8061,9 +8061,6 @@ static int icom_parse_spectrum_frame(RIG *rig, int length, const unsigned char *
 
 int icom_is_async_frame(RIG *rig, int frame_len, const unsigned char *frame)
 {
-    struct rig_state *rs = &rig->state;
-    struct icom_priv_data *priv = (struct icom_priv_data *) rs->priv;
-
     if (frame_len < ACKFRMLEN)
     {
         return 0;
@@ -8167,7 +8164,7 @@ int icom_decode_event(RIG *rig)
 
     if (frm_len < 1)
     {
-        RETURNFUNC(0);
+        RETURNFUNC(RIG_OK);
     }
 
     retval = icom_frame_fix_preamble(frm_len, buf);
diff --git a/src/event.c b/src/event.c
index 79b3d4e73..e0e9cbf1c 100644
--- a/src/event.c
+++ b/src/event.c
@@ -56,9 +56,9 @@
 #include <signal.h>
 #include <errno.h>
 
-
 #include <hamlib/rig.h>
 #include "event.h"
+#include "misc.h"
 
 #if defined(WIN32) && !defined(HAVE_TERMIOS_H)
 #  include "win32termios.h"
@@ -300,6 +300,8 @@ static int search_rig_and_decode(RIG *rig, rig_ptr_t data)
     struct timeval tv;
     int retval;
 
+    ENTERFUNC;
+
     /*
      * so far, only file oriented ports have event reporting support
      */
@@ -309,6 +311,22 @@ static int search_rig_and_decode(RIG *rig, rig_ptr_t data)
         return -1;
     }
 
+    /*
+     * TODO: FIXME: We may end up calling decode_event right before or after the hold_decode lock is released
+     *   by backend transaction routine. With the Icom backend this will end up waiting for the next CI-V frame
+     *   to be read and this will interfere with reading of the next response to any command.
+     *   => It is difficult to find a way to avoid this routine picking up regular responses.
+     */
+
+    /*
+     * Do not disturb, the backend is currently receiving data
+     */
+    if (rig->state.hold_decode)
+    {
+        rig_debug(RIG_DEBUG_TRACE, "%s: hold decode, backend is receiving data\n", __func__);
+        RETURNFUNC(-1);
+    }
+
     /* FIXME: siginfo is not portable, however use it where available */
 #if 0&&defined(HAVE_SIGINFO_T)
     siginfo_t *si = (siginfo_t *)data;
@@ -334,10 +352,10 @@ static int search_rig_and_decode(RIG *rig, rig_ptr_t data)
     if (retval < 0)
     {
         rig_debug(RIG_DEBUG_ERR,
-                  "%s: select: %s\n",
+                  "%s: select() failed: %s\n",
                   __func__,
                   strerror(errno));
-        return -1;
+        RETURNFUNC(-1);
     }
 
 #endif
@@ -347,7 +365,8 @@ static int search_rig_and_decode(RIG *rig, rig_ptr_t data)
      */
     if (rig->state.hold_decode)
     {
-        return -1;
+        rig_debug(RIG_DEBUG_TRACE, "%s: hold decode, backend is receiving data\n", __func__);
+        RETURNFUNC(-1);
     }
 
     if (rig->caps->decode_event)
@@ -355,7 +374,7 @@ static int search_rig_and_decode(RIG *rig, rig_ptr_t data)
         rig->caps->decode_event(rig);
     }
 
-    return 1;   /* process each opened rig */
+    RETURNFUNC(1);   /* process each opened rig */
 }
 
 
diff --git a/tests/rigctld.c b/tests/rigctld.c
index 105185217..50845a95f 100644
--- a/tests/rigctld.c
+++ b/tests/rigctld.c
@@ -910,11 +910,19 @@ int main(int argc, char *argv[])
         timeout.tv_usec = 0;
         retcode = select(sock_listen + 1, &set, NULL, NULL, &timeout);
 
-        if (-1 == retcode)
+        if (retcode == -1)
         {
-            rig_debug(RIG_DEBUG_ERR, "%s: select\n", __func__);
+            int errno_stored = errno;
+            rig_debug(RIG_DEBUG_ERR, "%s: select() failed: %s\n", __func__, strerror(errno_stored));
+
+            // TODO: FIXME: Why does this select() return EINTR after any command when set_trn RIG is enabled?
+            if (errno == EINTR)
+            {
+                rig_debug(RIG_DEBUG_VERBOSE, "%s: ignoring interrupted system call\n", __func__);
+                retcode = 0;
+            }
         }
-        else if (!retcode)
+        else if (retcode == 0)
         {
             if (ctrl_c)
             {
@@ -1039,11 +1047,13 @@ void *handle_socket(void *arg)
     int ext_resp = 0;
     char resp_sep = '\n';
 
+    ENTERFUNC;
+
     fsockin = get_fsockin(handle_data_arg);
 
     if (!fsockin)
     {
-        rig_debug(RIG_DEBUG_ERR, "fdopen(0x%d) in: %s\n", handle_data_arg->sock,
+        rig_debug(RIG_DEBUG_ERR, "%s: fdopen(0x%d) in: %s\n", __func__, handle_data_arg->sock,
                   strerror(errno));
         goto handle_exit;
     }
@@ -1052,7 +1062,7 @@ void *handle_socket(void *arg)
 
     if (!fsockout)
     {
-        rig_debug(RIG_DEBUG_ERR, "fdopen out: %s\n", strerror(errno));
+        rig_debug(RIG_DEBUG_ERR, "%s: fdopen out: %s\n", __func__, strerror(errno));
         fclose(fsockin);
 
         goto handle_exit;