FpySequencer V0.3 by zimri-leisher · Pull Request #3702 · nasa/fprime

zimri-leisher · 2025-06-06T02:06:05Z


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

Change Description

This PR adds registers, and several new directives for interacting with the registers.
- Each register is an 8 byte array, meant to store an integer or floating point value.
- Binary comparison directives
  - OR/AND/NOT do bitwise ops on registers
  - A full set of inequality/equality operators allow you to compare numeric values in registers
- EXIT directive
  - Allows for early completion of sequence, with either failure or success status
- DESER_SER_REG directives
  - Allows pulling a value from an sreg into a reg. Used to grab specific parts of tlm chan structs
- SET_REG directive
  - sets a register to a constant value
This PR also combines commands and directives. Now, instead of those being separate things, commands are just instances of the "CMD" directive. This saves space in the file and simplifies the code a bit.
"local variable" is renamed to "serializable register"
TODO rename "register" to something more specific
IF directive branches off of a register instead of sreg
Add DirectiveError codes, which let you diagnose the specific reason why a sequence failed at a bytecode level. Useful for debugging the sequencer

…-v03

Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp

Svc/FpySequencer/FpySequencer.cpp

github-advanced-security

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

…516-fpy-v03

…-v03

LeStarch · 2025-06-26T16:22:44Z

Svc/FpySequencer/FpySequencerRunState.cpp

+        case Fpy::DirectiveId::CMD: {
+            new (&deserializedDirective.cmd) FpySequencer_CmdDirective();
+            // same deserialization behavior as SET_LVAR
+
+            // first deserialize the opcode
+            FwOpcodeType opcode;
+            status = argBuf.deserialize(opcode);
+            if (status != Fw::SerializeStatus::FW_SERIALIZE_OK) {
+                this->log_WARNING_HI_DirectiveDeserializeError(stmt.getopCode(), this->m_runtime.nextStatementIndex - 1,
+                                                               status, argBuf.getBuffLeft(), argBuf.getBuffLength());
+                return Fw::Success::FAILURE;
+            }
+
+            deserializedDirective.cmd.setopCode(opcode);
+            //  how many bytes are left?
+            FwSizeType cmdArgBufSize = argBuf.getBuffLeft();
+
+            // check to make sure the value will fit in the FpySequencer_CmdDirective::argBuf
+            if (cmdArgBufSize > Fpy::MAX_LOCAL_VARIABLE_BUFFER_SIZE) {
+                this->log_WARNING_HI_DirectiveDeserializeError(stmt.getopCode(), this->m_runtime.nextStatementIndex - 1,
+                                                               Fw::SerializeStatus::FW_DESERIALIZE_FORMAT_ERROR,
+                                                               argBuf.getBuffLeft(), argBuf.getBuffLength());
+                return Fw::Success::FAILURE;
+            }
+
+            // okay, it will fit. put it in
+            status = argBuf.deserialize(deserializedDirective.cmd.getargBuf(), cmdArgBufSize, true);
+
+            if (status != Fw::SerializeStatus::FW_SERIALIZE_OK) {
+                this->log_WARNING_HI_DirectiveDeserializeError(stmt.getopCode(), this->m_runtime.nextStatementIndex - 1,
+                                                               status, argBuf.getBuffLeft(), argBuf.getBuffLength());
+                return Fw::Success::FAILURE;
+            }
+
+            // now there should be nothing left, otherwise coding err
+            FW_ASSERT(argBuf.getBuffLeft() == 0, static_cast<FwAssertArgType>(argBuf.getBuffLeft()));
+
+            // and set the buf size now that we know it
+            deserializedDirective.cmd.set_argBufSize(cmdArgBufSize);
+            break;
+        }
+        // fallthrough on purpose
+        case Fpy::DirectiveId::DESER_LVAR_8:
+        case Fpy::DirectiveId::DESER_LVAR_4:
+        case Fpy::DirectiveId::DESER_LVAR_2:
+        case Fpy::DirectiveId::DESER_LVAR_1: {
+            new (&deserializedDirective.deserLocalVar) FpySequencer_DeserLocalVarDirective();
+
+            U8 deserSize;
+
+            if (stmt.getopCode() == Fpy::DirectiveId::DESER_LVAR_1) {
+                deserSize = 1;
+            } else if (stmt.getopCode() == Fpy::DirectiveId::DESER_LVAR_2) {
+                deserSize = 2;
+            } else if (stmt.getopCode() == Fpy::DirectiveId::DESER_LVAR_4) {
+                deserSize = 4;
+            } else if (stmt.getopCode() == Fpy::DirectiveId::DESER_LVAR_8) {
+                deserSize = 8;
+            } else {
+                FW_ASSERT(0, static_cast<FwAssertArgType>(stmt.getopCode()));
+                return Fw::Success::FAILURE;
+            }
+
+            deserializedDirective.deserLocalVar.set_deserSize(deserSize);
+
+            U8 srcLvarIdx;
+            status = argBuf.deserialize(srcLvarIdx);
+            if (status != Fw::SerializeStatus::FW_SERIALIZE_OK) {
+                this->log_WARNING_HI_DirectiveDeserializeError(stmt.getopCode(), this->m_runtime.nextStatementIndex - 1,
+                                                               status, argBuf.getBuffLeft(), argBuf.getBuffLength());
+                return Fw::Success::FAILURE;
+            }
+            deserializedDirective.deserLocalVar.setsrcLvarIdx(srcLvarIdx);
+
+            FwSizeType srcOffset;
+            status = argBuf.deserialize(srcOffset);
+            if (status != Fw::SerializeStatus::FW_SERIALIZE_OK) {
+                this->log_WARNING_HI_DirectiveDeserializeError(stmt.getopCode(), this->m_runtime.nextStatementIndex - 1,
+                                                               status, argBuf.getBuffLeft(), argBuf.getBuffLength());
+                return Fw::Success::FAILURE;
+            }
+            deserializedDirective.deserLocalVar.setsrcOffset(srcOffset);
+
+            U8 destReg;
+            status = argBuf.deserialize(destReg);
+            if (status != Fw::SerializeStatus::FW_SERIALIZE_OK || argBuf.getBuffLeft() != 0) {
+                this->log_WARNING_HI_DirectiveDeserializeError(stmt.getopCode(), this->m_runtime.nextStatementIndex - 1,
+                                                               status, argBuf.getBuffLeft(), argBuf.getBuffLength());
+                return Fw::Success::FAILURE;
+            }
+            deserializedDirective.deserLocalVar.setdestReg(destReg);
+            break;
+        }
+        case Fpy::DirectiveId::SET_REG: {
+            new (&deserializedDirective.setReg) FpySequencer_SetRegDirective();
+            status = argBuf.deserialize(deserializedDirective.setReg);
+            if (status != Fw::SerializeStatus::FW_SERIALIZE_OK || argBuf.getBuffLeft() != 0) {
+                this->log_WARNING_HI_DirectiveDeserializeError(stmt.getopCode(), this->m_runtime.nextStatementIndex - 1,
+                                                               status, argBuf.getBuffLeft(), argBuf.getBuffLength());
+                return Fw::Success::FAILURE;
+            }
+            break;
+        }
+        // fallthrough on purpose
+        case Fpy::DirectiveId::OR:
+        case Fpy::DirectiveId::AND:
+        case Fpy::DirectiveId::EQ:
+        case Fpy::DirectiveId::NE:
+        case Fpy::DirectiveId::UGT:
+        case Fpy::DirectiveId::ULT:
+        case Fpy::DirectiveId::ULE:
+        case Fpy::DirectiveId::UGE: 


This is legitimate. We should track this as a fix for the future.

Svc/FpySequencer/FpySequencerDirectives.cpp

+        || directive.getres() >= Fpy::NUM_REGISTERS) {
+        error = DirectiveError::REGISTER_OUT_OF_BOUNDS;
+        return Signal::stmtResponse_failure;
+    }


Svc/FpySequencer/FpySequencerDirectives.cpp

+        || directive.getres() >= Fpy::NUM_REGISTERS) {
+        error = DirectiveError::REGISTER_OUT_OF_BOUNDS;
+        return Signal::stmtResponse_failure;
+    }


Svc/FpySequencer/FpySequencerDirectives.cpp

+Signal FpySequencer::not_directiveHandler(const FpySequencer_NotDirective& directive, DirectiveError& error) {
+    if (directive.getsrc() >= Fpy::NUM_REGISTERS 
+        || directive.getres() >= Fpy::NUM_REGISTERS) {
+        error = DirectiveError::REGISTER_OUT_OF_BOUNDS;


Svc/FpySequencer/FpySequencer.hpp

Svc/FpySequencer/FpySequencerDirectives.cpp

-Signal FpySequencer::setLocalVar_directiveHandler(const FpySequencer_SetLocalVarDirective& directive) {
-    if (directive.getindex() >= Fpy::MAX_SEQUENCE_LOCAL_VARIABLES) {
+//! Internal interface handler for directive_setSerReg
+Signal FpySequencer::setSerReg_directiveHandler(const FpySequencer_SetSerRegDirective& directive, DirectiveError& error) {


Svc/FpySequencer/FpySequencerDirectives.cpp

-    lvar.valueSize = prmValue.getBuffLength();
+    // copy value into serReg
+    Runtime::SerializableReg& serReg = this->m_runtime.serRegs[directive.getdestSerRegIndex()];
+    memcpy(serReg.value, prmValue.getBuffAddr(), static_cast<size_t>(prmValue.getBuffLength()));


Svc/FpySequencer/FpySequencerDirectives.cpp

+
+Signal FpySequencer::deserSerReg_directiveHandler(const FpySequencer_DeserSerRegDirective& directive, DirectiveError& error) {
+    if (directive.getsrcSerRegIdx() >= Fpy::NUM_SERIALIZABLE_REGISTERS) {
+        error = DirectiveError::SER_REG_OUT_OF_BOUNDS;


Svc/FpySequencer/FpySequencer.cpp

zimri-leisher · 2025-06-21T01:00:22Z

Svc/FpySequencer/FpySequencer.hpp

+        // values of unspecified type
+        I64 registers[Fpy::NUM_REGISTERS] = {0};
+
+        // local var -> local array, scratch array


todo delete

zimri-leisher · 2025-06-21T01:01:33Z

Svc/FpySequencer/FpySequencerDirectives.cpp

-    }
-
-    if (conditional) {
+    if (reg(directive.getconditionalReg())) {


if dir uses registers now

zimri-leisher · 2025-06-21T01:02:43Z

Svc/FpySequencer/FpySequencerDirectives.cpp

+        return Signal::stmtResponse_failure;
+    }
+
+    // TODO can I use htons/htonl? this code could be way simpler


i'd like to do this. are we allowed to use htons/htonl? or is that a linux/posix thing and we don't want to restrict?

zimri-leisher · 2025-06-21T01:09:17Z

Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp

-    ::testing::InitGoogleTest(&argc, argv);
-    return RUN_ALL_TESTS();
-}
+// int main(int argc, char** argv) {


this function wasn't necessary, you can remove it from all fprime components i believe

zimri-leisher · 2025-06-23T04:07:44Z

Todo remove all references to the word "statement", replace with "directive"

zimri-leisher · 2025-06-23T04:17:33Z

@LeStarch this is ready for review

Svc/FpySequencer/FpySequencer.cpp

LeStarch · 2025-06-25T21:24:35Z

@zimri-leisher is this ready for review?

zimri-leisher · 2025-06-25T21:37:31Z

@LeStarch this is ready for review

yep!

.github/actions/spelling/expect.txt

LeStarch · 2025-06-26T16:51:16Z

Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp

 }

 }  // namespace Svc
-


I am pretty sure this is necessary....no?

no don't believe so, gtest adds it automatically i think

LeStarch · 2025-06-26T17:01:29Z

Svc/FpySequencer/FpySequencerDirectives.fppi

+struct CmdDirective {
+    opCode: FwOpcodeType
+    @ the arg buf of the cmd
+    # TODO please don't let me merge this. need to find a better const here


This statement is specifically telling me to block the merge.

ah yes because I wanted to discuss... how big should the size of the cmd arg buf be in Fpy? I can't reference the FPrime CMD_ARG_BUF_MAX_SIZE because it's a #define constant. For now I'm using the size of the serializable registers

zimri-leisher and others added 11 commits June 2, 2025 19:06

Start combining dirs and cmd

f6d746f

Much closer to combining dir and cmd

ff96aa6

Finish making cmds work

5730dc2

Add directive error codes, fix UT

753a4f4

Add deser lvar directive

a89cf9b

Add comparison operators

4ac5450

Add unsigned/signed inequality ops

23f5160

Rename store to setreg

5a243c7

First UT with a binary comparison op!

cb72f50

Remove printfs

c1961ec

Merge branch 'devel' of github.com:zimri-leisher/fprime into 3516-fpy…

7a9e864

…-v03

github-advanced-security bot found potential problems Jun 6, 2025

View reviewed changes

Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp Fixed Show fixed Hide fixed

zimri-leisher commented Jun 6, 2025

View reviewed changes

Svc/FpySequencer/FpySequencer.cpp Show resolved Hide resolved

github-advanced-security bot found potential problems Jun 6, 2025

View reviewed changes

zimri-leisher and others added 8 commits June 10, 2025 23:30

Doc commnt

f4900b7

Add and/not, and/or under binary cmp

5cc89bf

Merge branch '3516-fpy-v03' of github.com:zimri-leisher/fprime into 3…

a625606

…516-fpy-v03

Fix a couple bugs in sequencer

04c28b8

Add exit directive'

442d3c7

Make exit actually end sequence if positive

306d8c8

Merge branch 'devel' of github.com:zimri-leisher/fprime into 3516-fpy…

abb615c

…-v03

A little more work on UTS

245528c

github-advanced-security bot found potential problems Jun 20, 2025

View reviewed changes

zimri-leisher added 5 commits June 20, 2025 19:58

Fix UTs

4a8c6e8

Add deser lvar UT

8163941

Add exit, not, set reg, and binary cmp UTs

674a1cb

Rename lvar to sreg

3d3dd04

Get the last few old refs to lvar

28780fa

github-advanced-security bot found potential problems Jun 21, 2025

View reviewed changes

zimri-leisher requested a review from LeStarch June 23, 2025 04:06

zimri-leisher marked this pull request as ready for review June 23, 2025 04:06

zimri-leisher commented Jun 23, 2025

View reviewed changes

zimri-leisher commented Jun 24, 2025

View reviewed changes

Svc/FpySequencer/FpySequencer.cpp Show resolved Hide resolved

zimri-leisher self-assigned this Jun 24, 2025

sp

74ec08c

zimri-leisher commented Jun 26, 2025

View reviewed changes

.github/actions/spelling/expect.txt Outdated Show resolved Hide resolved

Zimri Leisher added 2 commits June 26, 2025 11:47

Fix typo

79fe92d

Fix typo

0df4566

LeStarch requested changes Jun 26, 2025

View reviewed changes

LeStarch approved these changes Jun 26, 2025

View reviewed changes

LeStarch merged commit 50d3d4e into nasa:devel Jun 26, 2025
53 of 54 checks passed

Conversation

zimri-leisher commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Description

Uh oh!

Uh oh!

Uh oh!

github-advanced-security bot left a comment

Choose a reason for hiding this comment

Uh oh!

Check notice

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Check notice

Check notice

Check notice

Uh oh!

Check notice

Check notice

Check notice

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zimri-leisher commented Jun 23, 2025

Uh oh!

zimri-leisher commented Jun 23, 2025

Uh oh!

Uh oh!

LeStarch commented Jun 25, 2025

Uh oh!

zimri-leisher commented Jun 25, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zimri-leisher commented Jun 6, 2025 •

edited

Loading