search

nasa / osal

The Core Flight System (cFS) Operating System Abstraction Layer (OSAL)
Apache License 2.0
556 stars 217 forks source link

Refactor common code between VxWorks/Posix/Rtems into OSAL shared layer #28

Closed skliper closed 5 years ago

skliper commented 5 years ago

The OSAL library is essentially three separate libraries sharing a common API. The VxWorks, Rtems, and Posix implementations do not share any code aside from the common osapi header files. Depending on the OS selection, one of the three libraries will be compiled.

However, under the hood, many of the operations within these three libraries are very similar. Although the final call down into the OS differs, much of the "housekeeping" that OSAL performs is similar between all three implementations.

Each of them maintain internal tables that map OS objects to names and numbers, and all the OSAL API calls check their arguments against these internal tables to see e.g. if an object by some ID is valid or if an object by some name already exists.

All this housekeeping //really should be identical// and it could be argued that any difference is really a bug and not a feature, as differences here could affect the portability of application code from one OS to another.

Refactoring this "common" functionality into a layer which is actually shared among OS implementations provides several benefits:

skliper commented 5 years ago

Imported from trac issue 5. Created by jphickey on 2014-12-26T23:11:53, last modified: 2019-07-26T15:13:27

skliper commented 5 years ago

Trac comment by jphickey on 2015-04-16 14:47:09:

This is now ready for review and test, commit [changeset:5964f49]

DEPENDENCY NOTE: This depends on some modifications that were done as part of tracs #35, #36, and #54. Therefore this changeset is based on an intermediate merge that contains all these fixes.

Since this change set involves a significant refactoring of code, in order to make integration as safe/painless as possible, this is implemented largely as a new OSAL for POSIX and RTEMS rather than modifying the existing implementations. Therefore, projects can choose whether to use this implementation or the existing one.

skliper commented 5 years ago

Trac comment by sduran on 2015-05-15 15:54:17:

Recommend accept. JSC will need to decide if/when the ARINC653 OSAL can be refactored to this method.

skliper commented 5 years ago

Trac comment by jwilmot on 2015-05-19 11:54:00:

Approve. Rationale is valid for the long term.

skliper commented 5 years ago

Trac comment by jphickey on 2015-06-02 13:51:19:

Updated commit (rebased): [changeset:89be104]

skliper commented 5 years ago

Trac comment by jphickey on 2015-08-17 12:23:46:

== REBASE ==

This has been rebased to an intermediate merge containing the necessary prerequisites for this (#35 and #36).

Updated commit: [changeset:dd360ee]

It has been merged into the ic-ccb-review branch for review at the next CCB.

skliper commented 5 years ago

Trac comment by glimes on 2015-08-25 11:40:34:

Approve.

Looks like this didn't get packaged into ic-ccb-review. I've added a Bamboo build plan branch for the trac-5 git branch, and results are available here:

https://babelfish.arc.nasa.gov/bamboo/browse/CFS-CFSOSAL69/latest

Results look good. Note that "classic" build is still rejected due to a missing file; this is covered in my comment 8 of ticket #46.

edit: the above Bamboo plan branch is now gone, as this changeset is now part of the ic-ccb-review merge.

skliper commented 5 years ago

Trac comment by jphickey on 2018-05-14 16:32:47:

This is still awaiting review/approval and should be a matter of discussion for the next OSAL release.

There are quite a few other tickets submitted over the years, from various sources, that this will also help address:

88 #94 #95 #108 #156 #171 #195 #69 #122 #73 #210 #219 #116 #204 #205 #203 #208

(not suggesting it solves //all// of them, although it does resolve a number of them, for the others it will simply reduce the effort needed to resolve).

skliper commented 5 years ago

Trac comment by jphickey on 2018-05-22 13:51:09:

Per discussion on 2018-05-22:

skliper commented 5 years ago

Trac comment by sduran on 2018-05-24 14:33:31:

The coverage tests for both posix and vxworks are indeed in the released OSAL.

cd osal/build and follow directions in the readme.txt file to build and run them.

unit test source is located here: osal/src/unit-tests

The readme file does not have this, but if you build as follows it will build, run (this will provide test pass, fail statistics), and should generate coverage data (for some reason, this does not seem to be working - need to resolve this issue).

$ make unit-tests gcov

skliper commented 5 years ago

Trac comment by sduran on 2018-05-24 14:33:47:

Has the vxwork implementation been added to osal-ng? I think we would need that to be able to fully switch over to osal-ng

skliper commented 5 years ago

Trac comment by jphickey on 2018-05-25 15:10:48:

Steve -- I want to make sure we are on the same page here. We currently have three types of unit tests in OSAL in the current master branch and I want to make sure we are talking about the same thing.

There is:

  1. osal/src/tests
  2. osal/src/unit-tests
  3. osal/src/unit-test-coverage

The first is the original OSAL tests. These are more of a "sanity check" and do not cover all features, but these run under all OS's supported (including RTEMS).

The second is the extended tests that JSC did and we merged a couple years ago, that covers the full feature set as exposed though the OSAL API. These support VxWorks and Posix but RTEMS is lacking. This is the one that I would like to better integrate with the test framework so we can support RTEMS or any other (future) OS. This is //not// a code coverage test, however, as it does not really attempt to cover every line of code, it validates that the features work.

The third is the coverage test that I was referring to. This is the one that intends to execute every code path, using stub functions in place of real OS calls. It is built with gcov support to obtain line-by-line coverage reports. There is only a "vxworks6" implementation of this, I do not see a posix implementation of this in the osal-4.2.1a tag or in the development branch (not that it really matters either way, as it would have to be modified for the "ng" variants anyway). I've actually started work on creating a version of this to support the "shared" layer that is the common part of the ng OSAL.

skliper commented 5 years ago

Trac comment by jphickey on 2018-05-25 15:16:23:

Replying to [comment:12 sduran]:

Has the vxwork implementation been added to osal-ng? I think we would need that to be able to fully switch over to osal-ng

As of yet, the answer is no. But as I said, merging this branch does not affect the existing vxworks6 implementation. They can coeexist, and they do not interfere with eachother.

The limiting factor is that (as of yet) there has not been a vxworks-based platform available for testing a vxworks-ng implementation. If someone has a target and toolchain that could be used for testing this, then the vxworks implementation could be done in short order.

skliper commented 5 years ago

Trac comment by sseeger on 2018-05-29 15:33:39:

I have an mcp750 box at home with vxworks 6.9 I can do testing of vxworks-ng with. If Lorraine and Jonathan want me to tackle vxworks-ng I would be happy to do it.

I probably should have returned the mcp750 back to Goddard... like 2 years ago or so. My cat sits on it and announces his demand to have his head scratched.

skliper commented 5 years ago

Trac comment by sduran on 2018-05-30 16:38:02:

Replying to [comment:13 jphickey]:

Steve -- I want to make sure we are on the same page here. We currently have three types of unit tests in OSAL in the current master branch and I want to make sure we are talking about the same thing.

There is:

  1. osal/src/tests
  2. osal/src/unit-tests
  3. osal/src/unit-test-coverage

The first is the original OSAL tests. These are more of a "sanity check" and do not cover all features, but these run under all OS's supported (including RTEMS).

The second is the extended tests that JSC did and we merged a couple years ago, that covers the full feature set as exposed though the OSAL API. These support VxWorks and Posix but RTEMS is lacking. This is the one that I would like to better integrate with the test framework so we can support RTEMS or any other (future) OS. This is //not// a code coverage test, however, as it does not really attempt to cover every line of code, it validates that the features work.

The third is the coverage test that I was referring to. This is the one that intends to execute every code path, using stub functions in place of real OS calls. It is built with gcov support to obtain line-by-line coverage reports. There is only a "vxworks6" implementation of this, I do not see a posix implementation of this in the osal-4.2.1a tag or in the development branch (not that it really matters either way, as it would have to be modified for the "ng" variants anyway). I've actually started work on creating a version of this to support the "shared" layer that is the common part of the ng OSAL.

This is all correct. We can get (or used to be able to) coverage out of unit-tests, but that was not their intent. Their intent is API/functional testing. We should, at a minimum, run these on the ng version and prove that we get the same results. I think this is probably sufficient for posix osal. Adding RTEMs to this would be good too.

For unit-test-coverage, the focus was on vxworks for the class A cert, there was no class A (not sure that we ever would) for linux. The coverage tests are specific to each osal. Providing full coverage unit tests for the common layer is a good start. Once there is a vxworks-ng version, we would need to update or probably just write a new set of vxworks-ng coverage tests so that we can continue to provide class A artifacts/support for the ng version - its hard to envision a project moving to the ng version with out this. I think the preference would be, if we are all ready to accept moving to -ng OSAL, we should move to it on all currently supported osals it and deprecate the existing one.

skliper commented 5 years ago

Trac comment by jphickey on 2018-08-07 13:08:17:

Latest commit is [changeset:6ec9d33]

skliper commented 5 years ago

Trac comment by jphickey on 2018-09-18 13:16:51:

Current commits for review:

This has been broken into multiple parts for CCB review purposes.

First set, which includes exact equivalence to existing API, with no new functions added: [changeset:100c231] Second set, which adds select and socket API: [changeset:e32b4c2] Third set, which adds coverage testing: [changeset:05c872f]

skliper commented 5 years ago

Trac comment by jphickey on 2018-10-11 17:33:49:

This was reviewed at the 2018-10-09 CCB and it is agreed to merge this to get it into the development branch and any possible future issues should be submitted as new issues change requests.

Some feedback was incorporated, to add additional comments/descriptions to the new Socket API calls. Additionally a C++ build flags issue was addressed.

Final commits:

  1. Basic change: [changeset:ea712585]
  2. Socket API addition: [changeset:6df485b5]
  3. Coverage test: [changeset:86683ad]

Integration candidate: branch ic-ccb-20181009 : [changeset:45ce509]

skliper commented 5 years ago

Trac comment by jphickey on 2019-03-08 12:35:35:

Update: This was merged to development branch based on approvals at the 2018-10-09 CCB. Therefore I think this can finally be closed after 4 years.

I have submitted two follow on tickets for the remaining work:

230: Improve coverage test for "shared" layer

231: Port VxWorks OSAL to shared/ng OSAL architecture

skliper commented 5 years ago

Trac comment by jhageman on 2019-03-20 12:55:43:

CCB Closed 3/20/19

skliper commented 5 years ago

Trac comment by jhageman on 2019-07-26 15:13:27:

Milestone renamed