Skip to content

Commit

Permalink
Issue 132 continued. If a client app tried to send connected data when
Browse files Browse the repository at this point in the history
the transmitter was already on, the packet would get stuck in the outgoing
data queue.
  • Loading branch information
wb2osz committed Aug 5, 2018
1 parent 4ecaf47 commit 220e7dd
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 35 deletions.
65 changes: 34 additions & 31 deletions ax25_link.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.)
*
*------------------------------------------------------------------*/

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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 */
Expand Down
4 changes: 4 additions & 0 deletions ax25_link.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);


Expand Down
2 changes: 1 addition & 1 deletion direwolf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
47 changes: 47 additions & 0 deletions dlq.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion dlq.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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);


Expand Down
5 changes: 5 additions & 0 deletions recv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
65 changes: 63 additions & 2 deletions xmit.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
#include "morse.h"
#include "dtmf.h"
#include "xid.h"
#include "dlq.h"



Expand Down Expand Up @@ -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. */
Expand All @@ -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))


Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}

Expand Down

0 comments on commit 220e7dd

Please sign in to comment.