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

DTNN-132 Clean up bplib prototype readme #260

Merged
merged 15 commits into from
Aug 1, 2024

Conversation

gskenned
Copy link
Contributor

@gskenned gskenned commented Jul 18, 2024

Describe the contribution
Updates the main README with descriptive text and example configure/build/test scripts.

Testing performed
Tested all steps described in:

  • The main README

Expected behavior changes
The README update will benefit users with tested, reusable configure/build/test scripts.

System(s) tested on

  • Hardware: AWS Elastic Container
  • OS: Ubuntu 20.04.6 LTS

Contributor Info - All information REQUIRED for consideration of pull request
Grafton Kennedy, Vantage Systems, Inc. (grafton.s.kennedy@nasa.gov)

@gskenned gskenned marked this pull request as draft July 18, 2024 15:51
@czogby-nasa
Copy link
Collaborator

/doc/example-scripts/stand-alone/README.md:

"Copy the example scripts to $BPLIB_HOME."
This isn't specific enough. It should say "Copy the example scripts in this directory to $BPLIB_HOME".

@czogby-nasa
Copy link
Collaborator

czogby-nasa commented Jul 18, 2024

/doc/example-scripts/stand-alone/README.md:

"Verify that BPLIB_SOURCE in bplib-env-vars is the path to a clone of the bplib repository."
Isn't this script supposed to be installing the bplib repository from scratch? If so, this line is impossible.

@czogby-nasa
Copy link
Collaborator

/doc/example-scripts/README.md:

"Verify that environment variables in $CFS_HOME/cfs-env-vars are set to the correct paths."
This should note that most of the paths shouldn't exist yet. As it is, it makes the reader think the paths should already exist and cFS should already be cloned.

@gskenned
Copy link
Contributor Author

The /doc/example-scripts/bplib-unit-test-functional sometimes encounters a CMake error:

CMake Error at CMakeLists.txt:61 (find_package):
  By not providing "FindNasaOsal.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "NasaOsal",
  but CMake did not find one.

@gskenned
Copy link
Contributor Author

Commit 49f6450 fixes:

Comment: #260 (comment)

The /doc/example-scripts/bplib-unit-test-functional sometimes encounters a CMake error:

Resolution
Appended a line to set the NasaOsal_DIR in /doc/example-scripts/setup-osal.
NasaOsal_DIR provides the path for bplib-unit-test-functional CMake to find the right file.

The error happened when bplib-unit-test-functional ran on a system that did not have OSAL installed. This was a good catch, because I didn't catch it in development.

Tests Performed

  1. Reproduce the error environment by "uninstalling" existing OSAL installations in the test environment. In my case, I had to remove the OSAL installation done by bp/docs/example-scripts/build-bp make DESTDIR=$CFS_HOME/cfs-build-folder -j2 mission-install.
  2. Run the steps in bplib/doc/example-tests/README to demonstrate the fix works.

@gskenned gskenned force-pushed the DTNN-132-clean-up-bplib-prototype-readme branch from 3d3ae3c to 0674914 Compare July 19, 2024 21:06
@gskenned
Copy link
Contributor Author

Commit (0674914) fixes:

Comments:
#260 (comment)
#260 (comment)
#260 (comment)

Resolution
Revised the bplib/doc/example-scripts/README.md and bplib/doc/example-scripts/stand-alone/README.md as indicated in the comments.

@@ -39,99 +39,120 @@ library.

#### Prerequisites

1. The build requires the __cmake__ build system and a compiler toolchain (by default __gcc__).
1. The build requires Ubuntu 22.04.4 LTS, the __cmake__ build system and a compiler toolchain (by default __gcc__).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Recommend something along the lines of "BPLib has been tested on Ubuntu 22.04.4 LTS, although it is expected to work on other POSIX-compliant operating systems."

@gskenned
Copy link
Contributor Author

Updated, reviewed, tested, and ready to go.

@gskenned gskenned marked this pull request as ready for review July 30, 2024 18:57
@dzbaker dzbaker added ccb:ready Pull request is ready for CCB discussion CCB:FastTrack labels Aug 1, 2024
@dzbaker dzbaker merged commit d373117 into nasa:main Aug 1, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:FastTrack ccb:ready Pull request is ready for CCB discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants