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

cFE TIME unit tests break when different configuration options are used #109

Open
skliper opened this issue Sep 30, 2019 · 13 comments
Open

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

When using the unmodified sample version of the {{{cfe_platform_cfg.h}}} file, the test cases defined in {{{time_UT.c}}} all pass.

But if the user makes any time-related modifications to the platform config file, many of the unit test cases break. In particular, configuring a time server vs. time client or setting SRC_MET == TRUE, etc.

The test cases in "time_UT.c" need to include/accommodate other valid configuration options.

@skliper skliper added this to the 6.7.1 milestone Sep 30, 2019
@skliper skliper self-assigned this Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Imported from trac issue 78. Created by jphickey on 2015-07-09T16:30:14, last modified: 2019-07-03T12:48:08

@skliper skliper added the bug label Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2017-10-20 11:58:34:

This ticket points to a deeper issue and first requires a clean up of the TIME subsystem in general. Many of the configuration options trigger calls into functions that do not exist and are not defined in the respective API at all. Functions like:

OS_SelectTone()
OS_GetLocalMET()

Since these options have been broken in previous versions and this isn't a //new// issue in 6.6, I recommend deferring this to cfe-next.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2017-10-20 11:59:48:

NOTE: for example ticket #123 is one that should be a dependency of this one.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jhageman on 2019-04-01 09:41:02:

CCB 3/27/2019 - discussed removing unimplemented configuration options. Assigning to Alan for review.

Edit - fixed CCB date typo

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by acudmore on 2019-04-03 11:29:42:

As part of tickets 47, 78, and 92, I would like to do a survey of missions for TIME subsystem options.

Depending on the results of the survey, we should:

  1. Implement stub PSP functions to implement any necessary functions (PSP equivalents of OS_Get/SetLocalMET , OS_SelectTone) and update unit tests etc.
  2. Deprecate the un-needed options/features and prepare to remove them in a more comprehensive time subsystem simplification.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jphickey on 2019-04-03 11:40:01:

For what its worth, the "TimeBase" feature that was added and implemented in the "ng" OSAL versions may make these functions and special features in CFE_TIME unnecessary.

For the most part, CFE TIME just needs a properly synchronized 1Hz PPS signal from the PSP. Although I've never seen an actual implementation of e.g. OS_SelectTone my guess is that this would ultimately select the synchronization source for the 1Hz signal. The "MET" is probably analogous to the free run timer in the TimeBase object, which is already implemented (a monotonically incrementing tick counter that is reset when the system boots).

With the TimeBase layer the PSP can implement all this logic on its own and it is totally abstract to applications. CFE_TIME just gets a 1Hz callback based on the timebase, and the PSP can decide what to actually synchronize that to.

The main question to evaluate is whether any of the CFE_TIME ground commands or telemetry would change or report this status, and if so, we might need some form of PSP call to get the status in an abstract fashion.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by jhageman on 2019-07-03 12:48:08:

Moved unfinished 6.6.1 issues to next minor release

@skliper skliper assigned ghost and unassigned skliper Sep 30, 2019
@skliper skliper modified the milestones: 6.7.1, 6.7.0, 6.8.0 Sep 30, 2019
@skliper skliper removed this from the 6.8.0 milestone Nov 4, 2019
@skliper
Copy link
Contributor Author

skliper commented Feb 26, 2020

Related to #302

@BaldBeacon
Copy link

@skliper bump

We ran into this problem while building the unit tests for our WFIRST FSW. Could I expect this in an upcoming release?

@skliper
Copy link
Contributor Author

skliper commented Apr 6, 2020

Refactoring time services is a long term goal and would include fixing the unit tests, so I'd expect it will happen at some point. If it's a project request that needs to be prioritized you could initiate with whoever is handling project requests at this point. Maybe @jwilmot or @ejtimmon ?

@BaldBeacon
Copy link

IDK if it's something that has to be done on a deadline. In the meantime, we can create a new definitions directory that contains a modified version of the config that disables the use of the special time options.

I definitely don't speak for the project though, I was just adding some context about the timeline.

@jphickey
Copy link
Contributor

This issue may possibly need a bump in attention, as I noticed that something as simple as switching the config from CFE_PLATFORM_TIME_CFG_SERVER to CFE_PLATFORM_TIME_CFG_CLIENT causes time_UT.c to fail to build with errors.

Although all the different time sources/options are probably not part of the milestone 7.0.0 requirements, I expect we probably need at least CFG_CLIENT to work ... ?

@skliper
Copy link
Contributor Author

skliper commented Mar 24, 2022

Settings described in #2072 also fail to build, which is a setup used when there's an app providing the ExternalTime and ExternalTone.

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

No branches or pull requests

4 participants