From b357f170e7d46fd9bc47b0872b837a891443e973 Mon Sep 17 00:00:00 2001 From: Anh D Van Date: Thu, 16 May 2024 18:19:09 +0000 Subject: [PATCH] Fix #441, cf delete files during error when tx Fix #441, Update to handle error during tx. For polling, the file gets moved or deleted to pervent infinite loop. For others, the file does not get deleted. Update Unit Test and documentation. --- config/default_cf_tblstruct.h | 1 + docs/dox_src/cfs_cf.dox | 31 +++++--- fsw/src/cf_cfdp.c | 126 +++++++++++++++++++++++++------- fsw/src/cf_cfdp.h | 30 ++++++++ fsw/tables/cf_def_config.c | 1 + unit-test/cf_cfdp_tests.c | 86 ++++++++++++++++++++++ unit-test/stubs/cf_cfdp_stubs.c | 42 +++++++++++ 7 files changed, 280 insertions(+), 37 deletions(-) diff --git a/config/default_cf_tblstruct.h b/config/default_cf_tblstruct.h index 30df4779..9ed1a33e 100644 --- a/config/default_cf_tblstruct.h +++ b/config/default_cf_tblstruct.h @@ -51,6 +51,7 @@ typedef struct CF_ConfigTable uint16 outgoing_file_chunk_size; /**< \brief maximum size of outgoing file data chunk in a PDU. * Limited by CF_MAX_PDU_SIZE minus the PDU header(s) */ char tmp_dir[CF_FILENAME_MAX_PATH]; /**< \brief directory to put temp files */ + char fail_dir[CF_FILENAME_MAX_PATH]; /**< \brief fail directory */ } CF_ConfigTable_t; #endif diff --git a/docs/dox_src/cfs_cf.dox b/docs/dox_src/cfs_cf.dox index f5022144..3778eb44 100644 --- a/docs/dox_src/cfs_cf.dox +++ b/docs/dox_src/cfs_cf.dox @@ -398,16 +398,24 @@ When files are found in a polling directory that is enabled, the files are automatically queued by CF for output. The polling rate is configurable and each channel has a configurable number of polling directories. Each polling - directory has an enable, a class setting, a priority setting, a preserve - setting and a destination directory. When enabled, CF will periodically check - the polling directories for files. When a file is found, CF will place the - file information on the pending queue if it is closed and not already on the - queue and not currently active. All polling directories are checked at the - same frequency which is defined by the table parameter - NumWakeupsPerPollDirChk. Setting this parameter to one will cause CF to check - the polling directories at the fastest possible rate, every time it receives a - 'wake-up' command. Checking polling directories is a processor-intensive - effort, it is best to keep the polling rate as low as possible. + directory has an enable, a class setting, a priority setting, a destination + directory. When enabled, CF will periodically check the polling directories + for files. When a file is found, CF will place the file information on the + pending queue if it is closed and not already on the queue and not currently + active. All polling directories are checked at the same frequency which is + defined by the table parameter NumWakeupsPerPollDirChk. Setting this parameter + to one will cause CF to check the polling directories at the fastest possible + rate, every time it receives a 'wake-up' command. Checking polling directories + is a processor-intensive effort, it is best to keep the polling rate as low as + possible. + +

Failed Transmit and directory use

+ + Currently, the fail directory is used only to store class 2 TX files that failed + during transmission from a polling directory. When a file fails during transmission + from a polling directory, it is deleted from the polling directory to prevent an + infinite loop and moved to the fail directory. However, if a file fails during a + class 2 TX outside of a polling directory, the file will not be deleted.

Efficiency

@@ -645,7 +653,8 @@ CF_UnionArgs_Payload_t; and an event will be generated. If the command is not successful, the command error counter will increment and - an error event will be generated indicating the reason for failure. + an error event will be generated indicating the reason for failure. File will + not be deleted if the 'transmit' failed (unless in an polling directories). \verbatim typedef struct CF_TxFileCmd diff --git a/fsw/src/cf_cfdp.c b/fsw/src/cf_cfdp.c index 6f31fea1..f30fb4b1 100644 --- a/fsw/src/cf_cfdp.c +++ b/fsw/src/cf_cfdp.c @@ -1588,9 +1588,7 @@ void CF_CFDP_CycleEngine(void) *-----------------------------------------------------------------*/ void CF_CFDP_ResetTransaction(CF_Transaction_t *txn, int keep_history) { - char * filename; - char destination[OS_MAX_PATH_LEN]; - osal_status_t status = OS_ERROR; + CF_Channel_t *chan = &CF_AppData.engine.channels[txn->chan_num]; CF_Assert(txn->chan_num < CF_NUM_CHANNELS); @@ -1608,31 +1606,10 @@ void CF_CFDP_ResetTransaction(CF_Transaction_t *txn, int keep_history) if (OS_ObjectIdDefined(txn->fd)) { CF_WrappedClose(txn->fd); + if (!txn->keep) { - if (CF_CFDP_IsSender(txn)) - { - /* If move directory is defined attempt move */ - if (CF_AppData.config_table->chan[txn->chan_num].move_dir[0] != 0) - { - filename = strrchr(txn->history->fnames.src_filename, '/'); - if (filename != NULL) - { - snprintf(destination, sizeof(destination), "%s%s", - CF_AppData.config_table->chan[txn->chan_num].move_dir, filename); - status = OS_mv(txn->history->fnames.src_filename, destination); - } - } - - if (status != OS_SUCCESS) - { - OS_remove(txn->history->fnames.src_filename); - } - } - else - { - OS_remove(txn->history->fnames.dst_filename); - } + CF_CFDP_HandleNotKeepFile(txn); } } @@ -1829,3 +1806,100 @@ void CF_CFDP_DisableEngine(void) CFE_SB_DeletePipe(chan->pipe); } } + +/*---------------------------------------------------------------- + * + * Application-scope internal function + * See description in cf_cfdp.h for argument/return detail + * + *-----------------------------------------------------------------*/ +bool CF_CFDP_IsPollingDir(const char * src_file, uint8 chan_num) +{ + bool return_code = false; + char src_dir[CF_FILENAME_MAX_LEN] = "\0"; + CF_ChannelConfig_t *cc; + CF_PollDir_t * pd; + int i; + + char * last_slash = strrchr(src_file, '/'); + if(last_slash != NULL) + { + strncpy(src_dir, src_file, last_slash - src_file); + } + + cc = &CF_AppData.config_table->chan[chan_num]; + for (i = 0; i < CF_MAX_POLLING_DIR_PER_CHAN; ++i) + { + pd = &cc->polldir[i]; + if(strcmp(src_dir, pd->src_dir) == 0) + { + return_code = true; + break; + } + } + + return return_code; +} + +/*---------------------------------------------------------------- + * + * Application-scope internal function + * See description in cf_cfdp.h for argument/return detail + * + *-----------------------------------------------------------------*/ +void CF_CFDP_HandleNotKeepFile(CF_Transaction_t *txn) +{ + /* Sender */ + if (CF_CFDP_IsSender(txn)) + { + if(!CF_TxnStatus_IsError(txn->history->txn_stat)) + { + /* If move directory is defined attempt move */ + CF_CFDP_MoveFile(txn->history->fnames.src_filename, CF_AppData.config_table->chan[txn->chan_num].move_dir); + } + else + { + /* file inside an polling directory */ + if(CF_CFDP_IsPollingDir(txn->history->fnames.src_filename, txn->chan_num)) + { + /* If fail directory is defined attempt move */ + CF_CFDP_MoveFile(txn->history->fnames.src_filename, CF_AppData.config_table->fail_dir); + } + } + } + /* Not Sender */ + else + { + OS_remove(txn->history->fnames.dst_filename); + } +} + + +/*---------------------------------------------------------------- + * + * Application-scope internal function + * See description in cf_cfdp.h for argument/return detail + * + *-----------------------------------------------------------------*/ +void CF_CFDP_MoveFile(const char *src, const char *dest_dir) +{ + osal_status_t status = OS_ERROR; + char * filename; + char destination[OS_MAX_PATH_LEN]; + + if(dest_dir[0] != 0) + { + filename = strrchr(src, '/'); + if (filename != NULL) + { + snprintf(destination, sizeof(destination), "%s%s", + dest_dir, filename); + status = OS_mv(src, destination); + } + } + + if (status != OS_SUCCESS) + { + OS_remove(src); + } +} \ No newline at end of file diff --git a/fsw/src/cf_cfdp.h b/fsw/src/cf_cfdp.h index 1fa54db0..c1c04870 100644 --- a/fsw/src/cf_cfdp.h +++ b/fsw/src/cf_cfdp.h @@ -703,4 +703,34 @@ void CF_CFDP_ProcessPollingDirectories(CF_Channel_t *chan); */ CF_CListTraverse_Status_t CF_CFDP_DoTick(CF_CListNode_t *node, void *context); +/************************************************************************/ +/** @brief Check if source file came from polling directory + * + * + * @par Assumptions, External Events, and Notes: + * + * @retval true/false + */ +bool CF_CFDP_IsPollingDir(const char * src_file, uint8 chan_num); + +/************************************************************************/ +/** @brief Remove/Move file after transaction + * + * This helper is used to handle "not keep" file option after a transaction. + * + * @par Assumptions, External Events, and Notes: + * + */ +void CF_CFDP_HandleNotKeepFile(CF_Transaction_t *txn); + +/************************************************************************/ +/** @brief Move File + * + * This helper is used to move a file. + * + * @par Assumptions, External Events, and Notes: + * + */ +void CF_CFDP_MoveFile(const char *src, const char *dest_dir); + #endif /* !CF_CFDP_H */ diff --git a/fsw/tables/cf_def_config.c b/fsw/tables/cf_def_config.c index 35bba669..425666ff 100644 --- a/fsw/tables/cf_def_config.c +++ b/fsw/tables/cf_def_config.c @@ -80,5 +80,6 @@ CF_ConfigTable_t CF_config_table = { .move_dir = ""}}, 480, /* outgoing_file_chunk_size */ "/cf/tmp", /* temporary file directory */ + "/cf/fail", /* Stores failed tx file for "polling directory" */ }; CFE_TBL_FILEDEF(CF_config_table, CF.config_table, CF config table, cf_def_config.tbl) diff --git a/unit-test/cf_cfdp_tests.c b/unit-test/cf_cfdp_tests.c index 3661d89f..d0c13f10 100644 --- a/unit-test/cf_cfdp_tests.c +++ b/unit-test/cf_cfdp_tests.c @@ -1357,6 +1357,7 @@ void Test_CF_CFDP_ResetTransaction(void) CF_History_t * history; CF_Channel_t * chan; CF_Playback_t pb; + CF_ConfigTable_t *config; memset(&pb, 0, sizeof(pb)); @@ -1443,6 +1444,91 @@ void Test_CF_CFDP_ResetTransaction(void) UtAssert_UINT32_EQ(pb.num_ts, 9); UtAssert_UINT32_EQ(chan->num_cmd_tx, 7); UtAssert_STUB_COUNT(CF_FreeTransaction, 1); + + /* + * File is in Polling Directory, Not Keep, and is Error + * Move to fail directory successful + */ + UT_ResetState(UT_KEY(CF_FreeTransaction)); + UT_ResetState(UT_KEY(OS_remove)); + UT_ResetState(UT_KEY(OS_mv)); + UT_CFDP_SetupBasicTestState(UT_CF_Setup_TX, NULL, &chan, &history, &txn, &config); + UT_SetDefaultReturnValue(UT_KEY(CF_TxnStatus_IsError), true);\ + UT_SetDefaultReturnValue(UT_KEY(OS_mv), OS_SUCCESS); + txn->fd = OS_ObjectIdFromInteger(1); + txn->keep = 0; + txn->state = CF_TxnState_S2; + strcpy(history->fnames.src_filename, "/ram/poll1/test1"); + strcpy(config->chan[0].polldir[0].src_dir, "/ram/poll1"); + strcpy(config->fail_dir, "/ram/fail"); + + UtAssert_VOIDCALL(CF_CFDP_ResetTransaction(txn, 0)); + UtAssert_STUB_COUNT(CF_FreeTransaction, 1); + UtAssert_STUB_COUNT(OS_mv, 1); + UtAssert_STUB_COUNT(OS_remove, 0); + + /* + * File is in Polling Directory, Not Keep, and is Error + * Move to fail directory not successful + */ + UT_ResetState(UT_KEY(CF_FreeTransaction)); + UT_ResetState(UT_KEY(OS_remove)); + UT_ResetState(UT_KEY(OS_mv)); + UT_CFDP_SetupBasicTestState(UT_CF_Setup_TX, NULL, &chan, &history, &txn, &config); + UT_SetDefaultReturnValue(UT_KEY(CF_TxnStatus_IsError), true); + UT_SetDefaultReturnValue(UT_KEY(OS_mv), OS_ERROR); + txn->fd = OS_ObjectIdFromInteger(1); + txn->keep = 0; + txn->state = CF_TxnState_S2; + strcpy(history->fnames.src_filename, "/ram/poll1/test1"); + strcpy(config->chan[0].polldir[0].src_dir, "/ram/poll1"); + strcpy(config->fail_dir, "/ram/fail"); + + UtAssert_VOIDCALL(CF_CFDP_ResetTransaction(txn, 0)); + UtAssert_STUB_COUNT(CF_FreeTransaction, 1); + UtAssert_STUB_COUNT(OS_mv, 1); + UtAssert_STUB_COUNT(OS_remove, 1); + + /* + * Source file outside polling directory, Not Keep, is Error + */ + UT_ResetState(UT_KEY(CF_FreeTransaction)); + UT_ResetState(UT_KEY(OS_remove)); + UT_ResetState(UT_KEY(OS_mv)); + UT_CFDP_SetupBasicTestState(UT_CF_Setup_TX, NULL, &chan, &history, &txn, &config); + UT_SetDefaultReturnValue(UT_KEY(CF_TxnStatus_IsError), true); + txn->fd = OS_ObjectIdFromInteger(1); + txn->keep = 0; + txn->state = CF_TxnState_S2; + strcpy(history->fnames.src_filename, "/ram/test.txt"); + strcpy(config->chan[0].polldir[0].src_dir, "/ram/poll1"); + strcpy(config->fail_dir, "/ram/fail"); + + UtAssert_VOIDCALL(CF_CFDP_ResetTransaction(txn, 0)); + UtAssert_STUB_COUNT(CF_FreeTransaction, 1); + UtAssert_STUB_COUNT(OS_mv, 0); + UtAssert_STUB_COUNT(OS_remove, 0); + + /* + * Source File is empty string. Not Keep, is Error + */ + UT_ResetState(UT_KEY(CF_FreeTransaction)); + UT_ResetState(UT_KEY(OS_remove)); + UT_ResetState(UT_KEY(OS_mv)); + UT_CFDP_SetupBasicTestState(UT_CF_Setup_TX, NULL, &chan, &history, &txn, &config); + UT_SetDefaultReturnValue(UT_KEY(CF_TxnStatus_IsError), true); + txn->fd = OS_ObjectIdFromInteger(1); + txn->keep = 0; + txn->state = CF_TxnState_S2; + strcpy(history->fnames.src_filename, ""); + strcpy(config->chan[0].polldir[0].src_dir, "/ram/poll1"); + strcpy(config->fail_dir, "/ram/fail"); + + UtAssert_VOIDCALL(CF_CFDP_ResetTransaction(txn, 0)); + UtAssert_STUB_COUNT(CF_FreeTransaction, 1); + UtAssert_STUB_COUNT(OS_mv, 0); + UtAssert_STUB_COUNT(OS_remove, 1); + } void Test_CF_CFDP_SetTxnStatus(void) diff --git a/unit-test/stubs/cf_cfdp_stubs.c b/unit-test/stubs/cf_cfdp_stubs.c index 3db3c393..c3f844ab 100644 --- a/unit-test/stubs/cf_cfdp_stubs.c +++ b/unit-test/stubs/cf_cfdp_stubs.c @@ -243,6 +243,18 @@ void CF_CFDP_EncodeStart(CF_EncoderState_t *penc, void *msgbuf, CF_Logical_PduBu UT_GenStub_Execute(CF_CFDP_EncodeStart, Basic, NULL); } +/* + * ---------------------------------------------------- + * Generated stub function for CF_CFDP_HandleNotKeepFile() + * ---------------------------------------------------- + */ +void CF_CFDP_HandleNotKeepFile(CF_Transaction_t *txn) +{ + UT_GenStub_AddParam(CF_CFDP_HandleNotKeepFile, CF_Transaction_t *, txn); + + UT_GenStub_Execute(CF_CFDP_HandleNotKeepFile, Basic, NULL); +} + /* * ---------------------------------------------------- * Generated stub function for CF_CFDP_InitEngine() @@ -273,6 +285,36 @@ void CF_CFDP_InitTxnTxFile(CF_Transaction_t *txn, CF_CFDP_Class_t cfdp_class, ui UT_GenStub_Execute(CF_CFDP_InitTxnTxFile, Basic, NULL); } +/* + * ---------------------------------------------------- + * Generated stub function for CF_CFDP_IsPollingDir() + * ---------------------------------------------------- + */ +bool CF_CFDP_IsPollingDir(const char *src_file, uint8 chan_num) +{ + UT_GenStub_SetupReturnBuffer(CF_CFDP_IsPollingDir, bool); + + UT_GenStub_AddParam(CF_CFDP_IsPollingDir, const char *, src_file); + UT_GenStub_AddParam(CF_CFDP_IsPollingDir, uint8, chan_num); + + UT_GenStub_Execute(CF_CFDP_IsPollingDir, Basic, NULL); + + return UT_GenStub_GetReturnValue(CF_CFDP_IsPollingDir, bool); +} + +/* + * ---------------------------------------------------- + * Generated stub function for CF_CFDP_MoveFile() + * ---------------------------------------------------- + */ +void CF_CFDP_MoveFile(const char *src, const char *dest_dir) +{ + UT_GenStub_AddParam(CF_CFDP_MoveFile, const char *, src); + UT_GenStub_AddParam(CF_CFDP_MoveFile, const char *, dest_dir); + + UT_GenStub_Execute(CF_CFDP_MoveFile, Basic, NULL); +} + /* * ---------------------------------------------------- * Generated stub function for CF_CFDP_PlaybackDir()