Time Services need cleanup relative to requirements

[skliper]

Per CCB on 3/27/19, Time Services goes way beyond it's requirements. Need to re-evaluate configuration options and reduce mission specific code.

As part of the cleanup, factor out duplicate code. Specifically referenced at code review in command handling.

• CFE_TIME_SetTimeCmd, CFE_TIME_SetMETCmd, CFE_TIME_SetStcfCmd, and CFE_TIME_SetLeapSeconds are all basically the same logic, etc.
• break of files Refactor/split files (headers and c) into smaller topics #1322, specific note from CFS-43 was the lack of a cfe_time_tone.h
• CFE_TIME_ToneSendGPS and CFE_TIME_ToneSendTime are basically the same logic, refactor
• Factor out duplicate logic in CFE_TIME_RegisterSynchCallback and CFE_TIME_UnregisterSynchCallback #1536
• Refactor or replace API's related to CFE_TIME_GetReference #1535
• Mutually exclusive defines could just be a boolean (but will likely go away w/ refactor), note cfe_time_verify.h is excessively complex at this point
  

  cFE/modules/time/fsw/src/cfe_time_verify.h

  Lines 43 to 51 in 84ba9a9

  	#if (CFE_PLATFORM_TIME_CFG_SERVER == true)
  	#if (CFE_PLATFORM_TIME_CFG_CLIENT == true)
  	#error Cannot define both CFE_PLATFORM_TIME_CFG_SERVER and CFE_PLATFORM_TIME_CFG_CLIENT as true!
  	#endif
  	#else
  	#if (CFE_PLATFORM_TIME_CFG_CLIENT != true)
  	#error Must define either CFE_PLATFORM_TIME_CFG_SERVER or CFE_PLATFORM_TIME_CFG_CLIENT as true!
  	#endif
  	#endif

[skliper]

Imported from trac issue 271. Created by jhageman on 2019-04-01T09:44:50, last modified: 2019-05-23T16:43:54

[skliper]

Suggestions - Explicitly define TAT pulse and 1Hz pulse, the system should handle various configurations (any combination of 'virtual/internal' and external/API, synced or not)

TAT pulse source selection (SetSource) should be generalized to 'virtual' or passed to PSP, or get triggered via API (generalize implementation of cTIME2010 which currently just does PRIMARY/SECONDARY)
1Hz pulse source selection (new) should be generalized to 'virtual' aka timer based or passed to PSP (expect trigger via API), or based off of Tone Pulse (synced, 1Hz loop triggered by Tone Pulse)
MET source selection (new) should be generalized to 'virtual' or passed to PSP, or passed in via API
1Hz pulse when external shall flywheel after mission config elapsed time without a pulse (missing requirement)

Really TIME ifdefs need to be removed, and TIME should just do what's requested when requested.

Note #518 will fix some of the requirements but not all, so revisit after that update.

[skliper]

See cTIME2013 - need to simplify configuration to adjust SCTF when requested (regardless of mode).

[skliper]

SetSourceCmd - not well defined requirement or implementation. Suggest updates per the 1Hz comment above (sort of tied 1Hz cycle to Tone but needs work).

[skliper]

CFE_TIME_Locaal1HzISR related design also needs cleanup and implementation is not consistent with CFE_TIME_Tone1HzISR. Wouldn't expect an "ISR" to be exposed in an API, Tone1HzISR has an ExternalTone API wrapper, etc. See also #551 which only fixed the duplicate prototype, but bigger issues should be addressed as part of refactor.

[skliper]

Also noted in code review, inconsistent parameter names used in the APIs (Time1/2, TimeA/B). Improve consistency.

[skliper]

Also noted in code review, consider merge of external events (CFE_TIME_ExternalMET, CFE_TIME_ExternalGPS, CFE_TIME_ExternalTime). Although might be simpler to just include all three and support selection (internal/virtual/external MET, OS/GPS/Time (and relation to tone), etc) instead of #ifdefs all over. They are APIs, you don't have to use them.

[thnkslprpt]

Per CCB on 3/27/19, Time Services goes way beyond it's requirements. Need to re-evaluate configuration options and reduce mission specific code.

As part of the cleanup, factor out duplicate code. Specifically referenced at code review in command handling.

• CFE_TIME_SetTimeCmd, CFE_TIME_SetMETCmd, CFE_TIME_SetStcfCmd, and CFE_TIME_SetLeapSeconds are all basically the same logic, etc.
  ...

This first point is addressed here (excl. CFE_TIME_SetLeapSeconds() which does not have enough common logic to justify being included):

• Fix #2339, Refactor common logic out of CFE_TIME_SetTime/MET/STCFCmd() #2340

[thnkslprpt]

Also noted in code review, inconsistent parameter names used in the APIs (Time1/2, TimeA/B). Improve consistency.

This is addressed here:

• Fix #2341, Rename TimeA/TimeB parameter to improve consistency #2342