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

Fix #540, add event callback framework #541

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions src/os/inc/osapi-os-core.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,75 @@ typedef enum
OS_STREAM_STATE_WRITABLE = 0x08, /**< @brief whether the stream is writable */
} OS_StreamState_t;

/**
* @brief A set of events that can be used with event callback routines
*/
typedef enum
{
OS_EVENT_RESERVED = 0, /**< no-op/reserved event id value */

/**
* resource/id has been newly allocated but not yet created.
*
* This event is invoked from WITHIN the locked region, in
* the context of the task which is allocating the resource.
*
* If the handler returns non-success, the error will be returned
* to the caller and the creation process is aborted.
*/
OS_EVENT_RESOURCE_ALLOCATED,

/**
* resource/id has been fully created/finalized.
*
* Invoked outside locked region, in the context
* of the task which created the resource.
*
* Data object is not used, passed as NULL.
*
* Return value is ignored - this is for information purposes only.
*/
OS_EVENT_RESOURCE_CREATED,

/**
* resource/id has been deleted.
*
* Invoked outside locked region, in the context
* of the task which deleted the resource.
*
* Data object is not used, passed as NULL.
*
* Return value is ignored - this is for information purposes only.
*/
OS_EVENT_RESOURCE_DELETED,

/**
* New task is starting.
*
* Invoked outside locked region, in the context
* of the task which is currently starting, before
* the entry point is called.
*
* Data object is not used, passed as NULL.
*
* If the handler returns non-success, task startup is aborted
* and the entry point is not called.
*/
OS_EVENT_TASK_STARTUP,

OS_EVENT_MAX /**< placeholder for end of enum, not used */
} OS_Event_t;

/**
* @brief A callback routine for event handling.
*
* @param[in] event The event that occurred
* @param[in] object_id The associated object_id, or 0 if not associated with an object
* @param[inout] data An abstract data/context object associated with the event, or NULL.
* @return status Execution status, see @ref OSReturnCodes.
*/
typedef int32 (*OS_EventHandler_t)(OS_Event_t event, osal_id_t object_id, void *data);

/**
* @brief For the @ref OS_GetErrorName() function, to ensure
* everyone is making an array of the same length.
Expand Down Expand Up @@ -504,6 +573,25 @@ void OS_ForEachObject (osal_id_t creator_id, OS_ArgCallback_t callback
* @param[in] callback_arg Opaque Argument to pass to callback function
*/
void OS_ForEachObjectOfType (uint32 objtype, osal_id_t creator_id, OS_ArgCallback_t callback_ptr, void *callback_arg);

/*-------------------------------------------------------------------------------------*/
/**
* @brief Callback routine registration
*
* This hook enables the application code to perform extra platform-specific
* operations on various system events such as resource creation/deletion.
*
* @note Some events are invoked while the resource is "locked" and therefore
* application-defined handlers for these events should not block or attempt
* to access other OSAL resources.
*
* @param[in] handler The application-provided event handler
* @return Execution status, see @ref OSReturnCodes.
* @retval #OS_SUCCESS @copybrief OS_SUCCESS
* @retval #OS_ERROR @copybrief OS_ERROR
*/
int32 OS_RegisterEventHandler (OS_EventHandler_t handler);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make sense to have a bitmask so the caller can specify which events are of interest. This idea also scales forward as more events are added. It could be 32-bit or 64-bit number which gives that many extra bits for future use. Perhaps 0 means "all events."


/**@}*/


Expand Down
16 changes: 16 additions & 0 deletions src/os/shared/inc/os-shared-common.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ struct OS_shared_global_vars
int32 MicroSecPerTick;
int32 TicksPerSecond;

/*
* The event handler is an application-defined callback
* that gets invoked as resources are created/configured/deleted.
*/
OS_EventHandler_t EventHandler;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can there only be one? It might be useful to follow the observer/observable pattern and have a list of observers, so that apps can monitor events as well to get an idea of the state of the system. I am not fully briefed on the use-case for this work, so this thought may be beyond scope.


#ifdef OSAL_CONFIG_DEBUG_PRINTF
uint8 DebugLevel;
#endif
Expand All @@ -69,6 +75,16 @@ struct OS_shared_global_vars
*/
extern OS_SharedGlobalVars_t OS_SharedGlobalVars;

/*---------------------------------------------------------------------------------------
Name: OS_NotifyEvent
Purpose: Notify the user application of a change in the state of an OSAL resource
returns: OS_SUCCESS on success, or relevant error code
---------------------------------------------------------------------------------------*/
int32 OS_NotifyEvent(OS_Event_t event, osal_id_t object_id, void *data);


/*---------------------------------------------------------------------------------------
Name: OS_API_Impl_Init
Expand Down
43 changes: 43 additions & 0 deletions src/os/shared/src/osapi-common.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,36 @@ OS_SharedGlobalVars_t OS_SharedGlobalVars =
.ShutdownFlag = 0,
.MicroSecPerTick = 0, /* invalid, _must_ be set by implementation init */
.TicksPerSecond = 0, /* invalid, _must_ be set by implementation init */
.EventHandler = NULL,
#if defined(OSAL_CONFIG_DEBUG_PRINTF)
.DebugLevel = 1,
#endif
};


/*----------------------------------------------------------------
*
* Function: OS_NotifyEvent
*
* Purpose: Helper function to invoke the user-defined event handler
*
*-----------------------------------------------------------------*/
int32 OS_NotifyEvent(OS_Event_t event, osal_id_t object_id, void *data)
{
int32 status;

if (OS_SharedGlobalVars.EventHandler != NULL)
{
status = OS_SharedGlobalVars.EventHandler(event, object_id, data);
}
else
{
status = OS_SUCCESS;
}

return status;
}

/*
*********************************************************************************
* PUBLIC API (application-callable functions)
Expand Down Expand Up @@ -199,6 +224,24 @@ int32 OS_API_Init(void)
return(return_code);
} /* end OS_API_Init */

/*----------------------------------------------------------------
*
* Function: OS_RegisterEventHandler
*
* Purpose: Implemented per public OSAL API
* See description in API and header file for detail
*
*-----------------------------------------------------------------*/
int32 OS_RegisterEventHandler (OS_EventHandler_t handler)
{
if (handler == NULL)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it an error if null is passed in? The effect would be to say "I am no longer interested in notifications," which seems like would not be an error.

{
return OS_INVALID_POINTER;
}

OS_SharedGlobalVars.EventHandler = handler;
return OS_SUCCESS;
}

/*----------------------------------------------------------------
*
Expand Down
27 changes: 27 additions & 0 deletions src/os/shared/src/osapi-idmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,7 @@ void OS_Unlock_Global(uint32 idtype)
int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_common_record_t *record, osal_id_t *outid)
{
uint32 idtype = OS_ObjectIdToType_Impl(record->active_id);
osal_id_t callback_id;

/* if operation was unsuccessful, then clear
* the active_id field within the record, so
Expand All @@ -708,6 +709,9 @@ int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_common_record_t *record,
OS_objtype_state[idtype].last_id_issued = record->active_id;
}

/* snapshot the ID for callback - will be needed after unlock */
callback_id = record->active_id;

if (outid != NULL)
{
/* always write the final value to the output buffer */
Expand All @@ -717,6 +721,12 @@ int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_common_record_t *record,
/* Either way we must unlock the object type */
OS_Unlock_Global(idtype);

/* Give event callback to the application */
if (OS_ObjectIdDefined(callback_id))
{
OS_NotifyEvent(OS_EVENT_RESOURCE_CREATED, callback_id, NULL);
}

return operation_status;
} /* end OS_ObjectIdFinalizeNew */

Expand All @@ -729,16 +739,28 @@ int32 OS_ObjectIdFinalizeNew(int32 operation_status, OS_common_record_t *record,
int32 OS_ObjectIdFinalizeDelete(int32 operation_status, OS_common_record_t *record)
{
uint32 idtype = OS_ObjectIdToType_Impl(record->active_id);
osal_id_t callback_id;

/* Clear the OSAL ID if successful - this returns the record to the pool */
if (operation_status == OS_SUCCESS)
{
callback_id = record->active_id;
record->active_id = OS_OBJECT_ID_UNDEFINED;
}
else
{
callback_id = OS_OBJECT_ID_UNDEFINED;
}

/* Either way we must unlock the object type */
OS_Unlock_Global(idtype);

/* Give event callback to the application */
if (OS_ObjectIdDefined(callback_id))
{
OS_NotifyEvent(OS_EVENT_RESOURCE_DELETED, callback_id, NULL);
}

return operation_status;
}

Expand Down Expand Up @@ -1023,6 +1045,11 @@ int32 OS_ObjectIdAllocateNew(uint32 idtype, const char *name, uint32 *array_inde
return_code = OS_ObjectIdFindNext(idtype, array_index, record);
}

if (return_code == OS_SUCCESS)
{
return_code = OS_NotifyEvent(OS_EVENT_RESOURCE_ALLOCATED, (*record)->active_id, NULL);
}

/* If allocation failed for any reason, unlock the global.
* otherwise the global should stay locked so remaining initialization can be done */
if (return_code != OS_SUCCESS)
Expand Down
6 changes: 6 additions & 0 deletions src/os/shared/src/osapi-task.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ static int32 OS_TaskPrepare(osal_id_t task_id, osal_task_entry *entrypt)
return_code = OS_TaskRegister_Impl(task_id);
}

if (return_code == OS_SUCCESS)
{
/* Give event callback to the application */
return_code = OS_NotifyEvent(OS_EVENT_TASK_STARTUP, task_id, NULL);
}

if (return_code != OS_SUCCESS)
{
*entrypt = NULL;
Expand Down
34 changes: 34 additions & 0 deletions src/ut-stubs/osapi-utstub-common.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,40 @@ int32 OS_API_Init(void)
return status;
}

/*****************************************************************************
*
* Stub function for OS_NotifyEvent()
*
*****************************************************************************/
int32 OS_NotifyEvent(OS_Event_t event, osal_id_t object_id, void *data)
{
UT_Stub_RegisterContextGenericArg(UT_KEY(OS_NotifyEvent), event);
UT_Stub_RegisterContextGenericArg(UT_KEY(OS_NotifyEvent), object_id);
UT_Stub_RegisterContextGenericArg(UT_KEY(OS_NotifyEvent), data);

int32 status;

status = UT_DEFAULT_IMPL(OS_NotifyEvent);

return status;
}

/*****************************************************************************
*
* Stub function for OS_RegisterEventHandler()
*
*****************************************************************************/
int32 OS_RegisterEventHandler (OS_EventHandler_t handler)
{
UT_Stub_RegisterContextGenericArg(UT_KEY(OS_RegisterEventHandler), handler);

int32 status;

status = UT_DEFAULT_IMPL(OS_RegisterEventHandler);

return status;
}


/*****************************************************************************
*
Expand Down