Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move "FM_GlobalData" back into private/local data structures #75

Closed
3 tasks done
jphickey opened this issue Jan 25, 2023 · 0 comments · Fixed by #78
Closed
3 tasks done

Move "FM_GlobalData" back into private/local data structures #75

jphickey opened this issue Jan 25, 2023 · 0 comments · Fixed by #78

Comments

@jphickey
Copy link
Contributor

jphickey commented Jan 25, 2023

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
The FM_GlobalData object is only used within the FM app for private data storage. It should not be visible externally. However, it is currently defined here in the public fm_msg.h file:

FM/fsw/inc/fm_msg.h

Lines 542 to 594 in c856997

typedef struct
{
FM_MonitorTable_t *MonitorTablePtr; /**< \brief File System Table Pointer */
CFE_TBL_Handle_t MonitorTableHandle; /**< \brief File System Table Handle */
CFE_SB_PipeId_t CmdPipe; /**< \brief cFE software bus command pipe */
CFE_ES_TaskId_t ChildTaskID; /**< \brief Child task ID */
osal_id_t ChildSemaphore; /**< \brief Child task wakeup counting semaphore */
osal_id_t ChildQueueCountSem; /**< \brief Child queue counter mutex semaphore */
uint8 ChildCmdCounter; /**< \brief Child task command success counter */
uint8 ChildCmdErrCounter; /**< \brief Child task command error counter */
uint8 ChildCmdWarnCounter; /**< \brief Child task command warning counter */
uint8 ChildWriteIndex; /**< \brief Array index for next write to command args */
uint8 ChildReadIndex; /**< \brief Array index for next read from command args */
uint8 ChildQueueCount; /**< \brief Number of pending commands in queue */
uint8 CommandCounter; /**< \brief Application command success counter */
uint8 CommandErrCounter; /**< \brief Application command error counter */
uint8 Spare8a; /**< \brief Placeholder for unused command warning counter */
uint8 ChildCurrentCC; /**< \brief Command code currently executing */
uint8 ChildPreviousCC; /**< \brief Command code previously executed */
uint8 Spare8b; /**< \brief Structure alignment spare */
uint32 FileStatTime; /**< \brief Modify time from most recent OS_stat */
uint32 FileStatSize; /**< \brief File size from most recent OS_stat */
uint32 FileStatMode; /**< \brief File mode from most recent OS_stat (OS_FILESTAT_MODE) */
FM_DirListFileStats_t DirListFileStats; /**< \brief Get dir list to file statistics structure */
FM_DirListPkt_t DirListPkt; /**< \brief Get dir list to packet telemetry packet */
FM_MonitorReportPkt_t
MonitorReportPkt; /**< \brief Telemetry packet reporting status of items in the monitor table */
FM_FileInfoPkt_t FileInfoPkt; /**< \brief Get file info telemetry packet */
FM_OpenFilesPkt_t OpenFilesPkt; /**< \brief Get open files telemetry packet */
FM_HousekeepingPkt_t HousekeepingPkt; /**< \brief Application housekeeping telemetry packet */
char ChildBuffer[FM_CHILD_FILE_BLOCK_SIZE]; /**< \brief Child task file I/O buffer */
FM_ChildQueueEntry_t ChildQueue[FM_CHILD_QUEUE_DEPTH]; /**< \brief Child task command queue */
#ifdef FM_INCLUDE_DECOMPRESS
FS_LIB_Decompress_State_t DecompressState;
#endif
} FM_GlobalData_t;

Describe the solution you'd like
This should be defined in one of the internal header files, not in a public interface file.

Additional context
Public API should generally only be constants / #define's, and typedefs. API calls only for libraries - apps do not have public API calls. Extern data structs / globals should not be exposed in either apps or libs for a variety of reasons.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Jan 25, 2023
jphickey added a commit to jphickey/FM that referenced this issue Jan 30, 2023
Global data structures should not be defined in the public include
files, they should be private to the local source directory.

Unit test is justified in accessing this file directly, so provisions
are also made to permit this.
jphickey added a commit to jphickey/FM that referenced this issue Jan 30, 2023
Global data structures should not be defined in the public include
files, they should be private to the local source directory.

Unit test is justified in accessing this file directly, so provisions
are also made to permit this.
jphickey added a commit to jphickey/FM that referenced this issue Jan 30, 2023
Global data structures should not be defined in the public include
files, they should be private to the local source directory.

Unit test is justified in accessing this file directly, so provisions
are also made to permit this.
jphickey added a commit to jphickey/FM that referenced this issue Jan 30, 2023
Global data structures should not be defined in the public include
files, they should be private to the local source directory.

Unit test is justified in accessing this file directly, so provisions
are also made to permit this.
dzbaker added a commit that referenced this issue Feb 2, 2023
Fix #75, move app global to internal include
@dzbaker dzbaker added this to the Equuleus milestone Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants