new osal_id_t code and unit tests using osal stubs

[CDKnightNASA]

Describe the bug
The SBN application's coverage tests include tests of tasks, such as:

{
...
    PeerPtr->Connected  = true;
    PeerPtr->SendTaskID = 1;

    UT_SetDeferredRetcode(UT_KEY(OS_TaskGetId), 1, 1);
    UT_SetDeferredRetcode(UT_KEY(CFE_SB_RcvMsg), 2, -1);

    SBN_SendTask();
}

The issue is, now, the result of OS_TaskGetId() is an osal_id_t value that I need to compute to put into the SendTaskID field above. There is a helper function UT_ObjIdCompose() that could be used by unit tests but it is not currently exposed. For now, I've removed the UT_SetDeferredRetcode(...TaskGetId)...) call and instead I hand-copied the ID to write it ...SendTaskID = 1073807361 (the value returned by ObjIdCompose()) which is clearly not sustainable.

Expected behavior
Expose the ObjIdCompose() function from the OSAL stubs so that unit test code can generate matching ID's.

Reporter Info
Christopher.D.Knight@nasa.gov

[jphickey]

Well, the fact that OS_ObjIdCompose() is internal/hidden is actually a bit by design - to discourage apps from making too many assumptions about how object IDs are formed. They are supposed to be opaque, and we (as in, OSAL devs) reserve the right to change the bits as needed.

If all you want is a valid task ID - just call OS_TaskCreate(&PeerPtr->SendTaskID ... ) and the stub will assign you an ID, which should be the same ID that gets returned by default from OS_TaskGetID().

What's interesting is that this will be a different value than what you'd get by calling OS_ObjIdCompose() with OS_OBJECT_TYPE_OS_TASK ... the UT stubs actually use different bits for their IDs .... to (possibly) catch bad actors assuming they know what an OSAL ID is.

[CDKnightNASA]

I believe this will work for SBN, however if the code under test was calling OS_TaskCreate() and OS_TaskGetID(), having the test code also call OS_TaskCreate() will result in a different task ID. It's my understanding that I can't set a deferred return value for OS_TaskGetID()?

[jphickey]

Yeah, I can see that as being a possible future issue ... what we really should have is the ability for more control of what ID the OS_TaskCreate() call outputs as there may be test cases where it is necessary.

N.B. that despite using a typedef, the osal_id_t is still fundamentally an integer - right now it is just a typedef to uint32 so it really shouldn't have broken any code (I think?).

Did this actually break something or is this more of a future concern?

[jphickey]

It's my understanding that I can't set a deferred return value for OS_TaskGetID()?

Incorrect -- you should be able to use a deferred return code just fine. Did this not work?

In the current version of the stubs - the "status" you set reflects the integer index (0-based) and the stub converts this to a "fake task ID" ... maybe that is not appropriate for all use cases?

[CDKnightNASA]

See nasa/SBN@7305e53 -- I was calling UT_SetDeferredRetcode(UT_KEY(OS_TaskGetId), 1, 0); (yes, I realize that 0 is no longer a valid task ID) and calls to OS_TaskGetId() would return 1073807361.

[jphickey]

Right - the stub is interpreting the given value as an index - which is then converting to a UT-style task ID - which is actually different than normal task ID - as it uses UT_OBJTYPE_TASK (0x4001) rather than OS_OBJECT_TYPE_OS_TASK (0x0001) in the upper 16 bits. Are you sure you got 1073807361 (0x40010001)? I would have expected 1073807360 (0x40010000) in your SBN case.

In retrospect - I do think this stub is doing too much. Importantly there is no way to have it actually fail and have it return OS_OBJECT_ID_INVALID - which is a problem. I will look into fixing this. It probably shouldn't go through OS_ObjIdCompose() at all.

Also note that the first call to OS_TaskCreate() stub will return the same value as OS_TaskGetID() returns by default. So if a test case wants to know what value the task create is going to return without explicitly setting something, it can preemptively call OS_TaskGetID().