arq: drop an unreachable pattern correlation; refuse to truncate callsigns

Three connect-path cleanups, no wire change.

1. expect_pattern_ack no longer includes ACCEPTING. No pattern can arrive in
   that state: the caller answers an ACCEPT with either a coded DATAC16 confirm
   or a first MFSK DATA burst, never a pattern — send_ack() emits one only once
   the session is CONNECTED. So the correlator ran for the whole ~18 s ACCEPT
   window, and per the measurement already recorded at that site it costs about
   half the RX sample budget (3.5k samp/s consumed against 8k arriving, capture
   ring growing to ~400 kB). It was burning that precisely while the answerer
   had to decode the frames that complete the connect.

   Note this is NOT visible in connect_bench: the sim has no DSP, so it cannot
   show RX budget. The justification is that the branch is unreachable plus the
   existing DIAG measurement of what the correlator costs; confirming the gain
   needs the live path (make DEBUG_IO=1).

2. Callsign encoders refuse over-long input instead of truncating. Truncating
   an arithmetic code does not shorten the string — it decodes to a DIFFERENT
   one, so the peer answers a call from a station that does not exist, or drops
   one that does. Both encode_callsign_payload (CALL/ACCEPT) and
   encode_callsign_only_payload (CQ) had this. They now return -1, which
   build_call_accept already propagates, and send_call_accept says why rather
   than leaving a session retrying against silence.

   Tests: a round-trip over realistic callsigns including SSIDs (PU2UIT-2,
   DL9ABC-15) and a refusal test verified to fail against the old truncating
   code.

3. Dead code: startup_deadline_ms was written at three sites and read nowhere;
   ARQ_CONNECT_GRACE_SLOTS and ARQ_CONNECT_BUSY_EXT_S were never referenced.

   Left alone deliberately, because it is user-facing and the call is not
   mine: `startup_max_s` is a documented mercury.ini knob, clamped 2..60 and
   written back by cfg_write, whose ONLY effect was to set the unread field
   above. It is now inert — an operator can tune it and nothing happens. It
   should either be implemented or removed.

Full C suite green, go test -count=1 green (203 s). The arq.c:754
format-truncation warning is pre-existing (present on the unmodified tree).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: rafael2k

datalink_arq/arq.c

@@ -1107,13 +1107,20 @@ bool arq_get_runtime_snapshot(arq_runtime_snapshot_t *snapshot)
      * guard then flushing bursts that were still arriving.  Trunk, which has no
      * pattern ACK, holds 7.99k samp/s on the same bench.
      *
-     * WAIT_ACK is by definition the only in-session state where the peer's
-     * pattern ACK is due (see ARQ_DFLOW_WAIT_ACK), and ACCEPTING is the
-     * handshake equivalent.  Everywhere else -- above all IDLE_IRS, where the
-     * receiver is busy demodulating a 13.5 s burst -- there is nothing to
-     * detect and the CPU is needed elsewhere. */
+     * WAIT_ACK is by definition the only state where a pattern ACK is due
+     * (see ARQ_DFLOW_WAIT_ACK).  Everywhere else -- above all IDLE_IRS, where
+     * the receiver is busy demodulating a 13.5 s burst -- there is nothing to
+     * detect and the CPU is needed elsewhere.
+     *
+     * ACCEPTING used to be included as "the handshake equivalent".  It is not:
+     * no pattern can arrive there.  The caller answers an ACCEPT with either a
+     * coded DATAC16 confirm (send_call_accept -> ARQ_DFLOW_ACK_TX with
+     * pending_connect_confirm, arq_fsm.c) or a first MFSK DATA burst -- never a
+     * pattern, which send_ack() emits only once the session is CONNECTED.  So
+     * the correlator ran for the whole ~18 s ACCEPT window and spent roughly
+     * half the RX budget precisely while the answerer had to decode the frames
+     * that complete the connect. */
     snapshot->expect_pattern_ack =
-        (g_sess.conn_state == ARQ_CONN_ACCEPTING) ||
         (g_sess.conn_state == ARQ_CONN_CONNECTED &&
          g_sess.dflow_state == ARQ_DFLOW_WAIT_ACK);
     snapshot->trx              = trx;

datalink_arq/arq_fsm.c

@@ -461,6 +461,14 @@ static void send_call_accept(arq_session_t *sess, bool is_accept)
                                     my_call, sess->remote_call, bw_hz);
     if (n > 0)
         send_frame(PACKET_TYPE_ARQ_CALL, sess->control_mode, (size_t)n, frame, 0);
+    else
+        /* Almost always an over-long callsign: the 10-byte SRC slot holds ~14
+         * characters at ~5.25 bits each.  The encoder refuses rather than
+         * truncating (a truncated arithmetic code decodes to a different
+         * string), so say why — otherwise this is a CALL that never goes out
+         * and a session that retries against silence. */
+        HLOGW(LOG_COMP, "%s not sent: cannot encode callsign '%s' (too long?)",
+              is_accept ? "ACCEPT" : "CALL", my_call);
 }
 
 static void send_ctrl_frame(arq_session_t *sess, arq_subtype_t subtype)
@@ -788,7 +796,6 @@ static void fsm_listening(arq_session_t *sess, const arq_event_t *ev)
         {
             sess->role        = ARQ_ROLE_CALLEE;
             reset_session_data_state(sess);
-            sess->startup_deadline_ms = time_now_ms() + (ARQ_STARTUP_MAX_S * 1000ULL);
             if (g_cbs.notify_connected)
                 g_cbs.notify_connected(sess->remote_call, sess->local_call);
             if (g_timing)
@@ -815,8 +822,6 @@ static void fsm_calling(arq_session_t *sess, const arq_event_t *ev)
             bool has_tx_backlog = session_tx_backlog(sess) > 0;
             sess->role        = ARQ_ROLE_CALLER;
             reset_session_data_state(sess);  /* discard stale retransmit buf; MFSK-start */
-            sess->startup_deadline_ms =
-                time_now_ms() + (ARQ_STARTUP_MAX_S * 1000ULL);
             if (g_cbs.notify_connected)
                 g_cbs.notify_connected(sess->remote_call, sess->local_call);
             if (g_timing)
@@ -889,8 +894,6 @@ static void fsm_accepting(arq_session_t *sess, const arq_event_t *ev)
     case ARQ_EV_RX_ACK:
         sess->role        = ARQ_ROLE_CALLEE;
         reset_session_data_state(sess);  /* discard stale retransmit buf; MFSK-start */
-        sess->startup_deadline_ms =
-            time_now_ms() + (ARQ_STARTUP_MAX_S * 1000ULL);
         if (g_cbs.notify_connected)
             g_cbs.notify_connected(sess->remote_call, sess->local_call);
         if (g_timing)

datalink_arq/arq_fsm.h

@@ -195,7 +195,6 @@ typedef struct
     /* --- Retry/timeout bookkeeping --- */
     int      tx_retries_left;          /* retries remaining for current frame  */
     uint64_t state_enter_ms;          /* when current conn_state was entered   */
-    uint64_t startup_deadline_ms;     /* end of control-mode-only startup      */
 
     /* --- Peer state observed from frames --- */
     bool     peer_has_data;            /* peer's HAS_DATA flag in last frame   */

datalink_arq/arq_protocol.c

@@ -440,9 +440,14 @@ static int encode_callsign_payload(const char *src, const char *dst,
     if (enc_len <= 0)
         return -1;
 
+    /* Refuse rather than truncate.  A truncated arithmetic code is not a
+     * shortened callsign — it decodes to a DIFFERENT string at the far end,
+     * so the peer answers a call from a station that does not exist, or
+     * silently drops one that does.  Failing here makes the operator's
+     * over-long callsign visible instead of turning it into a wrong one. */
     size_t src_cap = out_cap - ARQ_CONNECT_DST_CRC_SIZE;
     if ((size_t)enc_len > src_cap)
-        enc_len = (int)src_cap;
+        return -1;
     memcpy(out + ARQ_CONNECT_DST_CRC_SIZE, tmp, (size_t)enc_len);
     return (int)(ARQ_CONNECT_DST_CRC_SIZE + (size_t)enc_len);
 }
@@ -489,8 +494,10 @@ static int encode_callsign_only_payload(const char *src, uint8_t *out, size_t ou
     if (enc_len <= 0)
         return -1;
 
+    /* Same reasoning as encode_callsign_payload: truncation corrupts, it does
+     * not shorten. */
     if ((size_t)enc_len > out_cap)
-        enc_len = (int)out_cap;
+        return -1;
     memcpy(out, tmp, (size_t)enc_len);
     return enc_len;
 }

datalink_arq/arq_protocol.h

@@ -245,8 +245,6 @@ extern _Atomic float arq_callint_override_s;
 #define ARQ_ACCEPT_RETRY_SLOTS     atomic_load(&arq_accept_retry_slots)
 #define ARQ_DATA_RETRY_SLOTS       atomic_load(&arq_data_retry_slots)
 #define ARQ_DISCONNECT_RETRY_SLOTS atomic_load(&arq_disconnect_retry_slots)
-#define ARQ_CONNECT_GRACE_SLOTS       2     /* extra wait slots for ACCEPT         */
-#define ARQ_CONNECT_BUSY_EXT_S        2     /* busy-extension guard after CALL     */
 #define ARQ_STARTUP_MAX_S_DEFAULT     10    /* control-mode-only startup window    */
 extern _Atomic int arq_startup_max_s;
 #define ARQ_STARTUP_MAX_S  atomic_load(&arq_startup_max_s)

tests/datalink_arq/test_arq_protocol.c

@@ -315,6 +315,52 @@ void test_mode_timing_datac16_unaffected_by_override(void)
     TEST_ASSERT_FLOAT_WITHIN(0.01f, 8.0f, t->retry_interval_s);
 }
 
+/* ---- callsign packing: round-trip, and refusal instead of corruption ---- */
+
+/* The SRC callsign is arithmetic-coded over a 38-symbol alphabet (A-Z, 0-9,
+ * '-', EOF) with a uniform model, i.e. ~5.25 bits/char, into a 10-byte slot.
+ * Realistic callsigns must survive a round trip unchanged. */
+void test_callsign_roundtrip_realistic(void)
+{
+    static const char *calls[] = {
+        "PU2UIT", "PU2UIT-2", "A0AAA", "ZL1ANY", "W1AW", "DL9ABC-15", "PY2-XYZ"
+    };
+    for (unsigned i = 0; i < sizeof calls / sizeof calls[0]; i++)
+    {
+        uint8_t frame[ARQ_CONTROL_FRAME_SIZE];
+        int n = arq_protocol_build_call(frame, sizeof frame, 0x2A,
+                                        calls[i], "DSTCALL", 2300);
+        char msg[96];
+        snprintf(msg, sizeof msg, "build_call failed for '%s'", calls[i]);
+        TEST_ASSERT_GREATER_THAN_MESSAGE(0, n, msg);
+
+        uint8_t sid = 0; int bw = 0;
+        char src[CALLSIGN_MAX_SIZE] = {0}, dst[CALLSIGN_MAX_SIZE] = {0};
+        TEST_ASSERT_EQUAL_INT_MESSAGE(0,
+            arq_protocol_parse_call(frame, (size_t)n, &sid, src, dst, &bw), msg);
+        TEST_ASSERT_EQUAL_STRING_MESSAGE(calls[i], src, msg);
+        TEST_ASSERT_EQUAL_UINT8_MESSAGE(0x2A, sid, msg);
+    }
+}
+
+/* A callsign too long for the slot must be REFUSED, not truncated.
+ *
+ * Truncating an arithmetic code does not shorten the string — it decodes to a
+ * DIFFERENT one, so the peer either answers a call from a station that does
+ * not exist or drops one that does. The old code truncated and reported
+ * success. */
+void test_callsign_too_long_is_refused_not_truncated(void)
+{
+    uint8_t frame[ARQ_CONTROL_FRAME_SIZE];
+    const char *huge = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+
+    int n = arq_protocol_build_call(frame, sizeof frame, 0x2A,
+                                    huge, "DSTCALL", 2300);
+    TEST_ASSERT_LESS_OR_EQUAL_INT_MESSAGE(0, n,
+        "over-long callsign was accepted — it will decode to a different "
+        "station at the far end");
+}
+
 int main(void)
 {
     UNITY_BEGIN();
@@ -348,5 +394,7 @@ int main(void)
     RUN_TEST(test_call_interval_override);
     RUN_TEST(test_call_interval_reset);
     RUN_TEST(test_mode_timing_datac16_unaffected_by_override);
+    RUN_TEST(test_callsign_roundtrip_realistic);
+    RUN_TEST(test_callsign_too_long_is_refused_not_truncated);
     return UNITY_END();
 }