Skip to content

Issue #1067, Fixed Add QNX to cFS multi-target build - #1114

Open
dmolock-NASA wants to merge 1 commit into
nasa:devfrom
dmolock-NASA:1067-qnx-multi-target-build
Open

dmolock-NASA wants to merge 1 commit into
nasa:devfrom
dmolock-NASA:1067-qnx-multi-target-build

Conversation

@dmolock-NASA

Copy link
Copy Markdown

name: FSW Code Change
about: Flight Software code changes
labels: fsw

Description of Change

Linked Issue

Closes #

Requirements Impact

  • Requirement ID(s):
  • Requirements updated as necessary
  • Existing requirements are still satisfied by this change

Testing Evidence

Unit Tests (UT Assert)

COSMOS Test Suite

Areas of Expertise Touched

  • ASTRO
  • CI/CD
  • COSMOS
  • Cybersecurity
  • Docker
  • EDS
  • Git
  • PSPs
  • SBN
  • SMP
  • Tables
  • TSN
  • Unit Tests
  • Other

Author Checklist

  • Linked GitHub issue is referenced above
  • Code has been formatted with .clang-format
  • Static analysis workflows ran and passed
  • Unit tests (UT Assert) updated/added to cover code changes
  • Unit test workflows ran and passed
  • COSMOS test suite was run; tests updated/added if relevant changes were made
  • Requirements have been reviewed; updated or confirmed still satisfied (see above)
  • Testing evidence is included above
  • Self-review of the diff completed

Reviewer Checklist

  • Code logic is correct and matches the stated intent
  • Code is readable, maintainable, and follows project conventions (ask your lead if you are unsure of where to find these conventions)
  • .clang-format has been applied
  • Static analysis results reviewed and acceptable
  • The change has been exercised by the unit tests (not just that tests pass — the new/changed code paths are actually covered)
  • COSMOS test suite was executed against this change and results reviewed (or confirmed N/A with justification)
  • Reviewer has independently verified the change behaves as described (e.g., by running the tests locally, reviewing CI output in detail, or performing additional ad-hoc testing as warranted)
  • Memory safety reviewed (allocation, bounds, lifetime, stack usage)
  • Requirements impact reviewed and appropriate
  • Error handling is appropriate
  • Appropriate Expert areas have been reviewed

Reviewer Testing Notes

Comment thread target-configs.mk
##############
# QNX builds
##############
PREP_OPTS_aarch64le_qnx_gnu += -DENABLE_UNIT_TESTS=FALSE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

whats the reason unit test is set to false?

@jphickey jphickey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we simplify this?

Comment thread target-configs.mk
CONFIG_NAMES += qemu_yocto_linux
CONFIG_NAMES += osal edslib
CONFIG_NAMES += aarch64le_qnx_gnu aarch64le_qnx_gnu_eds
CONFIG_NAMES += x86_64_qnx_gnu x86_64_qnx_gnu_eds

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The idea with the multitarget build is to craft a small number of configs that (as a whole) try to span a good sampling of the build variations/options we offer. It is not (and will never be) a comprehensive set covering every option.

In that sense, I don't think we need 4 different QNX configs. Just 1 would suffice.

Also -- don't name it "x86_64" or "aarch64le" ... instead make it a specific board/target name. Does the development board being used for QNX testing have a name (like Raspberry Pi/rpi?). Preference is to use that board name because this config encompasses everything about running on that board, not just the processor type (this is just one detail out of many)

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.

4 participants