Skip to content

Fix #508, Adds complete transaction housekeeping counters - #511

Open
chillfig wants to merge 1 commit into
nasa:devfrom
chillfig:fix-508-tx-complete-hk-counters
Open

Fix #508, Adds complete transaction housekeeping counters#511
chillfig wants to merge 1 commit into
nasa:devfrom
chillfig:fix-508-tx-complete-hk-counters

Conversation

@chillfig

@chillfig chillfig commented Aug 5, 2026

Copy link
Copy Markdown
Member

name: Adds complete transaction housekeeping counters
about: Flight Software code changes
labels: fsw

Description of Change

Adds a housekeeping counter for successfully sent and received files. This was included was a valuable feature for ground/operators. It was implemented simply by simply isolating the incrementation to when CF notes a CF_RxSubState_COMPLETE/CF_TxSubState_COMPLETE and guarding for when that transaction is error prone.

Linked Issue

Closes #508

Requirements Impact

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

Testing Evidence

unit test coverage

Unit Tests (UT Assert)

COSMOS Test Suite

Will require changes in the CF housekeeping structure. Pending feedback on this pr.

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
  • .clang-format has been applied
  • Static analysis results reviewed and acceptable
  • Unit tests are meaningful and adequately cover the changes
  • 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

@chillfig chillfig self-assigned this Aug 5, 2026
uint32 pdu; /**< \brief Sent PDUs counter */
uint32 nak_segment_requests; /**< \brief Sent NAK segment requests counter */
uint32 files_sent; /**< \brief Files successfully sent counter */
uint8 spare[4]; /**< \brief Alignment spare to avoid implicit padding */

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

explicit trailing padding because of 8-byte alignment enforced by the compiler/strictest member being uint64

uint16 dropped; /**< \brief Received PDUs dropped due to a transaction error */
uint32 nak_segment_requests; /**< \brief Received NAK segment requests counter */
uint32 files_recv; /**< \brief Files successfully received counter */
uint8 spare[4]; /**< \brief Alignment spare to avoid implicit padding */

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

same as above

Comment thread fsw/src/cf_cfdp_r.c
break;
case CF_RxSubState_COMPLETE:
/* only a transaction that reached this point without error received a complete file */
if (CF_CFDP_TxnIsOK(txn))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We only care about successful completions so guard against a CRC mismatch, file-size error, or inactivity timeout which all land on this line (CF_RxSubState_COMPLETE)

Comment thread fsw/src/cf_cfdp_s.c
break;
case CF_TxSubState_COMPLETE:
/* only a transaction that reached this point without error sent a complete file */
if (CF_CFDP_TxnIsOK(txn))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We only care about successful completions so guard against read failures, filestore rejections, and NAK/ACK-limit failures which all land on this line (CF_TxSubState_COMPLETE)

Comment thread unit-test/cf_cmd_tests.c

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this was a bug. it was only ever measuring the first 8 bytes (size of a pointer assuming 64 bit machine). Dropping the & makes it measure the struct itself and actually verify the whole thing

@jphickey
jphickey self-requested a review August 6, 2026 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Housekeeping does not report successfully completed file transmissions

1 participant