Skip to content

Integration Candidate: Fast Track#78

Merged
astrogeco merged 4 commits intomasterfrom
integration-candidate
May 5, 2020
Merged

Integration Candidate: Fast Track#78
astrogeco merged 4 commits intomasterfrom
integration-candidate

Conversation

@astrogeco
Copy link
Contributor

@astrogeco astrogeco commented May 4, 2020

Describe the contribution
Combines the following Fast Track Pull Requests
osal: nasa/osal#444
cfe: nasa/cFE#672

Testing performed
See PRs
Bundle CI - https://github.com/nasa/cFS/pull/78/checks?check_run_id=644721591

Expected behavior changes
See osal PR and cFE PR

System(s) tested on
Bundle CI - Ubuntu:Bionic

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

Gerardo E. Cruz-Ortiz and others added 2 commits May 4, 2020 15:42
@astrogeco
Copy link
Contributor Author

@jphickey see Travis unit test failures

 45 - bin-sem-flush-test (Child aborted)

	 46 - bin-sem-test (Child aborted)

	 47 - bin-sem-timeout-test (Child aborted)

	 48 - count-sem-test (Child aborted)

	 49 - file-api-test (Child aborted)

	 50 - mutex-test (Child aborted)

	 51 - osal-core-test (Child aborted)

	 52 - queue-timeout-test (Child aborted)

	 53 - sem-speed-test (Child aborted)

	 54 - symbol-api-test (Child aborted)

	 55 - timer-test (Child aborted)

	 56 - osal_core_UT (Failed)

	 57 - osal_loader_UT (Child aborted)

	 58 - osal_filesys_UT (Child aborted)

	 59 - osal_file_UT (Failed)

	 60 - osal_network_UT (Child aborted)

	 61 - osal_timer_UT (Child aborted)

@astrogeco astrogeco requested a review from skliper May 4, 2020 20:34
@astrogeco astrogeco marked this pull request as draft May 4, 2020 20:34
@jphickey
Copy link
Contributor

jphickey commented May 4, 2020

The issue is that it still wasn't picking up the PERMISSIVE mode setting. Two minor fixups solve it:

  • had to propagate the native_osconfig.cmake from my local work area to cfe/cmake/sample_defs directory so this build will also pick it up. Patched by nasa/cFE@4fba145.
  • OSAL also wasn't picking up the file because include() needed a foreach loop (a silent failure, it only read the first entry, ignoring the second, which is the one that had PERMISSIVE...). Patched by nasa/osal@20f8b18

@astrogeco
Copy link
Contributor Author

The issue is that it still wasn't picking up the PERMISSIVE mode setting. Two minor fixups solve it:

* had to propagate the `native_osconfig.cmake` from my local work area to `cfe/cmake/sample_defs` directory so this build will also pick it up.  Patched by [nasa/cFE@4fba145](https://github.com/nasa/cFE/commit/4fba145).

* OSAL also wasn't picking up the file because `include()` needed a foreach loop (a silent failure, it only read the first entry, ignoring the second, which is the one that had PERMISSIVE...).  Patched by [nasa/osal@20f8b18](https://github.com/nasa/osal/commit/20f8b18)

Can you add those to the respective IC branches?

jphickey added 2 commits May 4, 2020 17:28
Patch to resolve warnings about undefined references.
@skliper
Copy link
Contributor

skliper commented May 5, 2020

Could we call it osconfig-doxygen.h as a slightly more specific name?

@skliper
Copy link
Contributor

skliper commented May 5, 2020

Could we call it osconfig-doxygen.h as a slightly more specific name?

Or could the osconfig.h.in itself be passed in? I don't think it has to be a .h file.

@astrogeco astrogeco marked this pull request as ready for review May 5, 2020 14:27
@jphickey
Copy link
Contributor

jphickey commented May 5, 2020

Or could the osconfig.h.in itself be passed in? I don't think it has to be a .h file.

Tried this and it doesn't work ... doxygen appears to just ignore it.

I chose osconfig-example.h as it this is what it is (an example without real values) for documentation purposes. This only exists in the build dir, not in source tree obviously, so this name is never used except it will appear in the generated docs when you click on the links to stuff in this file.

Is this the only thing holding this up at this point? I have several other branches that need to be rebased after this goes to "master", so I'd like to move past this.

I recommend we just move forward with the way it is but if you don't like the name I chose, please just tell me specifically what name you will like, and I will patch it again.

@skliper
Copy link
Contributor

skliper commented May 5, 2020

Or could the osconfig.h.in itself be passed in? I don't think it has to be a .h file.

Tried this and it doesn't work ... doxygen appears to just ignore it.

It's possible (FILE_PATTERN, EXTENSION_MAPPING), but by the time I got it working it felt more complex than your example implementation. Lets just go with this.

@astrogeco astrogeco merged commit 8f8ec69 into master May 5, 2020
@skliper skliper added this to the 6.8.0 milestone Jun 1, 2020
chillfig pushed a commit to chillfig/cFS that referenced this pull request Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants