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

Fix #2228, Update UTs to use correct cmd types #2229

Merged
merged 2 commits into from
Dec 20, 2022
Merged

Fix #2228, Update UTs to use correct cmd types #2229

merged 2 commits into from
Dec 20, 2022

Conversation

dmknutsen
Copy link
Contributor

@dmknutsen dmknutsen commented Dec 7, 2022 •
edited
Loading

Fix #2228, Update UTs to use correct cmd types

Checklist (Please check before submitting)

Describe the contribution
These are the unit test updates requested by @shion-ito-nasa, and are associated with #2228
The requested change originated from #2220

Testing performed
Build and run all tests'

Expected behavior changes
No impact to behavior

System(s) tested on
Ubuntu 22.04 and 20.04 (workflows)

Contributor Info - All information REQUIRED for consideration of pull request
Dan Knutsen
NASA/Goddard

Fix nasa#2228, Update UTs to use correct cmd types
4f4d492 
Fix nasa#2228, Update UTs to use correct cmd types
@dmknutsen dmknutsen self-assigned this Dec 7, 2022
@dmknutsen dmknutsen added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Dec 7, 2022
jphickey
jphickey requested changes Dec 8, 2022
View reviewed changes
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

If you want, I could theoretically take this one and do the updates I suggested. The problem is the change as proposed will break the EDS build. (Fundamental issue is that we don't actually define a real message called NoArgsCmd in any of these modules; that was just an implementation shortcut to avoid repeating the same definition, so it really shouldn't be referenced outside of the msg.h file).

modules/es/ut-coverage/es_UT.c Outdated Show resolved Hide resolved
modules/evs/ut-coverage/evs_UT.c Outdated Show resolved Hide resolved
modules/tbl/ut-coverage/tbl_UT.c Outdated Show resolved Hide resolved
@jphickey
Fix nasa#2228, Use correct message types
0f9313d 
Update remaining cases where the CFE_MSG_CommandHeader_t was used for a
command that had no arguments, replacing it with a real data type that
is named according to the command.

In particular, this introduces a "SendHkCmd" type into all modules,
where previously the HK command used CFE_MSG_CommandHeader_t directly.
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 9, 2022
View reviewed changes
modules/es/fsw/src/cfe_es_task.c
@@ -655,7 +655,7 @@
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
int32 CFE_ES_HousekeepingCmd(const CFE_MSG_CommandHeader_t *data)
int32 CFE_ES_HousekeepingCmd(const CFE_ES_SendHkCmd_t *data)

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/evs/fsw/src/cfe_evs_task.c
@@ -613,7 +613,7 @@
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
int32 CFE_EVS_ReportHousekeepingCmd(const CFE_MSG_CommandHeader_t *data)
int32 CFE_EVS_ReportHousekeepingCmd(const CFE_EVS_SendHkCmd_t *data)

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/sb/fsw/src/cfe_sb_task.c
@@ -503,7 +503,7 @@
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
int32 CFE_SB_SendHKTlmCmd(const CFE_MSG_CommandHeader_t *data)
int32 CFE_SB_SendHKTlmCmd(const CFE_SB_SendHkCmd_t *data)

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
modules/es/fsw/src/cfe_es_task.c
@@ -655,7 +655,7 @@
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
int32 CFE_ES_HousekeepingCmd(const CFE_MSG_CommandHeader_t *data)
int32 CFE_ES_HousekeepingCmd(const CFE_ES_SendHkCmd_t *data)

Check notice

Code scanning / CodeQL-coding-standard

Function too long

CFE_ES_HousekeepingCmd has too many lines (106, while 60 are allowed).
jphickey
jphickey approved these changes Dec 9, 2022
View reviewed changes
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

I'm good with the changes now aside from maybe squashing the commits (I kept it separate for now for review purposes)

@dzbaker dzbaker added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Dec 15, 2022
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 20, 2022
@dzbaker
IC: Caelum-rc4+dev33
01254b1 
*Combines:*

cFE v7.0.0-rc4+dev242
t0_lab v2.5.0-rc4+dev35

**Includes:**

*cFE*
- nasa/cFE#2231
- nasa/cFE#2229
- nasa/cFE#2192

*to_lab*
- nasa/to_lab#142

Co-authored by: Daniel Knutsen <dmknutsen@users.noreply.github.com>
Co-authored by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Avi Weiss: <thnkslprpt@users.noreply.github.com>
This was referenced Dec 20, 2022
Merged
Merged
@dzbaker dzbaker merged commit 9b047b8 into nasa:main Dec 20, 2022
dzbaker added a commit to nasa/cFS that referenced this pull request Dec 20, 2022
@dzbaker
IC: Caelum-rc4+dev33
08ec65b 
*Combines:*

cFE v7.0.0-rc4+dev242
to_lab v2.5.0-rc4+dev35

**Includes:**

*cFE*
- nasa/cFE#2231
- nasa/cFE#2229
- nasa/cFE#2192

*to_lab*
- nasa/to_lab#142

Co-authored by: Daniel Knutsen <dmknutsen@users.noreply.github.com>
Co-authored by: Justin Figueroa <chillfig@users.noreply.github.com>
Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored by: Avi Weiss: <thnkslprpt@users.noreply.github.com>
@dmknutsen dmknutsen added this to the Draco milestone Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shion-ito-nasa shion-ito-nasa shion-ito-nasa left review comments

@jphickey jphickey jphickey approved these changes

Assignees

@dmknutsen dmknutsen

Labels
CCB:Approved Indicates code review and approval by community CCB draco-rc4
Projects
None yet
Milestone
Draco
Development

Successfully merging this pull request may close these issues.

Update unit tests to use correct command types
4 participants
@dmknutsen @jphickey @shion-ito-nasa @dzbaker