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 #11, Update OS_TimerSet to return OS_ERROR #19

Merged
merged 1 commit into from
Oct 1, 2019
Merged

Fix #11, Update OS_TimerSet to return OS_ERROR #19

merged 1 commit into from
Oct 1, 2019

Conversation

avan989
Copy link
Contributor

@avan989 avan989 commented Sep 17, 2019

Describe the contribution
Update OS_TimerSet to return OS_ERROR when both parameters are zero.

Fix #11

Testing performed
Steps taken to test the contribution:

  1. Build steps
  2. Modify sample app to make OS_TimerSet API call with both parameters as zero
  3. Verify return.

Expected behavior changes

  • API Change: OS_TimerSet

System(s) tested on:

  • Hardware
  • OS: Ubuntu 18.04.03
  • cFS 6.7, GroundSystem 2.1.0,

Contributor Info
Anh Van, NASA Goddard

Community contributors
You must attach a signed CLA (required for acceptance) or reference one already submitted

Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Contributor

@skliper skliper left a comment

Choose a reason for hiding this comment

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

Likely needs unit test update to cover this logic. Check if one of the test packages exercise this error, and if not add it.

@skliper
Copy link
Contributor

skliper commented Sep 18, 2019

See code below, ensure it covers and checks for the error case:

void Test_OS_TimerSet(void)
{
/*
* Test Case For:
* int32 OS_TimerSet(uint32 timer_id, uint32 start_time, uint32 interval_time)
*/
int32 expected = OS_SUCCESS;
int32 actual = OS_TimerSet(1, 0, 0);
UtAssert_True(actual == expected, "OS_TimerSet() (%ld) == OS_SUCCESS", (long)actual);
OS_timecb_table[2].timebase_ref = 0;
OS_timecb_table[2].flags = TIMECB_FLAG_DEDICATED_TIMEBASE;
OS_global_timebase_table[0].active_id = 2;
actual = OS_TimerSet(2, 0, 0);
UtAssert_True(actual == expected, "OS_TimerSet() (%ld) == OS_SUCCESS", (long)actual);
memset(OS_timecb_table, 0, sizeof(OS_timecb_table));
expected = OS_TIMER_ERR_INVALID_ARGS;
actual = OS_TimerSet(2, 1 << 31, 1 << 31);
UtAssert_True(actual == expected, "OS_TimerSet() (%ld) == OS_TIMER_ERR_INVALID_ARGS", (long)actual);
UT_SetForceFail(UT_KEY(OS_TaskGetId_Impl), 1 | (OS_OBJECT_TYPE_OS_TIMEBASE << OS_OBJECT_TYPE_SHIFT));
expected = OS_ERR_INCORRECT_OBJ_STATE;
actual = OS_TimerSet(2, 0, 0);
UtAssert_True(actual == expected, "OS_TimerSet() (%ld) == OS_ERR_INCORRECT_OBJ_STATE", (long)actual);
UT_ClearForceFail(UT_KEY(OS_TaskGetId_Impl));
}

@avan989 avan989 closed this Sep 23, 2019
@avan989 avan989 reopened this Sep 23, 2019
@avan989
Copy link
Contributor Author

avan989 commented Sep 23, 2019

update coverage test, unit test successful.

Had to do some modification to the original unit test, all unit test cases where both start time and interval time = 0 are no longer valid because of the new condition in OS_TimerSet. Changed those conditions to start_time = o and interval_time =1 to make valid:

[BEGIN] 04 OS_TimerSet
[ PASS] 04.001 coveragetest-time.c:161 - OS_TimerSet() (-1) == OS_ERROR
[ PASS] 04.002 coveragetest-time.c:165 - OS_TimerSet() (0) == OS_SUCCESS
[ PASS] 04.003 coveragetest-time.c:171 - OS_TimerSet() (0) == OS_SUCCESS
[ PASS] 04.004 coveragetest-time.c:176 - OS_TimerSet() (-29) == OS_TIMER_ERR_INVALID_ARGS
[ PASS] 04.005 coveragetest-time.c:181 - OS_TimerSet() (-35) == OS_ERR_INCORRECT_OBJ_STATE
[  END] 04 OS_TimerSet          TOTAL::5     PASS::5     FAIL::0      MIR::0      TSF::0      N/A::0

@skliper skliper added this to the 5.1.0 milestone Oct 1, 2019
@skliper skliper added the CCB:Approved Indicates code review and approval by community CCB label Oct 1, 2019
@skliper skliper changed the base branch from master to merge-20191001 October 1, 2019 19:12
@skliper skliper merged commit 58cf977 into nasa:merge-20191001 Oct 1, 2019
@skliper skliper linked an issue May 13, 2020 that may be closed by this pull request
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return OS_ERROR if OS_TimerSet is called with both parameters as zero
3 participants