Skip to content

Commit

Permalink
Fix #664, change type of sync callback argument to osal_id_t
Browse files Browse the repository at this point in the history
ID is preferable to an array index because direct table access
should not be done outside OSAL itself.
  • Loading branch information
jphickey committed Dec 2, 2020
1 parent e752541 commit cf9db6d
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 76 deletions.
4 changes: 2 additions & 2 deletions src/os/inc/osapi-os-timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
/*
** Typedefs
*/
typedef void (*OS_TimerCallback_t)(osal_id_t timer_id); /**< @brief Timer callback */
typedef uint32 (*OS_TimerSync_t)(osal_index_t timer_id); /**< @brief Timer sync */
typedef void (*OS_TimerCallback_t)(osal_id_t timer_id); /**< @brief Timer callback */
typedef uint32 (*OS_TimerSync_t)(osal_id_t timer_id); /**< @brief Timer sync */

/** @brief Timer properties */
typedef struct
Expand Down
65 changes: 36 additions & 29 deletions src/os/posix/src/os-impl-timebase.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,42 +141,49 @@ void OS_TimeBaseUnlock_Impl(const OS_object_token_t *token)
* Purpose: Local helper routine, not part of OSAL API.
*
*-----------------------------------------------------------------*/
static uint32 OS_TimeBase_SigWaitImpl(osal_index_t timer_id)
static uint32 OS_TimeBase_SigWaitImpl(osal_id_t obj_id)
{
int ret;
OS_impl_timebase_internal_record_t *local;
OS_object_token_t token;
OS_impl_timebase_internal_record_t *impl;
OS_timebase_internal_record_t * timebase;
uint32 interval_time;
int sig;

local = &OS_impl_timebase_table[timer_id];

ret = sigwait(&local->sigset, &sig);
interval_time = 0;

if (ret != 0)
{
/*
* the sigwait call failed.
* returning 0 will cause the process to repeat.
*/
interval_time = 0;
}
else if (local->reset_flag == 0)
if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TIMEBASE, obj_id, &token) == OS_SUCCESS)
{
/*
* Normal steady-state behavior.
* interval_time reflects the configured interval time.
*/
interval_time = OS_timebase_table[timer_id].nominal_interval_time;
}
else
{
/*
* Reset/First interval behavior.
* timer_set() was invoked since the previous interval occurred (if any).
* interval_time reflects the configured start time.
*/
interval_time = OS_timebase_table[timer_id].nominal_start_time;
local->reset_flag = 0;
impl = OS_OBJECT_TABLE_GET(OS_impl_timebase_table, token);
timebase = OS_OBJECT_TABLE_GET(OS_timebase_table, token);

ret = sigwait(&impl->sigset, &sig);

if (ret != 0)
{
/*
* the sigwait call failed.
* returning 0 will cause the process to repeat.
*/
}
else if (impl->reset_flag == 0)
{
/*
* Normal steady-state behavior.
* interval_time reflects the configured interval time.
*/
interval_time = timebase->nominal_interval_time;
}
else
{
/*
* Reset/First interval behavior.
* timer_set() was invoked since the previous interval occurred (if any).
* interval_time reflects the configured start time.
*/
interval_time = timebase->nominal_start_time;
impl->reset_flag = 0;
}
}

return interval_time;
Expand Down
50 changes: 28 additions & 22 deletions src/os/rtems/src/os-impl-timebase.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,33 +162,39 @@ static rtems_timer_service_routine OS_TimeBase_ISR(rtems_id rtems_timer_id, void
* Pends on the semaphore for the next timer tick
*
*-----------------------------------------------------------------*/
static uint32 OS_TimeBase_WaitImpl(osal_index_t local_id)
static uint32 OS_TimeBase_WaitImpl(osal_id_t timebase_id)
{
OS_impl_timebase_internal_record_t *local;
OS_object_token_t token;
OS_impl_timebase_internal_record_t *impl;
uint32 tick_time;

local = &OS_impl_timebase_table[local_id];

/*
* Pend for the tick arrival
*/
rtems_semaphore_obtain(local->tick_sem, RTEMS_WAIT, RTEMS_NO_TIMEOUT);
tick_time = 0;

/*
* Determine how long this tick was.
* Note that there are plenty of ways this become wrong if the timer
* is reset right around the time a tick comes in. However, it is
* impossible to guarantee the behavior of a reset if the timer is running.
* (This is not an expected use-case anyway; the timer should be set and forget)
*/
if (local->reset_flag == 0)
if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TIMEBASE, timebase_id, &token) == OS_SUCCESS)
{
tick_time = local->configured_interval_time;
}
else
{
tick_time = local->configured_start_time;
local->reset_flag = 0;
impl = OS_OBJECT_TABLE_GET(OS_impl_timebase_table, token);

/*
* Pend for the tick arrival
*/
rtems_semaphore_obtain(impl->tick_sem, RTEMS_WAIT, RTEMS_NO_TIMEOUT);

/*
* Determine how long this tick was.
* Note that there are plenty of ways this become wrong if the timer
* is reset right around the time a tick comes in. However, it is
* impossible to guarantee the behavior of a reset if the timer is running.
* (This is not an expected use-case anyway; the timer should be set and forget)
*/
if (impl->reset_flag == 0)
{
tick_time = impl->configured_interval_time;
}
else
{
tick_time = impl->configured_start_time;
impl->reset_flag = 0;
}
}

return tick_time;
Expand Down
2 changes: 1 addition & 1 deletion src/os/vxworks/inc/os-vxworks.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ int32 OS_VxWorks_DirAPI_Impl_Init(void);
int OS_VxWorks_TaskEntry(int arg);
int OS_VxWorks_ConsoleTask_Entry(int arg);

uint32 OS_VxWorks_SigWait(osal_index_t local_id);
uint32 OS_VxWorks_SigWait(osal_id_t timebase_id);
int OS_VxWorks_TimeBaseTask(int arg);
void OS_VxWorks_RegisterTimer(osal_id_t obj_id);
void OS_VxWorks_UsecToTimespec(uint32 usecs, struct timespec *time_spec);
Expand Down
26 changes: 12 additions & 14 deletions src/os/vxworks/src/os-impl-timebase.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,26 +148,24 @@ void OS_VxWorks_UsecToTimespec(uint32 usecs, struct timespec *time_spec)
* Blocks the calling task until the timer tick arrives
*
*-----------------------------------------------------------------*/
uint32 OS_VxWorks_SigWait(osal_index_t local_id)
uint32 OS_VxWorks_SigWait(osal_id_t timebase_id)
{
OS_impl_timebase_internal_record_t *local;
OS_common_record_t * global;
osal_id_t active_id;
OS_object_token_t token;
OS_impl_timebase_internal_record_t *impl;
uint32 tick_time;
int signo;
int ret;

local = &OS_impl_timebase_table[local_id];
global = &OS_global_timebase_table[local_id];
active_id = global->active_id;
tick_time = 0;

if (OS_ObjectIdDefined(active_id) && local->assigned_signal > 0)
if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TIMEBASE, timebase_id, &token) == OS_SUCCESS)
{
impl = OS_OBJECT_TABLE_GET(OS_impl_timebase_table, token);

/*
* Pend for the tick arrival
*/
ret = sigwait(&local->timer_sigset, &signo);
ret = sigwait(&impl->timer_sigset, &signo);

/*
* The sigwait() can be interrupted....
Expand All @@ -187,17 +185,17 @@ uint32 OS_VxWorks_SigWait(osal_index_t local_id)
* conditions. Samples from before/after a reconfig
* are generally not comparable.
*/
if (ret == OK && signo == local->assigned_signal && OS_ObjectIdEqual(global->active_id, active_id))
if (ret == OK && signo == impl->assigned_signal)
{
if (local->reset_flag)
if (impl->reset_flag)
{
/* first interval after reset, use start time */
tick_time = local->configured_start_time;
local->reset_flag = false;
tick_time = impl->configured_start_time;
impl->reset_flag = false;
}
else
{
tick_time = local->configured_interval_time;
tick_time = impl->configured_interval_time;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tests/time-base-api-test/time-base-api-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
#include "uttest.h"
#include "utbsp.h"

static uint32 UT_TimerSync(osal_index_t timer_id)
static uint32 UT_TimerSync(osal_id_t timer_id)
{
OS_TaskDelay(1);
return 1;
Expand Down
2 changes: 1 addition & 1 deletion src/unit-test-coverage/shared/src/coveragetest-timebase.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ static uint32 TimerSyncCount = 0;
static uint32 TimerSyncRetVal = 0;
static uint32 TimeCB = 0;

static uint32 UT_TimerSync(osal_index_t timer_id)
static uint32 UT_TimerSync(osal_id_t timer_id)
{
++TimerSyncCount;
return TimerSyncRetVal;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void UT_TimeBaseTest_Setup(osal_index_t local_id, int signo, bool reset_flag);
* Invokes OS_VxWorks_SigWait() with the given arguments.
* This is normally a static function but exposed via a non-static wrapper for UT purposes.
*/
int32 UT_TimeBaseTest_CallSigWaitFunc(osal_index_t local_id);
int32 UT_TimeBaseTest_CallSigWaitFunc(osal_id_t timebase_id);

/* Invokes the static OS_VxWorks_TimeBaseTask() function with given argument */
int UT_TimeBaseTest_CallHelperTaskFunc(int arg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ int32 UT_Call_OS_VxWorks_TimeBaseAPI_Impl_Init(void)
return OS_VxWorks_TimeBaseAPI_Impl_Init();
}

int32 UT_TimeBaseTest_CallSigWaitFunc(osal_index_t local_id)
int32 UT_TimeBaseTest_CallSigWaitFunc(osal_id_t timebase_id)
{
return OS_VxWorks_SigWait(local_id);
return OS_VxWorks_SigWait(timebase_id);
}

int UT_TimeBaseTest_CallHelperTaskFunc(int arg)
Expand Down
6 changes: 3 additions & 3 deletions src/unit-test-coverage/vxworks/src/coveragetest-timebase.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,11 @@ void Test_OS_VxWorks_SigWait(void)
OS_TimeBaseSet_Impl(&token, 1111111, 2222222);

UT_SetDataBuffer(UT_KEY(OCS_sigwait), &signo, sizeof(signo), false);
OSAPI_TEST_FUNCTION_RC(UT_TimeBaseTest_CallSigWaitFunc(UT_INDEX_0), 1111111);
OSAPI_TEST_FUNCTION_RC(UT_TimeBaseTest_CallSigWaitFunc(OS_OBJECT_ID_UNDEFINED), 1111111);
UT_SetDataBuffer(UT_KEY(OCS_sigwait), &signo, sizeof(signo), false);
OSAPI_TEST_FUNCTION_RC(UT_TimeBaseTest_CallSigWaitFunc(UT_INDEX_0), 2222222);
OSAPI_TEST_FUNCTION_RC(UT_TimeBaseTest_CallSigWaitFunc(OS_OBJECT_ID_UNDEFINED), 2222222);
UT_SetDataBuffer(UT_KEY(OCS_sigwait), &signo, sizeof(signo), false);
OSAPI_TEST_FUNCTION_RC(UT_TimeBaseTest_CallSigWaitFunc(UT_INDEX_0), 2222222);
OSAPI_TEST_FUNCTION_RC(UT_TimeBaseTest_CallSigWaitFunc(OS_OBJECT_ID_UNDEFINED), 2222222);

UT_TimeBaseTest_Setup(UT_INDEX_0, 0, false);
OS_global_timebase_table[0].active_id = OS_OBJECT_ID_UNDEFINED;
Expand Down

0 comments on commit cf9db6d

Please sign in to comment.