Issue 132 continued. If a client app tried to send connected data when

the transmitter was already on, the packet would get stuck in the outgoing
data queue.

Author: wb2osz

ax25_link.c

@@ -156,6 +156,8 @@
  *		Implemented Multi Selective Reject.
  *		More efficient generation of SREJ frames.
  *		Reduced number of duplicate I frames sent for both REJ and SREJ cases.
+ *		Avoided unnecessary RR when I frame could take care of the ack.
+ *		(This led to issue 132 where outgoing data sometimes got stuck in the queue.)
  *
  *------------------------------------------------------------------*/
 
@@ -591,8 +593,6 @@ static int AX25MODULO(int n, int m, const char *file, const char *func, int line
 
 static void dl_data_indication (ax25_dlsm_t *S, int pid, char *data, int len);
 
-static void lm_seize_confirm (ax25_dlsm_t *S);
-
 static void i_frame (ax25_dlsm_t *S, cmdres_t cr, int p, int nr, int ns, int pid, char *info_ptr, int info_len);
 static void i_frame_continued (ax25_dlsm_t *S, int p, int ns, int pid, char *info_ptr, int info_len);
 static int is_ns_in_window (ax25_dlsm_t *S, int ns);
@@ -1801,8 +1801,6 @@ static void dl_data_indication (ax25_dlsm_t *S, int pid, char *data, int len)
  *
  * Description:	We need to pause the timers when the channel is busy.
  *
- *		Signal lm_seize_confirm when we have started to transmit.
- *	
  *------------------------------------------------------------------------------*/
 
 static int dcd_status[MAX_CHANS];
@@ -1859,14 +1857,6 @@ void lm_channel_busy (dlq_item_t *E)
 	      S->radio_channel_busy = 1;
 	      PAUSE_T1;
 	      PAUSE_TM201;
-
-	      // Did channel become busy due to PTT turning on?
-
-	      if ( E->activity == OCTYPE_PTT && E->status == 1) {
-
-	        lm_seize_confirm (S);	// C4.2.  "This primitive indicates, to the Data-link State
-					// machine, that the transmission opportunity has arrived."
-	      }
 	    }
 	    else if ( ! busy && S->radio_channel_busy) {
 	      S->radio_channel_busy = 0;
@@ -1901,32 +1891,43 @@ void lm_channel_busy (dlq_item_t *E)
  *
  *------------------------------------------------------------------------------*/
 
-static void lm_seize_confirm (ax25_dlsm_t *S)
+void lm_seize_confirm (dlq_item_t *E)
 {
 
-	switch (S->state) {
+	assert (E->chan >= 0 && E->chan < MAX_CHANS);
 
-	  case 	state_0_disconnected:
-	  case 	state_1_awaiting_connection:
-	  case 	state_2_awaiting_release:
-	  case 	state_5_awaiting_v22_connection:
+	ax25_dlsm_t *S;
 
-	    break;
+	for (S = list_head; S != NULL; S = S->next) {
 
-	  case 	state_3_connected:
-	  case 	state_4_timer_recovery:
+	  if (E->chan == S->chan) {
 
-	    // v1.5 change in strategy.
-	    // New I frames, not sent yet, are delayed until after processing anything in the received transmission.
-	    // Previously we started sending new frames, from the client app, as soon as they arrived.
-	    // Now, we first take care of those in progress before throwing more into the mix.
 
-	    i_frame_pop_off_queue(S);
+	    switch (S->state) {
 
-	    if (S->acknowledge_pending) {
-	      S->acknowledge_pending = 0;
-	      enquiry_response (S, frame_not_AX25, 0);
-	    }
+	      case 	state_0_disconnected:
+	      case 	state_1_awaiting_connection:
+	      case 	state_2_awaiting_release:
+	      case 	state_5_awaiting_v22_connection:
+
+	        break;
+
+	      case 	state_3_connected:
+	      case 	state_4_timer_recovery:
+
+	        // v1.5 change in strategy.
+	        // New I frames, not sent yet, are delayed until after processing anything in the received transmission.
+	        // Previously we started sending new frames, from the client app, as soon as they arrived.
+	        // Now, we first take care of those in progress before throwing more into the mix.
+
+	        i_frame_pop_off_queue(S);
+
+	        // Need an RR if we didn't have I frame send the necessary ack.
+
+	        if (S->acknowledge_pending) {
+	          S->acknowledge_pending = 0;
+	          enquiry_response (S, frame_not_AX25, 0);
+	        }
 
 // Implementation difference: The flow chart for state 3 has LM-RELEASE Request here.
 // I don't think I need it because the transmitter will turn off
@@ -1935,7 +1936,9 @@ static void lm_seize_confirm (ax25_dlsm_t *S)
 // Erratum: The original spec had LM-SEIZE request here, for state 4, which didn't seem right.
 // The 2006 revision has LM-RELEASE Request so states 3 & 4 are the same.
 
-	    break;
+	        break;
+	    }
+	  }
 	}
 
 } /* lm_seize_confirm */

ax25_link.h

@@ -52,6 +52,8 @@ void ax25_link_init (struct misc_config_s *pconfig);
 // These functions must be called on a single thread, one at a time.
 // The Data Link Queue (DLQ) is used to serialize events from multiple sources.
 
+// Maybe the dispatch switch should be moved to ax25_link.c so they can all
+// be made static and they can't be called from the wrong place accidentally.
 
 void dl_connect_request (dlq_item_t *E);
 
@@ -68,6 +70,8 @@ void dl_client_cleanup (dlq_item_t *E);
 
 void lm_data_indication (dlq_item_t *E);
 
+void lm_seize_confirm (dlq_item_t *E);
+
 void lm_channel_busy (dlq_item_t *E);
 
 

direwolf.c

@@ -262,7 +262,7 @@ int main (int argc, char *argv[])
 
 	text_color_init(t_opt);
 	text_color_set(DW_COLOR_INFO);
-	dw_printf ("Dire Wolf version %d.%d (%s) Beta Test 3\n", MAJOR_VERSION, MINOR_VERSION, __DATE__);
+	dw_printf ("Dire Wolf version %d.%d (%s) Beta Test 4\n", MAJOR_VERSION, MINOR_VERSION, __DATE__);
 	//dw_printf ("Dire Wolf DEVELOPMENT version %d.%d %s (%s)\n", MAJOR_VERSION, MINOR_VERSION, "C", __DATE__);
 	//dw_printf ("Dire Wolf version %d.%d\n", MAJOR_VERSION, MINOR_VERSION);
 

dlq.c

@@ -767,6 +767,53 @@ void dlq_channel_busy (int chan, int activity, int status)
 
 
 
+
+
+/*-------------------------------------------------------------------
+ *
+ * Name:        dlq_seize_confirm
+ *
+ * Purpose:     Inform data link state machine that the transmitter is on.
+ *		This is in reponse to lm_seize_request.
+ *
+ * Inputs:	chan		- Radio channel number.
+ *
+ * Outputs:	Request is appended to queue for processing by
+ *		the data link state machine.
+ *
+ * Description:	When removed from the data link state machine queue, this
+ *		becomes lm_seize_confirm.
+ *
+ *--------------------------------------------------------------------*/
+
+void dlq_seize_confirm (int chan)
+{
+	struct dlq_item_s *pnew;
+
+#if DEBUG
+	text_color_set(DW_COLOR_DEBUG);
+	dw_printf ("dlq_seize_confirm (chan=%d)\n", chan);
+#endif
+
+
+/* Allocate a new queue item. */
+
+	pnew = (struct dlq_item_s *) calloc (sizeof(struct dlq_item_s), 1);
+	s_new_count++;
+
+	pnew->type = DLQ_SEIZE_CONFIRM;
+	pnew->chan = chan;
+
+/* Put it into queue. */
+
+	append_to_queue (pnew);
+
+
+} /* end dlq_seize_confirm */
+
+
+
+
 /*-------------------------------------------------------------------
  *
  * Name:        dlq_client_cleanup

dlq.h

@@ -35,7 +35,7 @@ typedef struct cdata_s {
 
 /* Types of things that can be in queue. */
 
-typedef enum dlq_type_e {DLQ_REC_FRAME, DLQ_CONNECT_REQUEST, DLQ_DISCONNECT_REQUEST, DLQ_XMIT_DATA_REQUEST, DLQ_REGISTER_CALLSIGN, DLQ_UNREGISTER_CALLSIGN, DLQ_CHANNEL_BUSY, DLQ_CLIENT_CLEANUP} dlq_type_t;
+typedef enum dlq_type_e {DLQ_REC_FRAME, DLQ_CONNECT_REQUEST, DLQ_DISCONNECT_REQUEST, DLQ_XMIT_DATA_REQUEST, DLQ_REGISTER_CALLSIGN, DLQ_UNREGISTER_CALLSIGN, DLQ_CHANNEL_BUSY, DLQ_SEIZE_CONFIRM, DLQ_CLIENT_CLEANUP} dlq_type_t;
 
 
 /* A queue item. */
@@ -116,6 +116,8 @@ void dlq_unregister_callsign (char addr[AX25_MAX_ADDR_LEN], int chan, int client
 
 void dlq_channel_busy (int chan, int activity, int status);
 
+void dlq_seize_confirm (int chan);
+
 void dlq_client_cleanup (int client);
 
 

recv.c

@@ -378,6 +378,11 @@ void recv_process (void)
 	          lm_channel_busy (pitem);
 	          break;
 
+		case DLQ_SEIZE_CONFIRM:
+
+	          lm_seize_confirm (pitem);
+	          break;
+
 	        case DLQ_CLIENT_CLEANUP:
 
 	          dl_client_cleanup (pitem);

xmit.c

@@ -74,6 +74,7 @@
 #include "morse.h"
 #include "dtmf.h"
 #include "xid.h"
+#include "dlq.h"
 
 
 
@@ -104,8 +105,8 @@ static int xmit_txtail[MAX_CHANS];	/* Amount of time to keep transmitting after
 static int xmit_fulldup[MAX_CHANS];	/* Full duplex if non-zero. */
 
 static int xmit_bits_per_sec[MAX_CHANS];	/* Data transmission rate. */
-					/* Often called baud rate which is equivalent in */
-					/* this case but could be different with other */
+					/* Often called baud rate which is equivalent for */
+					/* 1200 & 9600 cases but could be different with other */
 					/* modulation techniques. */
 
 static int g_debug_xmit_packet;		/* print packet in hexadecimal form for debugging. */
@@ -114,11 +115,42 @@ static int g_debug_xmit_packet;		/* print packet in hexadecimal form for debuggi
 // TODO: When this was first written, bits/sec was same as baud.
 // Need to revisit this for PSK modes where they are not the same.
 
+#if 0		// Added during 1.5 beta test
+
+static int BITS_TO_MS (int b, int ch) {
+
+	int bits_per_symbol;
+
+	switch (save_audio_config_p->achan[ch].modem_type) {
+	  case MODEM_QPSK:	bits_per_symbol = 2; break;
+	  case MODEM_8PSK:	bits_per_symbol = 3; break;
+	  case default:		bits_per_symbol = 1; break;
+	}
+
+	return ( (b * 1000) / (xmit_bits_per_sec[(ch)] * bits_per_symbol) );
+}
+
+static int MS_TO_BITS (int ms, int ch) {
+
+	int bits_per_symbol;
+
+	switch (save_audio_config_p->achan[ch].modem_type) {
+	  case MODEM_QPSK:	bits_per_symbol = 2; break;
+	  case MODEM_8PSK:	bits_per_symbol = 3; break;
+	  case default:		bits_per_symbol = 1; break;
+	}
+
+	return ( (ms * xmit_bits_per_sec[(ch)] * bits_per_symbol) / 1000 );  TODO...
+}
+
+#else		// OK for 1200, 9600 but wrong for PSK
 
 #define BITS_TO_MS(b,ch) (((b)*1000)/xmit_bits_per_sec[(ch)])
 
 #define MS_TO_BITS(ms,ch) (((ms)*xmit_bits_per_sec[(ch)])/1000)
 
+#endif
+
 #define MAXX(a,b) (((a)>(b)) ? (a) : (b))
 
 
@@ -723,13 +755,24 @@ static void xmit_ax25_frames (int chan, int prio, packet_t pp, int max_bundle)
 #endif
 	ptt_set (OCTYPE_PTT, chan, 1);
 
+
+// Inform data link state machine that we are now transmitting.
+
+	dlq_seize_confirm (chan);	// C4.2.  "This primitive indicates, to the Data-link State
+					// machine, that the transmission opportunity has arrived."
+
 	pre_flags = MS_TO_BITS(xmit_txdelay[chan] * 10, chan) / 8;
 	num_bits =  hdlc_send_flags (chan, pre_flags, 0);
 #if DEBUG
 	text_color_set(DW_COLOR_DEBUG);
 	dw_printf ("xmit_thread: txdelay=%d [*10], pre_flags=%d, num_bits=%d\n", xmit_txdelay[chan], pre_flags, num_bits);
 #endif
 
+	SLEEP_MS (10);			// Give data link state machine a chance to
+					// to stuff more frames into the transmit queue,
+					// in response to dlq_seize_confirm, so
+					// we don't run off the end too soon.
+
 
 /*
  * Transmit the frame.
@@ -913,6 +956,24 @@ static int send_one_frame (int c, int p, packet_t pp)
 
 
 	if (ax25_is_null_frame(pp)) {
+
+	  // Issue 132 - We could end up in a situation where:
+	  // Transmitter is already on.
+	  // Application wants to send a frame.
+	  // dl_seize_request turns into this null frame.
+	  // It was being ignored here so the data got stuck in the queue.
+	  // I think the solution is to send back a seize confirm here.
+	  // It shouldn't hurt if we send it redundantly.
+	  // Added for 1.5 beta test 4.
+
+	  dlq_seize_confirm (c);	// C4.2.  "This primitive indicates, to the Data-link State
+					// machine, that the transmission opportunity has arrived."
+
+	  SLEEP_MS (10);		// Give data link state machine a chance to
+					// to stuff more frames into the transmit queue,
+					// in response to dlq_seize_confirm, so
+					// we don't run off the end too soon.
+
 	  return(0);
 	}