From 119e3e60e79b28ea702415264be3baec8b54a1db Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 4 Jan 2021 13:21:43 -0500 Subject: [PATCH] Fix #1051, use OSAL time conversion/access methods Instead of accessing OS_time_t values directly, use the OSAL-provided conversion and access methods. This provides independence/abstraction from the specific OS_time_t definition and allows OSAL to transition to a 64 bit value. --- fsw/cfe-core/src/es/cfe_es_backgroundtask.c | 17 +--- fsw/cfe-core/src/time/cfe_time_api.c | 100 ++++---------------- fsw/cfe-core/src/time/cfe_time_utils.c | 4 +- fsw/cfe-core/unit-test/time_UT.c | 53 ++--------- fsw/cfe-core/unit-test/ut_support.c | 3 +- 5 files changed, 30 insertions(+), 147 deletions(-) diff --git a/fsw/cfe-core/src/es/cfe_es_backgroundtask.c b/fsw/cfe-core/src/es/cfe_es_backgroundtask.c index 28653956c..db2254b4c 100644 --- a/fsw/cfe-core/src/es/cfe_es_backgroundtask.c +++ b/fsw/cfe-core/src/es/cfe_es_backgroundtask.c @@ -136,25 +136,12 @@ void CFE_ES_BackgroundTask(void) { /* * compute the elapsed time (difference) between last - * execution and now, in microseconds. - * - * Note this calculation is done as a uint32 which will overflow - * after about 35 minutes, but the max delays ensure that this - * executes at least every few seconds, so that should never happen. + * execution and now, in milliseconds. */ CFE_PSP_GetTime(&CurrTime); - ElapsedTime = 1000000 * (CurrTime.seconds - LastTime.seconds); - ElapsedTime += CurrTime.microsecs; - ElapsedTime -= LastTime.microsecs; + ElapsedTime = OS_TimeGetTotalMilliseconds(OS_TimeSubtract(CurrTime, LastTime)); LastTime = CurrTime; - /* - * convert to milliseconds. - * we do not really need high precision - * for background task timings - */ - ElapsedTime /= 1000; - NextDelay = CFE_ES_BACKGROUND_MAX_IDLE_DELAY; /* default; will be adjusted based on active jobs */ JobPtr = CFE_ES_BACKGROUND_JOB_TABLE; JobTotal = CFE_ES_BACKGROUND_NUM_JOBS; diff --git a/fsw/cfe-core/src/time/cfe_time_api.c b/fsw/cfe-core/src/time/cfe_time_api.c index 37e8060d7..96b39ca09 100644 --- a/fsw/cfe-core/src/time/cfe_time_api.c +++ b/fsw/cfe-core/src/time/cfe_time_api.c @@ -489,57 +489,16 @@ CFE_TIME_Compare_t CFE_TIME_Compare(CFE_TIME_SysTime_t TimeA, CFE_TIME_SysTime_ */ uint32 CFE_TIME_Sub2MicroSecs(uint32 SubSeconds) { - uint32 MicroSeconds; - - /* 0xffffdf00 subseconds = 999999 microseconds, so anything greater - * than that we set to 999999 microseconds, so it doesn't get to - * a million microseconds */ - - if (SubSeconds > 0xffffdf00) - { - MicroSeconds = 999999; - } - else - { - /* - ** Convert a 1/2^32 clock tick count to a microseconds count - ** - ** Conversion factor is ( ( 2 ** -32 ) / ( 10 ** -6 ) ). - ** - ** Logic is as follows: - ** x * ( ( 2 ** -32 ) / ( 10 ** -6 ) ) - ** = x * ( ( 10 ** 6 ) / ( 2 ** 32 ) ) - ** = x * ( ( 5 ** 6 ) ( 2 ** 6 ) / ( 2 ** 26 ) ( 2 ** 6) ) - ** = x * ( ( 5 ** 6 ) / ( 2 ** 26 ) ) - ** = x * ( ( 5 ** 3 ) ( 5 ** 3 ) / ( 2 ** 7 ) ( 2 ** 7 ) (2 ** 12) ) - ** - ** C code equivalent: - ** = ( ( ( ( ( x >> 7) * 125) >> 7) * 125) >> 12 ) - */ - - MicroSeconds = (((((SubSeconds >> 7) * 125) >> 7) * 125) >> 12); - + OS_time_t tm; - /* if the Subseconds % 0x4000000 != 0 then we will need to - * add 1 to the result. the & is a faster way of doing the % */ - if ((SubSeconds & 0x3ffffff) != 0) - { - MicroSeconds++; - } - - /* In the Micro2SubSecs conversion, we added an extra anomaly - * to get the subseconds to bump up against the end point, - * 0xFFFFF000. This must be accounted for here. Since we bumped - * at the half way mark, we must "unbump" at the same mark - */ - if (MicroSeconds > 500000) - { - MicroSeconds --; - } - - } /* end else */ - - return(MicroSeconds); + /* + ** Convert using the OSAL method. Note that there + ** is no range check here because any uint32 value is valid, + ** and OSAL will handle and properly convert any input. + */ + tm = OS_TimeAssembleFromSubseconds(0, SubSeconds); + + return OS_TimeGetMicrosecondsPart(tm); } /* End of CFE_TIME_Sub2MicroSecs() */ @@ -549,10 +508,12 @@ uint32 CFE_TIME_Sub2MicroSecs(uint32 SubSeconds) */ uint32 CFE_TIME_Micro2SubSecs(uint32 MicroSeconds) { + OS_time_t tm; uint32 SubSeconds; /* ** Conversion amount must be less than one second + ** (preserves existing behavior where output saturates at max value) */ if (MicroSeconds > 999999) { @@ -560,40 +521,11 @@ uint32 CFE_TIME_Micro2SubSecs(uint32 MicroSeconds) } else { - /* - ** Convert micro-seconds count to sub-seconds (1/2^32) count - ** - ** Conversion factor is ( ( 10 ** -6 ) / ( 2 ** -20 ). - ** - ** Logic is as follows: - ** x * ( ( 10 ** -6 ) / ( 2 ** -32 ) ) - ** = x * ( ( 2 ** 32 ) / ( 10 ** 6 ) ) - ** = x * ( ( ( 2 ** 26 ) ( 2 ** 6) ) / ( ( 5 ** 6 ) ( 2 ** 6 ) ) ) - ** = x * ( ( 2 ** 26 ) / ( 5 ** 6 ) ) - ** = x * ( ( ( 2 ** 11) ( 2 ** 3) (2 ** 12) ) / ( 5( 5 ** 5 ) ) ) - ** = x * ( ( ( ( ( 2 ** 11 ) / 5 ) * ( 2 ** 3 ) ) / ( 5 ** 5 ) ) * (2 ** 12) ) - ** - ** C code equivalent: - ** = ( ( ( ( ( x << 11 ) / 5 ) << 3 ) / 3125 ) << 12 ) - ** - ** Conversion factor was reduced and factored accordingly - ** to minimize precision loss and register overflow. - */ - SubSeconds = ( ( ( ( MicroSeconds << 11 ) / 5 ) << 3 ) / 3125 ) << 12; - - /* To get the SubSeconds to "bump up" against 0xFFFFF000 when - * MicroSeconds = 9999999, we add in another anomaly to the - * conversion at the half-way point (500000 us). This will bump - * all of the subseconds up by 0x1000, so 999999 us == 0xFFFFF00, - * 999998 == 0xFFFFE000, etc. This extra anomaly is accounted for - * in the Sub2MicroSecs conversion as well. - */ - - if (SubSeconds > 0x80001000) - { - SubSeconds += 0x1000; - } - + /* + ** Convert micro-seconds count to sub-seconds (1/2^32) count using OSAL + */ + tm = OS_TimeAssembleFromNanoseconds(0, MicroSeconds * 1000); + SubSeconds = OS_TimeGetSubsecondsPart(tm); } return(SubSeconds); diff --git a/fsw/cfe-core/src/time/cfe_time_utils.c b/fsw/cfe-core/src/time/cfe_time_utils.c index aeaeebeef..a29623875 100644 --- a/fsw/cfe-core/src/time/cfe_time_utils.c +++ b/fsw/cfe-core/src/time/cfe_time_utils.c @@ -91,8 +91,8 @@ CFE_TIME_SysTime_t CFE_TIME_LatchClock(void) /* ** Convert time to cFE format (seconds : 1/2^32 subseconds)... */ - LatchTime.Seconds = LocalTime.seconds; - LatchTime.Subseconds = CFE_TIME_Micro2SubSecs(LocalTime.microsecs); + LatchTime.Seconds = OS_TimeGetTotalSeconds(LocalTime); + LatchTime.Subseconds = OS_TimeGetSubsecondsPart(LocalTime); return(LatchTime); diff --git a/fsw/cfe-core/unit-test/time_UT.c b/fsw/cfe-core/unit-test/time_UT.c index 79455f147..91c922c53 100644 --- a/fsw/cfe-core/unit-test/time_UT.c +++ b/fsw/cfe-core/unit-test/time_UT.c @@ -1056,6 +1056,14 @@ void Test_ConvertTime(void) "CFE_TIME_MET2SCTime", testDesc); + /* NOTE: Microseconds <-> Subseconds conversion routines are implemented + * as part of OS_time_t in OSAL, and are coverage tested there. CFE time + * conversions are now just wrappers of these OSAL routines. + + * This should only sanity-check basic values to get coverage of the CFE + * wrappers. Testing of value corner cases / rounding should be limited to + * OSAL coverage test. */ + /* Test subseconds to microseconds conversion; zero subsecond value */ UT_InitData(); UT_Report(__FILE__, __LINE__, @@ -1063,31 +1071,13 @@ void Test_ConvertTime(void) "CFE_TIME_Sub2MicroSecs", "Convert 0 subsecond value"); - /* Test subseconds to microseconds conversion; positive microsecond - * adjust - */ - UT_InitData(); - UT_Report(__FILE__, __LINE__, - CFE_TIME_Sub2MicroSecs(0xffff) == 16, - "CFE_TIME_Sub2MicroSecs", - "+1 microsecond adjustment"); - - /* Test subseconds to microseconds conversion; no microsecond adjust */ + /* Test subseconds to microseconds conversion; half second */ UT_InitData(); UT_Report(__FILE__, __LINE__, CFE_TIME_Sub2MicroSecs(0x80000000) == 500000, "CFE_TIME_Sub2MicroSecs", "No microsecond adjustment"); - /* Test subseconds to microseconds conversion; negative microsecond - * adjust - */ - UT_InitData(); - UT_Report(__FILE__, __LINE__, - CFE_TIME_Sub2MicroSecs(0x80002000) == 500001, - "CFE_TIME_Sub2MicroSecs", - "-1 microsecond adjustment"); - /* Test subseconds to microseconds conversion; subseconds exceeds * microseconds limit */ @@ -1104,31 +1094,6 @@ void Test_ConvertTime(void) "CFE_TIME_Micro2SubSecs", "Convert 0 microseconds to 0 subseconds"); - /* Test microseconds to subseconds conversion; no subsecond adjust */ - UT_InitData(); - UT_Report(__FILE__, __LINE__, - CFE_TIME_Micro2SubSecs(0xffff) == 281468928, - "CFE_TIME_Micro2SubSecs", - "Microseconds below maximum value (no subsecond adjustment)"); - - /* Test microseconds to subseconds conversion; microseconds below - * maximum limit - */ - UT_InitData(); - UT_Report(__FILE__, __LINE__, - CFE_TIME_Micro2SubSecs(999998) == 0xffffe000, - "CFE_TIME_Micro2SubSecs", - "Microseconds below maximum value (subsecond adjustment)"); - - /* Test microseconds to subseconds conversion; microseconds equals - * maximum limit - */ - UT_InitData(); - UT_Report(__FILE__, __LINE__, - CFE_TIME_Micro2SubSecs(999999) == 0xfffff000, - "CFE_TIME_Micro2SubSecs", - "Microseconds equals maximum value (subsecond adjustment)"); - /* Test microseconds to subseconds conversion; microseconds exceeds * maximum limit */ diff --git a/fsw/cfe-core/unit-test/ut_support.c b/fsw/cfe-core/unit-test/ut_support.c index 7c545dcc3..e42dda886 100644 --- a/fsw/cfe-core/unit-test/ut_support.c +++ b/fsw/cfe-core/unit-test/ut_support.c @@ -373,8 +373,7 @@ void UT_SetBSP_Time(uint32 seconds, uint32 microsecs) { OS_time_t BSP_Time; - BSP_Time.seconds = seconds; - BSP_Time.microsecs = microsecs; + BSP_Time = OS_TimeAssembleFromNanoseconds(seconds, microsecs * 1000); UT_SetDataBuffer(UT_KEY(CFE_PSP_GetTime), &BSP_Time, sizeof(BSP_Time), true); }