Skip to content

Commit

Permalink
Merge pull request nasa#115 from jphickey/fix-113-acknak-counts
Browse files Browse the repository at this point in the history 
Fix nasa#113, replace acknack count union
  • Loading branch information
@astrogeco
astrogeco authored Dec 17, 2021
2 parents a894069 + d800b4b commit a0d6f56
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 62 deletions.
12 changes: 3 additions & 9 deletions fsw/src/cf_cfdp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,16 +134,10 @@ typedef struct CF_Poll
bool timer_set;
} CF_Poll_t;

typedef union CF_RxTxCounters
{
unsigned ack;
uint8 nak;
} CF_RxTxCounters_t;

typedef struct CF_TxS2_Data
{
uint8 fin_cc; /* remember the cc in the received fin pdu to echo in eof-fin */
CF_RxTxCounters_t counter;
uint8 fin_cc; /* remember the cc in the received fin pdu to echo in eof-fin */
uint8 acknak_count;
} CF_TxS2_Data_t;

typedef struct CF_TxState_Data
Expand All @@ -162,7 +156,7 @@ typedef struct CF_RxS2_Data
CF_CFDP_FinDeliveryCode_t dc;
CF_CFDP_FinFileStatus_t fs;
uint8 eof_cc; /* remember the cc in the received eof pdu to echo in eof-ack */
CF_RxTxCounters_t counter;
uint8 acknak_count;
} CF_RxS2_Data_t;

typedef struct CF_RxState_Data
Expand Down
14 changes: 7 additions & 7 deletions fsw/src/cf_cfdp_r.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,15 +181,15 @@ static void CF_CFDP_R2_Complete(CF_Transaction_t *t, int ok_to_send_nak)

if (send_nak && ok_to_send_nak)
{
if (++t->state_data.r.r2.counter.nak == CF_AppData.config_table->nak_limit)
if (++t->state_data.r.r2.acknak_count >= CF_AppData.config_table->nak_limit)
{
CFE_EVS_SendEvent(CF_EID_ERR_CFDP_R_NAK_LIMIT, CFE_EVS_EventType_ERROR, "CF R%d(%u:%u): nak limited reach",
(t->state == CF_TxnState_R2), t->history->src_eid, t->history->seq_num);
send_fin = 1;
++CF_AppData.hk.channel_hk[t->chan_num].counters.fault.nak_limit;
t->history->cc = CF_CFDP_ConditionCode_NAK_LIMIT_REACHED; /* don't use CF_CFDP_R2_SetCc because many places
in this function set send_fin */
t->state_data.r.r2.counter.nak = 0; /* reset for fin/ack */
t->state_data.r.r2.acknak_count = 0; /* reset for fin/ack */
}
else
{
Expand Down Expand Up @@ -493,7 +493,7 @@ static void CF_CFDP_R2_SubstateRecvFileData(CF_Transaction_t *t, const CF_CFDP_P
CF_CFDP_ArmAckTimer(t); /* re-arm ack timer, since we got data */
}

t->state_data.r.r2.counter.nak = 0;
t->state_data.r.r2.acknak_count = 0;

return;

Expand Down Expand Up @@ -916,9 +916,9 @@ static void CF_CFDP_R2_RecvMd(CF_Transaction_t *t, const CF_CFDP_PduHeader_t *ph
}
}

t->flags.rx.md_recv = 1;
t->state_data.r.r2.counter.nak = 0; /* in case part of nak */
CF_CFDP_R2_Complete(t, 1); /* check for completion now that md is received */
t->flags.rx.md_recv = 1;
t->state_data.r.r2.acknak_count = 0; /* in case part of nak */
CF_CFDP_R2_Complete(t, 1); /* check for completion now that md is received */
}
else
{
Expand Down Expand Up @@ -1147,7 +1147,7 @@ void CF_CFDP_R_Tick(CF_Transaction_t *t, int *cont /* unused */)
}
else if (t->state_data.r.sub_state == CF_RxSubState_WAIT_FOR_FIN_ACK)
{
if (++t->state_data.r.r2.counter.ack == CF_AppData.config_table->ack_limit)
if (++t->state_data.r.r2.acknak_count >= CF_AppData.config_table->ack_limit)
{
CFE_EVS_SendEvent(CF_EID_ERR_CFDP_R_ACK_LIMIT, CFE_EVS_EventType_ERROR,
"CF R2(%u:%u): ack limit reached, no fin-ack", t->history->src_eid,
Expand Down
2 changes: 1 addition & 1 deletion fsw/src/cf_cfdp_s.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ void CF_CFDP_S_Tick(CF_Transaction_t *t, int *cont /* unused */)
{
if (t->state_data.s.sub_state == CF_TxSubState_WAIT_FOR_EOF_ACK)
{
if (++t->state_data.s.s2.counter.ack == CF_AppData.config_table->ack_limit)
if (++t->state_data.s.s2.acknak_count >= CF_AppData.config_table->ack_limit)
{
CFE_EVS_SendEvent(CF_EID_ERR_CFDP_S_ACK_LIMIT, CFE_EVS_EventType_ERROR,
"CF S2(%u:%u), ack limit reached, no eof-ack", t->history->src_eid,
Expand Down
71 changes: 34 additions & 37 deletions unit-test/cf_cfdp_r_tests.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,9 +433,9 @@ void Test_CF_CFDP_R2_Complete_Given_t_Sets_send_nak_To_1_Given_ok_to_send_nak_Is
arg_t->state_data.r.sub_state = initial_sub_state;
arg_t->flags.rx.md_recv = 0;

CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = Any_uint8_Except(0);
arg_t->state_data.r.r2.counter.nak = CF_AppData.config_table->nak_limit - 1;
CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = Any_uint8_Except(0);
arg_t->state_data.r.r2.acknak_count = CF_AppData.config_table->nak_limit - 1;

/* Act */
CF_CFDP_R2_Complete(arg_t, arg_ok_to_send_nak);
Expand Down Expand Up @@ -470,9 +470,9 @@ void Test_CF_CFDP_R2_Complete_Given_t_Sets_send_nak_To_1_Given_ok_to_send_nak_Is
arg_t->flags.rx.md_recv = 0;
arg_t->flags.rx.send_nak = 0;

CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = Any_uint8_GreaterThan(1);
arg_t->state_data.r.r2.counter.nak = CF_AppData.config_table->nak_limit - 2;
CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = Any_uint8_GreaterThan(1);
arg_t->state_data.r.r2.acknak_count = CF_AppData.config_table->nak_limit - 2;

/* Act */
CF_CFDP_R2_Complete(arg_t, arg_ok_to_send_nak);
Expand Down Expand Up @@ -511,9 +511,9 @@ void Test_CF_CFDP_R2_Complete_Calls_CF_Chunks_ComputeGaps_Returns_non0_Set_send_

UT_SetDefaultReturnValue(UT_KEY(CF_ChunkList_ComputeGaps), Any_int_Except(0));

CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = Any_uint8_GreaterThan(1);
arg_t->state_data.r.r2.counter.nak = CF_AppData.config_table->nak_limit - 2;
CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = Any_uint8_GreaterThan(1);
arg_t->state_data.r.r2.acknak_count = CF_AppData.config_table->nak_limit - 2;

/* Act */
CF_CFDP_R2_Complete(arg_t, arg_ok_to_send_nak);
Expand Down Expand Up @@ -553,9 +553,9 @@ void Test_CF_CFDP_R2_Complete_Calls_CF_Chunks_ComputeGaps_Returns_non0_Set_send_
UT_SetDefaultReturnValue(UT_KEY(CF_ChunkList_ComputeGaps), 0);
arg_t->flags.rx.eof_recv = 0;

CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = Any_uint8_GreaterThan(1);
arg_t->state_data.r.r2.counter.nak = CF_AppData.config_table->nak_limit - 2;
CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = Any_uint8_GreaterThan(1);
arg_t->state_data.r.r2.acknak_count = CF_AppData.config_table->nak_limit - 2;

/* Act */
CF_CFDP_R2_Complete(arg_t, arg_ok_to_send_nak);
Expand Down Expand Up @@ -596,9 +596,9 @@ void Test_CF_CFDP_R2_Complete_Calls_CF_Chunks_ComputeGaps_Returns_non0_Set_send_
UT_SetDefaultReturnValue(UT_KEY(CF_ChunkList_ComputeGaps), 0);
arg_t->flags.rx.eof_recv = 1;

CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = Any_uint8_GreaterThan(1);
arg_t->state_data.r.r2.counter.nak = CF_AppData.config_table->nak_limit - 2;
CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = Any_uint8_GreaterThan(1);
arg_t->state_data.r.r2.acknak_count = CF_AppData.config_table->nak_limit - 2;

/* Act */
CF_CFDP_R2_Complete(arg_t, arg_ok_to_send_nak);
Expand Down Expand Up @@ -1302,13 +1302,13 @@ void Test_CF_CFDP_R2_SubstateRecvEof_CallTo_CF_CFDP_R_SubstateRecvEof_Returns_R_
CF_History_t dummy_history;
CF_ConfigTable_t dummy_config_table;

arg_t->history = &dummy_history;
arg_t->history->cc = CF_CFDP_ConditionCode_NO_ERROR;
arg_t->flags.rx.md_recv = 0;
arg_t->state_data.r.r2.counter.nak = 0;
CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = UINT8_MAX;
arg_t->state_data.r.sub_state = Any_uint8_Except(CF_RxSubState_FILEDATA);
arg_t->history = &dummy_history;
arg_t->history->cc = CF_CFDP_ConditionCode_NO_ERROR;
arg_t->flags.rx.md_recv = 0;
arg_t->state_data.r.r2.acknak_count = 0;
CF_AppData.config_table = &dummy_config_table;
CF_AppData.config_table->nak_limit = UINT8_MAX;
arg_t->state_data.r.sub_state = Any_uint8_Except(CF_RxSubState_FILEDATA);

/* Act */
CF_CFDP_R2_SubstateRecvEof(arg_t, arg_ph);
Expand Down Expand Up @@ -1555,7 +1555,7 @@ void Test_CF_CFDP_R2_SubstateRecvFileData_t_flags_rx_fd_nak_sent_Is_0_And_t_flag

UT_SetHandlerFunction(UT_KEY(CF_CFDP_RecvFd), Handler_int_ForcedReturnOnly, &forced_return_CF_CFDP_RecvFd);

arg_t->state_data.r.r2.counter.nak = Any_uint8_Except(0);
arg_t->state_data.r.r2.acknak_count = Any_uint8_Except(0);

arg_t->flags.rx.fd_nak_sent = 0;

Expand Down Expand Up @@ -1598,8 +1598,7 @@ void Test_CF_CFDP_R2_SubstateRecvFileData_t_flags_rx_fd_nak_sent_Is_0_And_t_flag
/* Assert */
UtAssert_STUB_COUNT(CF_CFDP_RecvFd, 1);
UtAssert_STUB_COUNT(CF_ChunkListAdd, 1);
UtAssert_True(arg_t->state_data.r.r2.counter.nak == 0, "t->state_data.r.r2.counter.nak is %u and should be 0",
arg_t->state_data.r.r2.counter.nak);
UtAssert_ZERO(arg_t->state_data.r.r2.acknak_count);
/* Assert for CF_CFDP_R_ProcessFd - 3 calls, one in function two in STATIC_CAST */
UtAssert_STUB_COUNT(CF_HeaderSize, 3);
/* Assert for CF_CFDP_R2_Reset via CF_CFDP_R1_Reset */
Expand All @@ -1620,7 +1619,7 @@ void Test_CF_CFDP_R2_SubstateRecvFileData_t_flags_rx_fd_nak_sent_Is_1_Call_CF_CF

UT_SetHandlerFunction(UT_KEY(CF_CFDP_RecvFd), Handler_int_ForcedReturnOnly, &forced_return_CF_CFDP_RecvFd);

arg_t->state_data.r.r2.counter.nak = Any_uint8_Except(0);
arg_t->state_data.r.r2.acknak_count = Any_uint8_Except(0);

arg_t->flags.rx.fd_nak_sent = 1;

Expand Down Expand Up @@ -1666,8 +1665,7 @@ void Test_CF_CFDP_R2_SubstateRecvFileData_t_flags_rx_fd_nak_sent_Is_1_Call_CF_CF
/* Assert */
UtAssert_STUB_COUNT(CF_CFDP_RecvFd, 1);
UtAssert_STUB_COUNT(CF_ChunkListAdd, 1);
UtAssert_True(arg_t->state_data.r.r2.counter.nak == 0, "t->state_data.r.r2.counter.nak is %u and should be 0",
arg_t->state_data.r.r2.counter.nak);
UtAssert_ZERO(arg_t->state_data.r.r2.acknak_count);
/* Assert for CF_CFDP_R_ProcessFd - 3 calls, one in function two in STATIC_CAST */
UtAssert_STUB_COUNT(CF_HeaderSize, 3);
/* Assert for CF_CFDP_R2_Reset via CF_CFDP_R1_Reset */
Expand All @@ -1688,7 +1686,7 @@ void Test_CF_CFDP_R2_SubstateRecvFileData_t_flags_rx_fd_nak_sent_Is_0_And_t_flag

UT_SetHandlerFunction(UT_KEY(CF_CFDP_RecvFd), Handler_int_ForcedReturnOnly, &forced_return_CF_CFDP_RecvFd);

arg_t->state_data.r.r2.counter.nak = Any_uint8_Except(0);
arg_t->state_data.r.r2.acknak_count = Any_uint8_Except(0);

arg_t->flags.rx.fd_nak_sent = 0;

Expand Down Expand Up @@ -1732,8 +1730,7 @@ void Test_CF_CFDP_R2_SubstateRecvFileData_t_flags_rx_fd_nak_sent_Is_0_And_t_flag
UtAssert_STUB_COUNT(CF_CFDP_RecvFd, 1);
UtAssert_STUB_COUNT(CF_ChunkListAdd, 1);
UtAssert_STUB_COUNT(CF_CFDP_ArmAckTimer, 1);
UtAssert_True(arg_t->state_data.r.r2.counter.nak == 0, "t->state_data.r.r2.counter.nak is %u and should be 0",
arg_t->state_data.r.r2.counter.nak);
UtAssert_ZERO(arg_t->state_data.r.r2.acknak_count);
/* Assert for CF_CFDP_R_ProcessFd - 3 calls, one in function two in STATIC_CAST */
UtAssert_STUB_COUNT(CF_HeaderSize, 3);
/* Assert for CF_CFDP_R2_Reset via CF_CFDP_R1_Reset */
Expand Down Expand Up @@ -3206,8 +3203,8 @@ void Test_CF_CFDP_R2_RecvMd_Given_t_flags_rx_md_recv_Is_0_CallTo_CF_CFDP_RecvMd_
UT_SetDataBuffer(UT_KEY(CF_WrappedOpenCreate), &context_CF_WrappedOpenCreate, sizeof(context_CF_WrappedOpenCreate),
false);

arg_t->flags.rx.md_recv = 0;
arg_t->state_data.r.r2.counter.nak = Any_uint8_Except(0);
arg_t->flags.rx.md_recv = 0;
arg_t->state_data.r.r2.acknak_count = Any_uint8_Except(0);

/* Arrange for CF_CFDP_R2_Complete */
arg_t->history->cc = CF_CFDP_ConditionCode_NO_ERROR;
Expand All @@ -3224,8 +3221,7 @@ void Test_CF_CFDP_R2_RecvMd_Given_t_flags_rx_md_recv_Is_0_CallTo_CF_CFDP_RecvMd_
UtAssert_STUB_COUNT(CF_WrappedOpenCreate, 1);
UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 0);
UtAssert_True(arg_t->flags.rx.md_recv == 1, "t->flags.rx.md_recv is %u and should be 1", arg_t->flags.rx.md_recv);
UtAssert_True(arg_t->state_data.r.r2.counter.nak == 0, "t->state_data.r.r2.counter.nak is %u and should be 0",
arg_t->state_data.r.r2.counter.nak);
UtAssert_ZERO(arg_t->state_data.r.r2.acknak_count);
/* Assert for CF_CFDP_R2_Complete */
UtAssert_True(arg_t->state_data.r.sub_state == CF_RxSubState_FILEDATA,
"t->state_data.r.sub_state is %u and should be %u (CF_RxSubState_FILEDATA)",
Expand Down Expand Up @@ -4499,7 +4495,8 @@ void Test_CF_CFDP_R_Tick_NothingElseSet_ack_timer_armed_Is_1_CAllTo_CF_Timer_Exp

arg_t->flags.rx.complete = 1;

arg_t->state_data.r.sub_state = CF_RxSubState_WAIT_FOR_FIN_ACK;
arg_t->state_data.r.sub_state = CF_RxSubState_WAIT_FOR_FIN_ACK;
arg_t->state_data.r.r2.acknak_count = 0;

/* Arrange for CF_CFDP_R2_Complete */
CF_History_t dummy_history;
Expand Down Expand Up @@ -4560,7 +4557,7 @@ void Test_CF_CFDP_R_Tick_NothingElseSet_ack_timer_armed_Is_1_CAllTo_CF_Timer_Exp

CF_AppData.config_table->ack_limit = Any_uint8_Except(0); /* Any_uint8 used for small value in test */

arg_t->state_data.r.r2.counter.ack = CF_AppData.config_table->ack_limit - 1; /* - 1 code increases value */
arg_t->state_data.r.r2.acknak_count = CF_AppData.config_table->ack_limit - 1; /* - 1 code increases value */

UT_SetDataBuffer(UT_KEY(CFE_EVS_SendEvent), &EventID, sizeof(EventID), false);
arg_t->history = &dummy_history;
Expand Down
18 changes: 10 additions & 8 deletions unit-test/cf_cfdp_s_tests.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1303,8 +1303,10 @@ void Test_CF_CFDP_S_CheckAndRespondNak_CallTo_CF_Chunks_GetFirstChunkReturned_no
CF_CFDP_SendFd_context_t context_CF_CFDP_SendFd;

dummy_c->offset = 0; /* dummy_c->offset = 0 used so that (foffs+bytes_to_read)<=t->fsize is never false */
arg_t->fsize = 512;
arg_t->foffs = 0;

dummy_config_table.outgoing_file_chunk_size = arg_t->foffs;
dummy_config_table.outgoing_file_chunk_size = dummy_c->size;
CF_AppData.config_table = &dummy_config_table;
arg_t->history = &dummy_history;

Expand Down Expand Up @@ -3470,7 +3472,7 @@ void Test_CF_CFDP_S_Tick_ArmedTimerExpiredAnd_sub_state_EqTo_SEND_WAIT_FOR_EOF_A
arg_t->flags.com.ack_timer_armed = 1;
arg_t->state_data.s.sub_state = CF_TxSubState_WAIT_FOR_EOF_ACK;

arg_t->state_data.s.s2.counter.ack = dummy_ack_limit - 1;
arg_t->state_data.s.s2.acknak_count = dummy_ack_limit - 1;

CF_AppData.config_table = &dummy_config_table;
CF_AppData.hk.channel_hk[arg_t->chan_num].counters.fault.inactivity_timer = initial_inactivity_timer;
Expand Down Expand Up @@ -3540,7 +3542,7 @@ void Test_CF_CFDP_S_Tick_ArmedTimerExpiredAnd_sub_state_EqTo_SEND_WAIT_FOR_EOF_A
int *arg_cont = NULL;
uint16 initial_inactivity_timer = Any_uint16();
uint16 initial_fault_ack_limit = Any_uint16();
uint8 dummy_ack_limit = Any_uint8();
uint8 dummy_ack_limit = 5;
CF_Timer_t *context_CF_Timer_Expired[2];
CF_Timer_t *context_CF_Timer_Tick;

Expand All @@ -3550,7 +3552,7 @@ void Test_CF_CFDP_S_Tick_ArmedTimerExpiredAnd_sub_state_EqTo_SEND_WAIT_FOR_EOF_A
arg_t->flags.com.ack_timer_armed = 1;
arg_t->state_data.s.sub_state = CF_TxSubState_WAIT_FOR_EOF_ACK;

arg_t->state_data.s.s2.counter.ack = Any_uint8_Except(dummy_ack_limit - 1);
arg_t->state_data.s.s2.acknak_count = 0;

CF_AppData.config_table = &dummy_config_table;
CF_AppData.hk.channel_hk[arg_t->chan_num].counters.fault.inactivity_timer = initial_inactivity_timer;
Expand Down Expand Up @@ -3610,7 +3612,7 @@ void Test_CF_CFDP_S_Tick_ArmedTimerExpiredAnd_sub_state_EqTo_SEND_WAIT_FOR_EOF_A
int *arg_cont = NULL;
uint16 initial_inactivity_timer = Any_uint16();
uint16 initial_fault_ack_limit = Any_uint16();
uint8 dummy_ack_limit = Any_uint8();
uint8 dummy_ack_limit = 10;
CF_Timer_t *context_CF_Timer_Expired[2];
CF_Timer_t *context_CF_Timer_Tick;

Expand All @@ -3620,7 +3622,7 @@ void Test_CF_CFDP_S_Tick_ArmedTimerExpiredAnd_sub_state_EqTo_SEND_WAIT_FOR_EOF_A
arg_t->flags.com.ack_timer_armed = 1;
arg_t->state_data.s.sub_state = CF_TxSubState_WAIT_FOR_EOF_ACK;

arg_t->state_data.s.s2.counter.ack = Any_uint8_Except(dummy_ack_limit - 1);
arg_t->state_data.s.s2.acknak_count = 5; /* must remain less than ack_limit after increment */

CF_AppData.config_table = &dummy_config_table;
CF_AppData.hk.channel_hk[arg_t->chan_num].counters.fault.inactivity_timer = initial_inactivity_timer;
Expand Down Expand Up @@ -3690,7 +3692,7 @@ void Test_CF_CFDP_S_Tick_ArmedTimerExpiredAnd_sub_state_EqTo_SEND_WAIT_FOR_EOF_A
int *arg_cont = NULL;
uint16 initial_inactivity_timer = Any_uint16();
uint16 initial_fault_ack_limit = Any_uint16();
uint8 dummy_ack_limit = Any_uint8();
uint8 dummy_ack_limit = 10;
CF_Timer_t *context_CF_Timer_Expired[2];
CF_Timer_t *context_CF_Timer_Tick;
CF_Transaction_t *context_CF_CFDP_ArmAckTimer;
Expand All @@ -3701,7 +3703,7 @@ void Test_CF_CFDP_S_Tick_ArmedTimerExpiredAnd_sub_state_EqTo_SEND_WAIT_FOR_EOF_A
arg_t->flags.com.ack_timer_armed = 1;
arg_t->state_data.s.sub_state = CF_TxSubState_WAIT_FOR_EOF_ACK;

arg_t->state_data.s.s2.counter.ack = Any_uint8_Except(dummy_ack_limit - 1);
arg_t->state_data.s.s2.acknak_count = 5; /* must remain less than ack_limit after increment */

CF_AppData.config_table = &dummy_config_table;
CF_AppData.hk.channel_hk[arg_t->chan_num].counters.fault.inactivity_timer = initial_inactivity_timer;
Expand Down

0 comments on commit a0d6f56

Please sign in to comment.