Skip to content

Fix #11, Update OS_TimerSet to return OS_ERROR #19

Merged
skliper merged 1 commit intonasa:merge-20191001from
avan989:OS_ERROR
Oct 1, 2019
Merged

Fix #11, Update OS_TimerSet to return OS_ERROR #19
skliper merged 1 commit intonasa:merge-20191001from
avan989:OS_ERROR

Conversation

@avan989
Copy link
Contributor

@avan989 avan989 commented Sep 17, 2019 •
edited by skliper
Loading

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

jphickey
jphickey approved these changes Sep 17, 2019
View reviewed changes
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

skliper
skliper suggested changes Sep 18, 2019
View reviewed changes
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:

osal/src/unit-test-coverage/shared/src/coveragetest-time.c

Lines 153 to 181 in d0f1a39

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 mentioned this pull request Oct 2, 2019
Merged
@skliper skliper linked an issue May 13, 2020 that may be closed by this pull request
Return OS_ERROR if OS_TimerSet is called with both parameters as zero #11
Closed
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
@skliper
Merge pull request nasa#19 from matzipan/patch-1
f26967b 
Fix code markup in readme
dzbaker pushed a commit that referenced this pull request Jan 15, 2026
@arielswalker
Fix #19, Remove Cache for Standalone Build
a05985e
dzbaker pushed a commit that referenced this pull request Jan 15, 2026
@arielswalker
Merge pull request #20 from cFS/fix-19-standalone-cache
dc42732 
Fix #19, Remove Cache for Standalone Build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jphickey jphickey jphickey approved these changes

+1 more reviewer

@skliper skliper skliper approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CCB:Approved Indicates code review and approval by community CCB

Projects

None yet

Milestone

Bootes

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

@avan989 @skliper @jphickey