Add Svc.FileDispatcher component #4552
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
LeStarch
left a comment
LeStarch
left a comment
There was a problem hiding this comment.
I really think you need 2 data structures here:
- Mapping for EXT to port
- List of ports to disable/enable
Right now the code looks like you are trying to separate the concepts, but cannot because of the singular data structure.
Either remove "port" from the data structure, make enable/disable a loop through all table entrys matching on port type, or split them up.
thomas-bc
changed the title
LeStarch
left a comment
LeStarch
left a comment
There was a problem hiding this comment.
I need to think a bit on the copy, and I need to evaluate the CodeQL warnings.
kevin-f-ortega
left a comment
kevin-f-ortega
left a comment
There was a problem hiding this comment.
Looks good, nice work! A few comments
|
|
||
| FileDispatcher ::~FileDispatcher() {} | ||
|
|
||
| void FileDispatcher ::configure(const FileDispatcherTable& table) { |
Check notice
Code scanning / CodeQL
Long function without assertion Note
| // Handler implementations for commands | ||
| // ---------------------------------------------------------------------- | ||
|
|
||
| void FileDispatcher ::ENABLE_DISPATCH_cmdHandler(FwOpcodeType opCode, |
Check notice
Code scanning / CodeQL
Long function without assertion Note
| FW_ASSERT(table.entries[entry].fileExt.length() > 0, | ||
| static_cast<FwAssertArgType>(table.entries[entry].fileExt.length())); // non-zero length | ||
| // Copy over table entry | ||
| this->m_dispatchTable.entries[entry].port = table.entries[entry].port; |
Check warning
Code scanning / CodeQL
Unchecked return value Warning
| static_cast<FwAssertArgType>(table.entries[entry].fileExt.length())); // non-zero length | ||
| // Copy over table entry | ||
| this->m_dispatchTable.entries[entry].port = table.entries[entry].port; | ||
| this->m_dispatchTable.entries[entry].fileExt = table.entries[entry].fileExt; |
Check warning
Code scanning / CodeQL
Unchecked return value Warning
| this->m_dispatchTable.entries[i].enabled = enable; | ||
| } | ||
| } | ||
| this->log_ACTIVITY_HI_FileDispatchState(file_type, enable); |
Check warning
Code scanning / CodeQL
Unchecked function argument Warning
| this->m_dispatchTable.entries[i].enabled = enable; | ||
| } | ||
| } | ||
| this->log_ACTIVITY_HI_FileDispatchState(file_type, enable); |
Check warning
Code scanning / CodeQL
Unchecked function argument Warning
|
|
||
| for (FwSizeType i = 0; i < this->m_dispatchTable.numEntries; i++) { | ||
| if (this->m_dispatchTable.entries[i].port == file_type) { | ||
| this->m_dispatchTable.entries[i].enabled = enable; |
There was a problem hiding this comment.
This should be this->m_dispatchTable.entries[i].enabled = (enable == Fw::Enabled::ENABLED);
kevin-f-ortega
left a comment
kevin-f-ortega
left a comment
There was a problem hiding this comment.
Looks good, just one minor comment
3 participants
Change Description
Adds a new Svc/FileDispatcher component for dispatching newly uplinked files
Rationale
Provides automation for automatically handling files like sequences.
Testing/Review Recommendations
Unit tests are provided which test some entries
Future Work
#4444 - this issue will add a port to Svc/CmdSequencer to automatically run sequences.
AI Usage (see policy)
Git Copilot provided suggestions while writing the code.