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

Integration Candidate 20191030 #281

Merged
merged 10 commits into from
Nov 7, 2019
Merged

Integration Candidate 20191030 #281

merged 10 commits into from
Nov 7, 2019

Conversation

skliper
Copy link
Contributor

@skliper skliper commented Oct 31, 2019

Describe the contribution
Fixes #230, fixes #266, fixes #269
Fixes #271, fixes #272, fixes #273, fixes #274

Testing performed
Steps taken to test the contribution:

  1. Checked out bundle with OSAL and cFE ic-20191030 branches
  2. make ENABLE_UNIT_TESTS=TRUE SIMULATION=native prep
  3. make
  4. make install
  5. make test
    Built without warnings, all tests passed except osal_timer_UT (nominal result on linux)
    executed cfe, successful startup with no warnings

Expected behavior changes
Coverage test updates (more coverage and works for vxworks)
Timer bug fixes
sem-speed-test functional test fix for vxworks

System(s) tested on:

  • cFS dev server
  • OS: Ubuntu 16.04
  • Versions: bundle with OSAL and cFE ic-20191030 branches

Additional context
None

Contributor Info
Jacob Hageman - NASA/GSFC

jphickey and others added 10 commits October 21, 2019 12:06
Rename to "vxworks" and "posix", respectively, to match the
actual implementation names.  The build scripts rely on
the name being the same and will fail if different.

This also updates the default build to be vxworks rather
than vxworks6, if not specified on the command line.
Add numerous unit test cases to improve the code coverage
ratios on the vxworks and shared implementation layers.

Prior to this change set, the coverage ratio was:
  lines......: 90.4% (2549 of 2820 lines)
  functions..: 95.9% (306 of 319 functions)

After this change set, the coverage ratio is:
  lines......: 99.9% (2846 of 2849 lines)
  functions..: 100.0% (330 of 330 functions)

Note these stats include some of the UT code itself,
and this is what added 11 functions.  No functions
were added to FSW code.

Note, one test condition will fail until the
fix for related bug #269 is merged.

This also fixes the posix coverage test so it
builds and runs, but coverage is still not
implemented here.
The timer code for VxWorks was fixed in bug #271 and the
coverage code test needs a corresponding update to cover
the code change.

This is kept as a separate update commit as neither
changeset is merged to master yet.
The test for failure of taskSpawn should be for the value of
ERROR, not 0, per the VxWorks API documentation.

Found when implementing the unit test improvements in #230.
This issue is generally only reproducible in UT where taskSpawn
can be made to fail.
Fixes #230 #266 #269
Code reviewed and approved at 20191023 CCB
On VxWorks, no minimum stack size is implemented,
and the implementation will create tasks with an
extremely small stack if "0" is passed in.

This causes the worker tasks to overrun the stack
on Vxworks, triggering undefined/inconsistent
behavior.
Cast the address value so it may be printed as an integer.
In VxWorks, the sigwait routine was not handling the actual time of
the first interval, which basically meant that the start_time was
elapsed twice before the first callback was generated.

This exposed a few other small but significant details regarding
how the intervals were dealt with across all operating systems.

For VxWorks and RTEMS, where the timer interval is rounded up to
an integer number of ticks, compute the actual interval time and
write a debug message in case it is different than the intended
value.  This makes it more obvious to the user that some
application/bsp changes might be necessary to get accurate timing.

For all operating systems, the "reset" flag needs to be sampled
AFTER the sigwait returns, not before it.  This flag gets set when
the OS_TimeBaseSet() API is called.

Removes an unused POSIX delay routine that was intended to support
use of clock_nanosleep() in place of the timer signal.  This mode
was never usable because OS_TimerCreate always allocates a signal and
fails if it cannot do so.  This code caused problems/incompatibilities
for the sigwait model which IS used, so it needed to be removed.
If the tick_time from the wait routine was perpetually
greater than the interval time, eventually the wait_time
became such that it was always below zero.  Once this
occurs, application callbacks would cease entirely.

To avoid this, run the callback condition in a loop
for periodic timers only (not for one-shot config).
Fixes #271 #272 #273 #274
Code reviewed and approved at 20191030 CCB
@skliper skliper requested a review from jphickey October 31, 2019 21:04
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.

Confirmed that this builds and runs as expected. Tested on my local linux machine and on the RTEMS test platform (QEMU).

Log of timer-test.exe execution for the record captured at: rtems-timer-test-ic-20191030.log

@skliper skliper merged commit 58d4d74 into master Nov 7, 2019
@skliper
Copy link
Contributor Author

skliper commented Nov 7, 2019

CCB 20191106 - approved for merge

@skliper skliper added this to the 5.1.0 milestone Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment