Skip to content

FpySequencer flag system #4259

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
Open
wants to merge 17 commits into
base: devel
Choose a base branch
from
Open

FpySequencer flag system #4259

wants to merge 17 commits into from
+426 −12

Conversation

zimri-leisher
Copy link
Collaborator

Related Issue(s) #4049
Has Unit Tests (y/n) y
Documentation Included (y/n) y
Generative AI was used in this contribution (y/n) n

Change Description

  • Adds a "flag" system to the sequencer.
    • Flags can be modified at runtime of a sequence, by the sequence itself, or by ground command
    • Flags get set to a default value at the beginning of running a sequence.
      • The default value is configurable via param
  • Adds SET_FLAG and GET_FLAG directives
    • These allow sequences to access and modify the value of flags during execution of a sequence
  • Add the EXIT_ON_CMD_FAIL flag
    • Default to false
    • If true, the sequence will immediately end if a command fails

@zimri-leisher
Copy link
Collaborator Author

@LeStarch this is ready for review

@zimri-leisher zimri-leisher requested a review from LeStarch October 3, 2025 04:27
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 3, 2025
View reviewed changes
Svc/FpySequencer/FpySequencer.cpp
this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR);
return;
}

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter opCode has not been checked.
Svc/FpySequencer/FpySequencer.cpp
this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::EXECUTION_ERROR);
return;
}

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter cmdSeq has not been checked.
Svc/FpySequencer/FpySequencer.cpp
this->m_runtime.flags[flag.e] = value;

this->cmdResponse_out(opCode, cmdSeq, Fw::CmdResponse::OK);
}

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter value has not been checked.
Svc/FpySequencer/FpySequencer.cpp
this->paramGet_FLAG_DEFAULT_EXIT_ON_CMD_FAIL(valid);
FW_ASSERT(valid != Fw::ParamValid::INVALID && valid != Fw::ParamValid::UNINIT,
static_cast<FwAssertArgType>(valid.e));
}

Check warning

Code scanning / CodeQL

Unchecked return value Warning

The return value of non-void function
paramGet_FLAG_DEFAULT_EXIT_ON_CMD_FAIL
Loading
is not checked.
Svc/FpySequencer/FpySequencerDirectives.cpp
this->push<U8>(flagVal);
return Signal::stmtResponse_success;
}
} // namespace Svc

Check warning

Code scanning / CodeQL

Unchecked return value Warning

The return value of non-void function
operator=
Loading
is not checked.
Svc/FpySequencer/FpySequencerRunState.cpp
break;
}
case Fpy::DirectiveId::SET_FLAG: {
new (&deserializedDirective.setFlag) FpySequencer_SetFlagDirective();

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter deserializedDirective has not been checked.
Svc/FpySequencer/FpySequencerRunState.cpp
break;
}
case Fpy::DirectiveId::GET_FLAG: {
new (&deserializedDirective.getFlag) FpySequencer_GetFlagDirective();

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter deserializedDirective has not been checked.
Svc/FpySequencer/FpySequencerRunState.cpp
return;
}
case Fpy::DirectiveId::SET_FLAG: {
this->directive_setFlag_internalInterfaceInvoke(directive.setFlag);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter directive has not been checked.
Svc/FpySequencer/FpySequencerRunState.cpp
return;
}
case Fpy::DirectiveId::GET_FLAG: {
this->directive_getFlag_internalInterfaceInvoke(directive.getFlag);

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter directive has not been checked.
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 3, 2025
View reviewed changes
Svc/FpySequencer/FpySequencer.cpp
void FpySequencer::SET_FLAG_cmdHandler(FwOpcodeType opCode, //!< The opcode
U32 cmdSeq, //!< The command sequence number
Svc::Fpy::FlagId flag,
bool value) {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Svc/FpySequencer/FpySequencerDirectives.cpp
if (directive.get_flagIdx() >= Fpy::FLAG_COUNT) {
error = DirectiveError::FLAG_IDX_OUT_OF_BOUNDS;
return Signal::stmtResponse_failure;
}

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Svc/FpySequencer/FpySequencerDirectives.cpp
if (directive.get_flagIdx() >= Fpy::FLAG_COUNT) {
error = DirectiveError::FLAG_IDX_OUT_OF_BOUNDS;
return Signal::stmtResponse_failure;
}

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
@LeStarch
Copy link
Collaborator

LeStarch commented Oct 8, 2025

@zimri-leisher merge of the last PR created conflicts here. Would you take a look? They aren't trivial.

LeStarch
LeStarch requested changes Oct 8, 2025
View reviewed changes
Copy link
Collaborator

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

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

Additionally some comments to answer.

Svc/FpySequencer/FpySequencer.cpp
// this is a sanity check, we shouldn't even get here if this isn't true
// because the enum should check for validity and raise a format err if not valid.
// actually what this really catches is an incorrect FLAG_COUNT value
FW_ASSERT(static_cast<I32>(flag.e) < Fpy::FLAG_COUNT, static_cast<FwAssertArgType>(flag.e));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why use an I32 here, I see 8-bits below.

Copy link
Collaborator Author

@zimri-leisher zimri-leisher Oct 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

the args to < will be converted to 32 bit types implicitly, this just makes it explicit.

@zimri-leisher
Copy link
Collaborator Author

@LeStarch feedback responded to

github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 10, 2025
View reviewed changes
Svc/FpySequencer/FpySequencerDirectives.cpp
//! Internal interface handler for directive_setFlag
void FpySequencer::directive_setFlag_internalInterfaceHandler(const Svc::FpySequencer_SetFlagDirective& directive) {
DirectiveError error = DirectiveError::NO_ERROR;
this->sendSignal(this->setFlag_directiveHandler(directive, error));

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter directive has not been checked.
Svc/FpySequencer/FpySequencerDirectives.cpp
//! Internal interface handler for directive_getFlag
void FpySequencer::directive_getFlag_internalInterfaceHandler(const Svc::FpySequencer_GetFlagDirective& directive) {
DirectiveError error = DirectiveError::NO_ERROR;
this->sendSignal(this->getFlag_directiveHandler(directive, error));

Check warning

Code scanning / CodeQL

Unchecked function argument Warning

This use of parameter directive has not been checked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LeStarch LeStarch LeStarch requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zimri-leisher @LeStarch