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 #196, Fix timer-test to be able to use OS_MAX_TIMERS value #490

Conversation

yammajamma
Copy link
Contributor

Fix #196, Fix timer-test to be able to use OS_MAX_TIMERS value

Describe the contribution
Changed timer-test to be able to use OS_MAX_TIMERS value on top of the hard-coded NUMBER_OF_TIMERS value. This will allow the test to be functional even if the OS_MAX_TIMERS value is reconfigured.

Testing performed
Steps taken to test the contribution:

  1. Standard build and ran unit tests.

System(s) tested on
cFS Dev Server
OS: Ubuntu 18.04
Versions: OSAL 5.0.11.0

Contributor Info - All information REQUIRED for consideration of pull request
Yasir Majeed Khan
NASA GSFC/Emergent Space

@yammajamma
Copy link
Contributor Author

@skliper, @jphickey would you mind reviewing this when you get a chance? Thanks!

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.

Minor change request.

Just to confirm - did you build and run this test with OS_MAX_TIMERS set to something less than 4 to confirm it works as expected?

void TimerTestSetup(void);
void TimerTestTask(void);
void TimerTestCheck(void);

OS_time_t StartTime;
OS_time_t EndTime;
uint32 TimerStart[NUMBER_OF_TIMERS];
uint32 TimerInterval[NUMBER_OF_TIMERS];

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks odd - variable declared without initializer and then defined with initializer. Possibly a remnant change? Should remove this addition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a typo. Fixed.

@yammajamma
Copy link
Contributor Author

@jphickey Yes, I tested it with less than 4 timers, etc. and it worked as expected. I'm pushing the other change now.

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.

OK looks good.

@astrogeco astrogeco added CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) CCB-20200624 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jun 24, 2020
@astrogeco
Copy link
Contributor

CCB-20200624 APPROVED

@astrogeco astrogeco added the CCB:Approved Indicates code review and approval by community CCB label Jul 1, 2020
@astrogeco astrogeco changed the base branch from master to integration-candidate July 1, 2020 13:49
@astrogeco astrogeco merged commit 83a3c42 into nasa:integration-candidate Jul 1, 2020
@skliper skliper added this to the 5.1.0 milestone Aug 21, 2020
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
Integration Candidate 20200121
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.

Functional Timer Test Hard Codes Configuration Value
4 participants