From 220e7dd1c45274aad5bc023b5d8087786868e7aa Mon Sep 17 00:00:00 2001 From: wb2osz Date: Sun, 5 Aug 2018 11:20:27 -0400 Subject: [PATCH] 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. --- ax25_link.c | 65 ++++++++++++++++++++++++++++------------------------- ax25_link.h | 4 ++++ direwolf.c | 2 +- dlq.c | 47 ++++++++++++++++++++++++++++++++++++++ dlq.h | 4 +++- recv.c | 5 +++++ xmit.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 7 files changed, 157 insertions(+), 35 deletions(-) diff --git a/ax25_link.c b/ax25_link.c index 777ae3a..565c11e 100644 --- a/ax25_link.c +++ b/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 */ diff --git a/ax25_link.h b/ax25_link.h index 060b6ab..00e8230 100644 --- a/ax25_link.h +++ b/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); diff --git a/direwolf.c b/direwolf.c index 58ffad9..147bf27 100644 --- a/direwolf.c +++ b/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); diff --git a/dlq.c b/dlq.c index 6a4236f..5131674 100644 --- a/dlq.c +++ b/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 diff --git a/dlq.h b/dlq.h index 6874fb0..336870c 100644 --- a/dlq.h +++ b/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); diff --git a/recv.c b/recv.c index 8d7826a..2f82d54 100644 --- a/recv.c +++ b/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); diff --git a/xmit.c b/xmit.c index 4246d86..19a9ad0 100644 --- a/xmit.c +++ b/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,6 +755,12 @@ 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 @@ -730,6 +768,11 @@ static void xmit_ax25_frames (int chan, int prio, packet_t pp, int max_bundle) 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); }