Skip to content

Conversation

@Willmac16
Copy link
Contributor

@Willmac16 Willmac16 commented Aug 27, 2025

Related Issue(s) N/A
Has Unit Tests (y/n) y
Documentation Included (y/n) y
Generative AI was used in this contribution (y/n) n

Change Description

Modifies DpCatalog to allow for runtime insertion of data products onto the queue through the DpWriter notifier port

Rationale

Long running missions with the need for automated file downlink would benefit from simultaneous catalog transmission and construction.

Testing/Review Recommendations

Progress towards Draft Completion

  • Refactor file addition into separate function
  • Call into processFile from addToCatalog port
  • Allow adding to catalog while Transmiting
  • Add UT coverage for addToCatalog port
  • Fix running the 2 recently enabled UTs back to back
  • Fix/Remove that final DISABLED unit test
  • Round out coverage

Nice to Haves

  • Clean all artifacts run to run
  • Catalog Navigation w/o the stack
    • would make auto-balancing easier
    • reduces memory req
    • probably doable w/ a current node + traversal direction (up/down) since I've added the parent link
  • Self balancing (prob AVL or Red-Black)
    • Seems prudent given how carved up the tree will be with simultaneous additions and removals
  • Random Sequence order validation @ the addToCat -> fileOut level

AI Usage (see policy)

N/A

@Willmac16 Willmac16 force-pushed the Willmac16/dp-cat-runtime-insertion branch 2 times, most recently from 189a5a8 to 90958a1 Compare August 27, 2025 22:27
this->log_WARNING_HI_NoDpMemory();
return;
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a check to see if the catalog has already been loaded. If it hasn't, just drop this call, since the new file will be noticed when the catalog is loaded.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Willmac16 Where is this checked an dropped?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I added a check at one point for the dpTree pointer being non-null; however, I later removed that check since nodes could be added after tree completion (and pruning) with the remainActive arg. I'm adding a state file initialization check, since that is the primary thing a runtime added file needs from the catalog build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am now using a catalogBuilt flag since you can't guarantee the state file that is loaded will have any items.

@timcanham
Copy link
Collaborator

Looks good so far!

@Willmac16 Willmac16 force-pushed the Willmac16/dp-cat-runtime-insertion branch 2 times, most recently from d15405c to b21bc30 Compare September 9, 2025 19:48
@Willmac16
Copy link
Contributor Author

Do you have a recommendation for a cmake way to clean up the test artifacts consistently regardless of if the test passes?

@Willmac16 Willmac16 force-pushed the Willmac16/dp-cat-runtime-insertion branch from 7e89f01 to 01fd6b1 Compare November 2, 2025 23:54
@Willmac16 Willmac16 force-pushed the Willmac16/dp-cat-runtime-insertion branch from 01fd6b1 to 9fd9624 Compare November 2, 2025 23:57
Comment on lines +104 to +124
for (FwSizeType ii = 0; ii <= (source_size - sub_size); ii++) {
const FwSizeType source_index = (source_size - sub_size) - ii;

// if the current character matches
for (FwSizeType sub_index = 0; sub_index < sub_size; sub_index++) {
// Prevent read overrun
FW_ASSERT((source_index + sub_index) < source_size);
// if there is a mismatch, go to next character
if (source_string[source_index + sub_index] != sub_string[sub_index]) {
break;
} else if (sub_index == (sub_size - 1)) {
// if we matched all the way to the end of the substring
match_index = source_index;

// Ensure the result converts properly
FW_ASSERT(static_cast<FwSizeType>(static_cast<FwSignedSizeType>(match_index)) == match_index);

return static_cast<FwSignedSizeType>(match_index);
}
}
}

Check warning

Code scanning / CodeQL

Unbounded loop Warning

This loop does not have a fixed bound.
// Grab the directory string (up until the final slash)
// Could be found directly w/ a dirname func or regex
FwSignedSizeType loc = Fw::StringUtils::substring_find_last(
fullFile.toChar(), fullFile.length(), DIRECTORY_DELIMITER,

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter fullFile has not been checked.
Fw::Buffer hdrBuff(dpBuff, sizeof(dpBuff)); // buffer for container header decoding
Fw::DpContainer container; // container object for extracting header fields

this->log_ACTIVITY_LO_ProcessingFile(fullFile);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter fullFile has not been checked.

// add entry to catalog.
DpStateEntry entry;
entry.dir = static_cast<FwIndexType>(dir);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter dir has not been checked.
}

Fw::FileNameString addedFileName;
addedFileName.format(DP_FILENAME_FORMAT, this->m_directories[dir].toChar(), entry.record.get_id(),

Check warning

Code scanning / CodeQL

Unchecked return value Warning

The return value of non-void function
format
is not checked.

// Check the catalog has been built
if (not this->m_catalogBuilt) {
this->log_ACTIVITY_HI_NotLoaded(fileName);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter fileName has not been checked.
}

// Both of these are grabbed from the header
(void)priority;

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter priority has not been checked.

// Both of these are grabbed from the header
(void)priority;
(void)size;

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter size has not been checked.

// Since this is a runtime addition
// Check if file is in one of our directories
FwSizeType dir = this->determineDirectory(fileName);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter fileName has not been checked.
void DpCatalog ::START_XMIT_CATALOG_cmdHandler(FwOpcodeType opCode, U32 cmdSeq, Fw::Wait wait) {
void DpCatalog ::START_XMIT_CATALOG_cmdHandler(FwOpcodeType opCode, U32 cmdSeq, Fw::Wait wait, bool remainActive) {
Fw::CmdResponse resp = this->doCatalogXmit();
this->m_remainActive = remainActive;

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter remainActive has not been checked.
// if we make it here, no matches were found
return -1;
}

Check warning

Code scanning / CppCheck

Could not find a newline character at the end of the file. Warning

Could not find a newline character at the end of the file.
return -1;
}

FwSignedSizeType Fw::StringUtils::substring_find_last(const CHAR* source_string,

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
}

void DpCatalog ::START_XMIT_CATALOG_cmdHandler(FwOpcodeType opCode, U32 cmdSeq, Fw::Wait wait) {
void DpCatalog ::START_XMIT_CATALOG_cmdHandler(FwOpcodeType opCode, U32 cmdSeq, Fw::Wait wait, bool remainActive) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
m_xmitCmdSeq(0),
m_pendingFiles(0),
m_pendingDpBytes(0),
m_remainActive(false) {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.
} // end fillBinaryTree()

bool DpCatalog::insertEntry(DpStateEntry& entry) {
FwSizeType DpCatalog::determineDirectory(Fw::String fullFile) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
this->pingOut_out(0, key);
}

void DpCatalog ::addToCat_handler(FwIndexType portNum,

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
return DP_MAX_DIRECTORIES;
}

int DpCatalog::processFile(Fw::String fullFile, FwSizeType dir) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
}

void DpCatalog::deleteEntry(DpStateEntry& entry) {}
void DpCatalog::deallocateNode(DpBtreeNode* node) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
}

void DpCatalog::deleteEntry(DpStateEntry& entry) {}
void DpCatalog::deallocateNode(DpBtreeNode* node) {

Check notice

Code scanning / CodeQL

Function too long Note

deallocateNode has too many lines (127, while 60 are allowed).
return compareEntries(*this, other) < 0;
}

DpCatalog::DpBtreeNode* DpCatalog::insertEntry(DpStateEntry& entry) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
// ----------------------------------------------------------------------
// DpStateEntry Comparison Ops
// ----------------------------------------------------------------------
int DpCatalog::DpStateEntry::compareEntries(const DpStateEntry& left, const DpStateEntry& right) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
}

// ret > 0 := success
int ret = processFile(fileName, dir);

Check notice

Code scanning / CodeQL

Use of basic integral type Note

ret uses the basic integral type int rather than a typedef with size and signedness.
return -1;
}

FwSignedSizeType Fw::StringUtils::substring_find_last(const CHAR* source_string,

Check notice

Code scanning / CodeQL

Use of basic integral type Note

source_string uses the basic integral type char rather than a typedef with size and signedness.

FwSignedSizeType Fw::StringUtils::substring_find_last(const CHAR* source_string,
FwSizeType source_size,
const CHAR* sub_string,

Check notice

Code scanning / CodeQL

Use of basic integral type Note

sub_string uses the basic integral type char rather than a typedef with size and signedness.
// make sure we haven't exceeded the limit
if (this->m_numDpRecords > this->m_numDpSlots) {
this->log_WARNING_HI_DpCatalogFull(entry.record);
int ret = processFile(fullFile, dir);

Check notice

Code scanning / CodeQL

Use of basic integral type Note

ret uses the basic integral type int rather than a typedef with size and signedness.
return DP_MAX_DIRECTORIES;
}

int DpCatalog::processFile(Fw::String fullFile, FwSizeType dir) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

processFile uses the basic integral type int rather than a typedef with size and signedness.
// ----------------------------------------------------------------------
// DpStateEntry Comparison Ops
// ----------------------------------------------------------------------
int DpCatalog::DpStateEntry::compareEntries(const DpStateEntry& left, const DpStateEntry& right) {

Check notice

Code scanning / CodeQL

Use of basic integral type Note

compareEntries uses the basic integral type int rather than a typedef with size and signedness.
} // end fillBinaryTree()

bool DpCatalog::insertEntry(DpStateEntry& entry) {
FwSizeType DpCatalog::determineDirectory(Fw::String fullFile) {

Check warning

Code scanning / CppCheck

Parameter 'fullFile' is passed by value. It could be passed as a const reference which is usually faster and recommended in C++. Warning

Parameter 'fullFile' is passed by value. It could be passed as a const reference which is usually faster and recommended in C++.
return DP_MAX_DIRECTORIES;
}

int DpCatalog::processFile(Fw::String fullFile, FwSizeType dir) {

Check warning

Code scanning / CppCheck

Parameter 'fullFile' is passed by value. It could be passed as a const reference which is usually faster and recommended in C++. Warning

Parameter 'fullFile' is passed by value. It could be passed as a const reference which is usually faster and recommended in C++.
@thomas-bc thomas-bc added the Deferred To Next Release This PR is marked as deferred until after the upcoming release. label Nov 12, 2025
@Willmac16
Copy link
Contributor Author

I am finally ready for re-review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Deferred To Next Release This PR is marked as deferred until after the upcoming release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants