Skip to content

Commit

Permalink
Merge pull request #1051 from jphickey/fix-1022-timebase-api-retcodes
Browse files Browse the repository at this point in the history
Fix #1022, resolve discrepancies between timebase API and unit tests
  • Loading branch information
astrogeco authored Jun 2, 2021
2 parents f3c3692 + fc0482b commit 494f2f4
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 52 deletions.
19 changes: 13 additions & 6 deletions src/os/inc/osapi-timebase.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,14 @@ typedef struct
* be configured to support at least (OS_MAX_TASKS + OS_MAX_TIMEBASES) threads,
* to account for the helper threads associated with time base objects.
*
* @param[out] timebase_id A non-zero ID corresponding to the timebase resource
* @param[in] timebase_name The name of the time base
* @param[out] timebase_id will be set to the non-zero ID of the newly-created resource @nonnull
* @param[in] timebase_name The name of the time base @nonnull
* @param[in] external_sync A synchronization function for BSP hardware-based timer ticks
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERR_NAME_TAKEN if the name specified is already used
* @retval #OS_ERR_NO_FREE_IDS if there can be no more timebase resources created
* @retval #OS_ERR_INCORRECT_OBJ_STATE if called from timer/timebase context
* @retval #OS_ERR_NAME_TOO_LONG if the timebase_name is too long
* @retval #OS_INVALID_POINTER if a pointer argument is NULL
Expand All @@ -103,6 +106,8 @@ int32 OS_TimeBaseCreate(osal_id_t *timebase_id, const char *timebase_name, OS_Ti
* @param[in] interval_time The amount of delay between ticks, in microseconds.
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERR_INVALID_ID if the id passed in is not a valid timebase
* @retval #OS_ERR_INCORRECT_OBJ_STATE if called from timer/timebase context
* @retval #OS_TIMER_ERR_INVALID_ARGS if start_time or interval_time are out of range
*/
Expand All @@ -118,6 +123,8 @@ int32 OS_TimeBaseSet(osal_id_t timebase_id, uint32 start_time, uint32 interval_t
* @param[in] timebase_id The timebase resource to delete
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERR_INVALID_ID if the id passed in is not a valid timebase
* @retval #OS_ERR_INCORRECT_OBJ_STATE if called from timer/timebase context
*/
int32 OS_TimeBaseDelete(osal_id_t timebase_id);
Expand All @@ -128,8 +135,8 @@ int32 OS_TimeBaseDelete(osal_id_t timebase_id);
*
* Given a time base name, find and output the ID associated with it.
*
* @param[out] timebase_id The timebase resource ID
* @param[in] timebase_name The name of the timebase resource to find
* @param[out] timebase_id will be set to the non-zero ID of the matching resource @nonnull
* @param[in] timebase_name The name of the timebase resource to find @nonnull
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
Expand All @@ -151,7 +158,7 @@ int32 OS_TimeBaseGetIdByName(osal_id_t *timebase_id, const char *timebase_name);
* all of the relevant info( name and creator) about the specified timebase.
*
* @param[in] timebase_id The timebase resource ID
* @param[out] timebase_prop Buffer to store timebase properties
* @param[out] timebase_prop Buffer to store timebase properties @nonnull
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
Expand Down Expand Up @@ -185,7 +192,7 @@ int32 OS_TimeBaseGetInfo(osal_id_t timebase_id, OS_timebase_prop_t *timebase_pro
* and calculate the difference between the consecutive samples.
*
* @param[in] timebase_id The timebase to operate on
* @param[out] freerun_val Buffer to store the free run counter
* @param[out] freerun_val Buffer to store the free run counter @nonnull
*
* @return Execution status, see @ref OSReturnCodes
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
Expand Down
138 changes: 92 additions & 46 deletions src/tests/time-base-api-test/time-base-api-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,34 @@
#include "uttest.h"
#include "utbsp.h"

static int32 SyncTimeBaseCreateRc = 0;
static int32 SyncTimeBaseDeleteRc = 0;
static int32 SyncTimeBaseSetRc = 0;
static int32 SyncTimeBaseGetByNameRc = 0;
static int32 SyncTimeBaseGetInfoRc = 0;

static OS_timebase_prop_t SyncTimeBaseProp;

static uint32 NumSyncs = 0;

static uint32 UT_TimerSync(osal_id_t timer_id)
{
/*
* Calls to time base configuration from the context of a sync function
* should be rejected with OS_ERR_INCORRECT_OBJ_STATE. However because
* UtAssert is not fully thread-safe, this does not assert here, it just
* calls the various functions on the first time through and stores the
* result, which is checked/asserted in the main task.
*/
if (NumSyncs == 0)
{
SyncTimeBaseCreateRc = OS_TimeBaseCreate(&timer_id, "sync", 0);
SyncTimeBaseDeleteRc = OS_TimeBaseDelete(timer_id);
SyncTimeBaseSetRc = OS_TimeBaseSet(timer_id, 100, 100);
SyncTimeBaseGetByNameRc = OS_TimeBaseGetIdByName(&timer_id, "TimeBaseC");
SyncTimeBaseGetInfoRc = OS_TimeBaseGetInfo(timer_id, &SyncTimeBaseProp);
}
++NumSyncs;
OS_TaskDelay(1);
return 1;
}
Expand All @@ -47,49 +73,19 @@ void TestTimeBaseApi(void)
{
int32 expected;
int32 actual;
int32 TimeBaseNum;
int32 tbc_results[OS_MAX_TIMEBASES];
uint32 freerun;
osal_id_t objid;
osal_id_t time_base_id;
osal_id_t time_base_id2;
osal_id_t tb_id[OS_MAX_TIMEBASES];
char overMaxTimeBase[12];
char TimeBaseIter[OS_MAX_TIMEBASES][12];
char timebase_name[OS_MAX_API_NAME + 5];
OS_timebase_prop_t timebase_prop;

/*
* Test Case For:
* int32 OS_TimeBaseCreate(uint32 *timer_id, const char *timebase_name, OS_TimerSync_t external_sync)
*/

/* Test for nominal inputs */
expected = OS_SUCCESS;

actual = OS_TimeBaseCreate(&time_base_id, "TimeBaseA", 0);
UtAssert_True(actual == expected, "OS_TimeBaseCreate() (%ld) == OS_SUCCESS", (long)actual);

actual = OS_TimeBaseCreate(&time_base_id2, "TimeBaseB", NULL);
UtAssert_True(actual == expected, "OS_TimeBaseCreate() (%ld) == OS_SUCCESS", (long)actual);

actual = OS_TimeBaseCreate(&time_base_id, "TimeBaseC", UT_TimerSync);
UtAssert_True(actual == expected, "OS_TimeBaseCreate() (%ld) == OS_SUCCESS", (long)actual);

/* Test for nominal, max/min cases */
objid = OS_OBJECT_ID_UNDEFINED;
actual = OS_TimeBaseCreate(&objid, "TimeBaseD", 0);
UtAssert_True(actual == expected, "OS_TimeBaseCreate() (%ld) == OS_SUCCESS", (long)actual);

/* Checking for OS_MAX_TIMEBASES */
for (int i = 0; i < OS_MAX_TIMEBASES; i++)
{
snprintf(TimeBaseIter[i], 12, "TimeBase%d", i);
tbc_results[i] = OS_TimeBaseCreate(&tb_id[i], TimeBaseIter[i], 0);
UtAssert_True(tbc_results[i] == expected, "OS_TimeBaseCreate() (%ld) == OS_SUCCESS", (long)actual);

OS_TimeBaseDelete(tb_id[i]);
}

/* Test for invalid inputs */
expected = OS_INVALID_POINTER;
actual = OS_TimeBaseCreate(NULL, NULL, NULL);
Expand All @@ -103,21 +99,64 @@ void TestTimeBaseApi(void)
actual = OS_TimeBaseCreate(&time_base_id, NULL, 0);
UtAssert_True(actual == expected, "OS_TimeBaseCreate() (%ld) == OS_INVALID_POINTER", (long)actual);

expected = OS_ERR_NAME_TAKEN;
actual = OS_TimeBaseCreate(&time_base_id, "TimeBaseA", 0);
UtAssert_True(actual == expected, "OS_TimeBaseCreate() (%ld) == OS_ERR_NAME_TAKEN", (long)actual);
memset(timebase_name, 'x', sizeof(timebase_name));
timebase_name[sizeof(timebase_name) - 1] = 0;
UtAssert_INT32_EQ(OS_TimeBaseCreate(&time_base_id, timebase_name, 0), OS_ERR_NAME_TOO_LONG);

/* Checking OS_MAX_TIMEBASES + 1 */
/* Checking for OS_MAX_TIMEBASES */
for (int i = 0; i < OS_MAX_TIMEBASES; i++)
{
snprintf(TimeBaseIter[i], sizeof(TimeBaseIter[i]), "TimeBase%d", i);
tbc_results[i] = OS_TimeBaseCreate(&tb_id[i], TimeBaseIter[i], 0);
/* On the final setup pass, while there is still one free slot,
* check attempting to create a duplicate name (index 0) - this
* should be rejected and _not_ consume the last empty slot */
if (i == (OS_MAX_TIMEBASES - 1))
{
UtAssert_INT32_EQ(OS_TimeBaseCreate(&time_base_id, "TimeBase0", 0), OS_ERR_NAME_TAKEN);
}

snprintf(timebase_name, sizeof(timebase_name), "TimeBase%d", i);
UtAssert_INT32_EQ(OS_TimeBaseCreate(&tb_id[i], timebase_name, 0), OS_SUCCESS);
}
TimeBaseNum = OS_MAX_TIMEBASES + 1;
snprintf(overMaxTimeBase, sizeof(overMaxTimeBase), "TimeBase%d", (int)TimeBaseNum);
expected = OS_ERR_NO_FREE_IDS;
actual = OS_TimeBaseCreate(&time_base_id, "overMaxTimeBase", 0);
UtAssert_True(actual == expected, "OS_TimeBaseCreate() (%ld) == OS_ERR_NO_FREE_IDS", (long)actual);

/* Checking OS_MAX_TIMEBASES + 1 */
UtAssert_INT32_EQ(OS_TimeBaseCreate(&time_base_id, "overMaxTimeBase", 0), OS_ERR_NO_FREE_IDS);

/* reset test environment */
OS_DeleteAllObjects();

/* Test for nominal inputs - these resources are used for the remainder of test */
expected = OS_SUCCESS;

actual = OS_TimeBaseCreate(&time_base_id, "TimeBaseA", 0);
UtAssert_True(actual == expected, "OS_TimeBaseCreate() (%ld) == OS_SUCCESS", (long)actual);

actual = OS_TimeBaseCreate(&time_base_id2, "TimeBaseB", NULL);
UtAssert_True(actual == expected, "OS_TimeBaseCreate() (%ld) == OS_SUCCESS", (long)actual);

actual = OS_TimeBaseCreate(&time_base_id, "TimeBaseC", UT_TimerSync);
UtAssert_True(actual == expected, "OS_TimeBaseCreate() (%ld) == OS_SUCCESS", (long)actual);

/* Let the TimeBaseC accumulate at least one sync, so it will
* attempt to call timebase APIs from its own context. */
while (NumSyncs == 0)
{
OS_TaskDelay(1);
}

/* Check that those configuration attempts all returned OS_ERR_INCORRECT_OBJ_STATE */
UtAssert_True(SyncTimeBaseCreateRc == OS_ERR_INCORRECT_OBJ_STATE,
"OS_TimeBaseCreate(&timer_id, \"sync\", 0) (%d) == OS_ERR_INCORRECT_OBJ_STATE",
(int)SyncTimeBaseCreateRc);
UtAssert_True(SyncTimeBaseDeleteRc == OS_ERR_INCORRECT_OBJ_STATE,
"OS_TimeBaseDelete(timer_id) (%d) == OS_ERR_INCORRECT_OBJ_STATE", (int)SyncTimeBaseDeleteRc);
UtAssert_True(SyncTimeBaseSetRc == OS_ERR_INCORRECT_OBJ_STATE,
"OS_TimeBaseSet(timer_id, 100, 100) (%d) == OS_ERR_INCORRECT_OBJ_STATE", (int)SyncTimeBaseSetRc);
UtAssert_True(SyncTimeBaseGetByNameRc == OS_ERR_INCORRECT_OBJ_STATE,
"OS_TimeBaseGetIdByName(&timer_id, \"TimeBaseC\") (%d) == OS_ERR_INCORRECT_OBJ_STATE",
(int)SyncTimeBaseGetByNameRc);
UtAssert_True(SyncTimeBaseGetInfoRc == OS_ERR_INCORRECT_OBJ_STATE,
"OS_TimeBaseGetInfo(timer_id, &SyncTimeBaseProp) (%d) == OS_ERR_INCORRECT_OBJ_STATE",
(int)SyncTimeBaseGetInfoRc);

/*
* Test Case For:
Expand Down Expand Up @@ -192,9 +231,13 @@ void TestTimeBaseApi(void)
UtAssert_True(!OS_ObjectIdDefined(objid), "OS_TimeBaseGetIdByName() objid (%lu) still OS_OBJECT_ID_UNDEFINED",
OS_ObjectIdToInteger(objid));

expected = OS_INVALID_POINTER;
actual = OS_TimeBaseGetIdByName(NULL, NULL);
UtAssert_True(actual == expected, "OS_TimeBaseGetIdByName() (%ld) == OS_INVALID_POINTER", (long)actual);
/* check each pointer input individually */
UtAssert_INT32_EQ(OS_TimeBaseGetIdByName(NULL, "NF"), OS_INVALID_POINTER);
UtAssert_INT32_EQ(OS_TimeBaseGetIdByName(&objid, NULL), OS_INVALID_POINTER);

memset(timebase_name, 'x', sizeof(timebase_name));
timebase_name[sizeof(timebase_name) - 1] = 0;
UtAssert_INT32_EQ(OS_TimeBaseGetIdByName(&objid, timebase_name), OS_ERR_NAME_TOO_LONG);

/*
* Test Case For:
Expand All @@ -214,7 +257,7 @@ void TestTimeBaseApi(void)

UtAssert_True(!OS_ObjectIdDefined(timebase_prop.creator), "timebase_prop.creator (%lu) undefined",
OS_ObjectIdToInteger(timebase_prop.creator));
UtAssert_True(strcmp(timebase_prop.name, "TimeBaseB") == 0, "timebase_prop.name (%s) == TimeBase2",
UtAssert_True(strcmp(timebase_prop.name, "TimeBaseB") == 0, "timebase_prop.name (%s) == TimeBaseB",
timebase_prop.name);
UtAssert_True(timebase_prop.nominal_interval_time == 0, "timebase_prop.nominal_interval_time (%lu) == 0",
(unsigned long)timebase_prop.nominal_interval_time);
Expand Down Expand Up @@ -249,6 +292,9 @@ void TestTimeBaseApi(void)
actual = OS_TimeBaseGetFreeRun(time_base_id2, &freerun);
UtAssert_True(actual == expected, "OS_TimeBaseGetFreeRun() (%ld) == OS_SUCCESS", (long)actual);

UtAssert_INT32_EQ(OS_TimeBaseGetFreeRun(OS_OBJECT_ID_UNDEFINED, &freerun), OS_ERR_INVALID_ID);
UtAssert_INT32_EQ(OS_TimeBaseGetFreeRun(time_base_id2, NULL), OS_INVALID_POINTER);

/* Test for invalid inputs */
expected = OS_ERR_INVALID_ID;
freerun = 0xFFFFFFFF;
Expand Down

0 comments on commit 494f2f4

Please sign in to comment.