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.

Sign up for GitHub

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

Jump to bottom

Os task refactor issue 2526 #2672

Merged
merged 29 commits into from
May 22, 2024
Merged

Os task refactor issue 2526 #2672

merged 29 commits into from
May 22, 2024

Conversation

LeStarch
Copy link
Collaborator

Related Issue(s)
Has Unit Tests (y/n)
Documentation Included (y/n)

Change Description

Starting a PR for Os::Task refactor.

@LeStarch LeStarch added the Update Instructions Needed Need to add instructions in the release notes for updates. label Apr 11, 2024
@LeStarch LeStarch marked this pull request as draft April 11, 2024 00:03
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 11, 2024
View reviewed changes
Os/Task.cpp Fixed Show fixed Hide fixed
Os/Task.hpp Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 11, 2024
View reviewed changes
Os/Posix/test/ut/PosixTaskTests.cpp Fixed Show fixed Hide fixed
Os/test/ut/task/TaskRules.hpp Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 15, 2024
View reviewed changes
Os/Stub/test/Task.cpp Fixed Show fixed Hide fixed
Os/Stub/test/Task.hpp Fixed Show fixed Hide fixed
@LeStarch LeStarch force-pushed the os-task-refactor-issue-2526 branch 2 times, most recently from 3a70670 to 66a58f5 Compare April 26, 2024 16:11
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 26, 2024
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@LeStarch LeStarch force-pushed the os-task-refactor-issue-2526 branch from d974342 to 71569b1 Compare April 30, 2024 23:04
@LeStarch LeStarch closed this May 6, 2024
@LeStarch LeStarch reopened this May 6, 2024
@LeStarch LeStarch mentioned this pull request May 6, 2024
Closed
@bocchino bocchino mentioned this pull request May 7, 2024
Merged
@LeStarch LeStarch requested a review from bocchino May 7, 2024 16:34
@LeStarch LeStarch marked this pull request as ready for review May 7, 2024 16:35
bocchino
bocchino reviewed May 15, 2024
View reviewed changes
config/IpCfg.hpp Outdated Show resolved Hide resolved
bocchino
bocchino reviewed May 15, 2024
View reviewed changes
Os/Posix/Task.hpp Outdated Show resolved Hide resolved
@LeStarch LeStarch force-pushed the os-task-refactor-issue-2526 branch from 4c29fdb to ea40b98 Compare May 16, 2024 22:01
@LeStarch
Copy link
Collaborator Author

@bocchino ready for review!

@LeStarch LeStarch mentioned this pull request May 16, 2024
Open
@LeStarch
Copy link
Collaborator Author

@bocchino ready for re-review!

@LeStarch LeStarch force-pushed the os-task-refactor-issue-2526 branch from 2ff0dc0 to dbf6197 Compare May 16, 2024 22:22
@LeStarch LeStarch force-pushed the os-task-refactor-issue-2526 branch from dbf6197 to 63e02d5 Compare May 17, 2024 01:21
thomas-bc
thomas-bc reviewed May 20, 2024
View reviewed changes
Copy link
Collaborator

@thomas-bc thomas-bc left a comment

Choose a reason for hiding this comment

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

Comments from review session

Os/Posix/Task.cpp Outdated Show resolved Hide resolved
Os/Stub/test/Task.cpp Show resolved Hide resolved
Fw/Comp/ActiveComponentBase.cpp Show resolved Hide resolved
@LeStarch
Copy link
Collaborator Author

LeStarch commented May 22, 2024 •
edited
Loading

@timcanham's comments:

  • Should invokeRoutine be private, add comments
  • Is m_identifier used?
  • Delete TaskCommon.cpp
  • Mutex for getNumTasks()
  • Improper padding in Posix/Task.cpp, line 28
  • Delete baremetal directories
  • Discuss interval time concept. Fw::Time should be absolute time.

@LeStarch LeStarch force-pushed the os-task-refactor-issue-2526 branch from 8f32105 to 4e6645a Compare May 22, 2024 01:03
bocchino
bocchino approved these changes May 22, 2024
View reviewed changes
Copy link
Collaborator

@bocchino bocchino left a comment

Choose a reason for hiding this comment

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

Looks good!

@LeStarch LeStarch merged commit 640cf44 into nasa:devel May 22, 2024
35 checks passed
This was referenced May 22, 2024
Merged
Merged
Merged
@thomas-bc thomas-bc mentioned this pull request Jun 4, 2024
Closed
@Lex-ari Lex-ari mentioned this pull request Jul 30, 2024
Merged
@nateinaction nateinaction mentioned this pull request Oct 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bocchino bocchino bocchino approved these changes

@thomas-bc thomas-bc thomas-bc approved these changes

@timcanham timcanham Awaiting requested review from timcanham

Assignees
No one assigned
Labels
Update Instructions Needed Need to add instructions in the release notes for updates.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@LeStarch @bocchino @thomas-bc @kevin-f-ortega