Skip to content

Issue #2777, Fix Add QUERY_TASK_AFFINITY command to TA CFE module - #2834

Open
dmolock-NASA wants to merge 1 commit into
nasa:devfrom
dmolock-NASA:2777-add-query_task_affinity-cmd-ta-mod
Open

dmolock-NASA wants to merge 1 commit into
nasa:devfrom
dmolock-NASA:2777-add-query_task_affinity-cmd-ta-mod

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

@dmolock-NASA
dmolock-NASA force-pushed the 2777-add-query_task_affinity-cmd-ta-mod branch from 03ae018 to df113f5 Compare September 4, 2026 19:16

@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.

If the intent is to have all task affinities reported in a single TLM, my suggestion would be to consider including the CFE ES TaskID (32 bits) rather than the full task name. This should reduce the size of the overall message considerably, and if the user wants to get task names then they can use the ES query command for that.

uint16 CoresConfigured; /**< \brief Number of physical cores configured */
uint16 TACoresMax; /**< \brief Maximum TA support bits*/
CFE_TA_AffinityTlm_Data_t TaskAffinity[OS_MAX_TASKS]; /**< \brief Tracked task array */
} CFE_TA_AffinityTlm_Payload_t;

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.

Did you check the size of this Payload structure?
We should try to avoid single TLM messages that are larger than ~1500 bytes (ethernet MTU). Large TLM have caused problems in the past requiring code mods to send them out as a series of smaller packets.

.QueryAllTasksCmd_indication = (int32_t (*)(const EdsDataType_CFE_TA_QueryAllTasksCmd_t *)) CFE_TA_QueryAllTasksCmd,
.SetTaskAffinityCmd_indication = (int32_t (*)(const EdsDataType_CFE_TA_SetTaskAffinityCmd_t *)) CFE_TA_SetTaskAffinityCmd,
.GetTaskAffinityCmd_indication = (int32_t (*)(const EdsDataType_CFE_TA_GetTaskAffinityCmd_t *)) CFE_TA_GetTaskAffinityCmd
.QueryTaskAffinityCmd_indication = (int32_t (*)(const EdsDataType_CFE_TA_QueryTaskAffinityCmd_t *)) CFE_TA_QueryTaskAffinityCmd

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.

These should not need casts (I know this isn't introduced by this PR, but can we clean it up?)

@sylvesterkaczmarek sylvesterkaczmarek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There is a comma missing after GetTaskAffinityCmd_indication in the EDS dispatch table. QueryTaskAffinityCmd_indication starts immediately on the next line, so this configuration will not compile. Just needs the separator before the new initializer.

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.

5 participants