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 #121, Unify CFE PSP with OSAL BSP #149

Merged

Conversation

jphickey
Copy link
Contributor

@jphickey jphickey commented Apr 3, 2020

Describe the contribution
Instead of redefining the basic entry point symbols, use the OSAL BSP and its abstractions. The CFE PSP just becomes an extension to the OSAL BSP, and only needs to define the OS_Application_Startup symbol like any other OSAL application.

Fixes #121

Testing performed
Build code for all supported targets (ppc-vxworks6.9, i686-rtems4.11, native/x86-64 linux). Verify clean build. Confirm CFE boots and responds to commands as normal for RTEMS and native linux targets.

Expected behavior changes
No behavior changes. Code structure/linking change only.

System(s) tested on
Ubuntu 18.04 LTS 64 bit (native, build host)
QEMU for RTEMS 4.11

Additional context
VxWorks testing pending on availability of test platform

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

@jphickey
Copy link
Contributor Author

jphickey commented Apr 3, 2020

Note - this pull request depends on the OSAL BSP changes associated with nasa/osal#261 in pull request nasa/osal#404.

It should be merged at the same cycle that pull request.

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.

Love it.

@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Apr 6, 2020
@astrogeco
Copy link
Contributor

CCB 20200408 - APPROVED

@astrogeco astrogeco added CCB - 20200408 CCB:Approved Indicates Approval by CCB and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Apr 8, 2020
@skliper
Copy link
Contributor

skliper commented Apr 13, 2020

I checked out nasa/osal#404 along with this branch and build failed for SIMULATION=native, ENABLE_UNIT_TESTS=true...

/export/home/jhageman/cFS/cFS-GitHub/psp/fsw/pc-linux/src/cfe_psp_memory.c: In function ‘CFE_PSP_InitProcessorReservedMemory’:
/export/home/jhageman/cFS/cFS-GitHub/psp/fsw/pc-linux/src/cfe_psp_memory.c:670:60: error: ‘S_IRWXU’ undeclared (first use in this function)
    tempFd = open(CFE_PSP_CDS_KEY_FILE, O_RDONLY | O_CREAT, S_IRWXU );

@skliper
Copy link
Contributor

skliper commented Apr 13, 2020

See also https://travis-ci.com/github/skliper/cFS/jobs/318899041, shows failure with the two branches. Is there another pull that needs to be included? @jphickey

@jphickey
Copy link
Contributor Author

Did you also include nasa/cFE#585? This needs to be merged together with the others.

Looking at the CFE, I'm not seeing this merged into the integration-candidate yet. Note that the CFE merge can happen before the OSAL/PSP ones (it will have no effect) but it cannot happen after.

@skliper
Copy link
Contributor

skliper commented Apr 13, 2020

Did you also include nasa/cFE#585? This needs to be merged together with the others.

That worked. Would help to have the version information covering what was tested in the pull request, it's not clear what was tested here.

@skliper
Copy link
Contributor

skliper commented Apr 13, 2020

18/56 Test #18: coverage-shared-filesys ..........***Failed 0.01 sec

Using nasa/cFE#585, #149, nasa/osal#404 with the bundle

@skliper
Copy link
Contributor

skliper commented Apr 13, 2020

4 failures in the test... fixing them now. Related to nasa/osal#387

EDIT - added link to pull that caused them

Instead of redefining the basic entry point symbols, use the
OSAL BSP and its abstractions.  The CFE PSP just becomes an
extension to the OSAL BSP, and only needs to define the
OS_Application_Startup symbol like any other OSAL application.
@skliper skliper force-pushed the fix-121-unify-osal-bsp-psp branch from 36c6974 to ab6aa2a Compare April 13, 2020 20:29
@skliper
Copy link
Contributor

skliper commented Apr 13, 2020

Rebased on master so it can be tested with the bundle

@astrogeco astrogeco changed the base branch from master to integration-candidate April 14, 2020 21:28
@astrogeco astrogeco merged commit 801bf27 into nasa:integration-candidate Apr 14, 2020
@skliper skliper added this to the 1.5.0 milestone Jun 1, 2020
@jphickey jphickey deleted the fix-121-unify-osal-bsp-psp branch January 6, 2021 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates Approval by CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve consistency and reduce duplication in PSP BSP implementation
3 participants