Skip to content

Add pktType into FrameContext and set it in the primary header - #5996

Open
ynielson-firefly wants to merge 4 commits into
nasa:develfrom
FireflySpace:spacePacketFramer_packetType
Open

ynielson-firefly wants to merge 4 commits into
nasa:develfrom
FireflySpace:spacePacketFramer_packetType

Conversation

@ynielson-firefly

Copy link
Copy Markdown

Added pktType (command/telemetry) into FrameContext and set it in the space packet primary header.

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

Change Description

Rationale

Space packet primary header has 1 bit packet type defined. But the Svc/Ccsds/SpacePacketFramer set this packet type to 0 for all the packet. (It is defined that the command packet has packet type 1 and the telemetry or data packet has packet type 0)

Testing/Review Recommendations

Future Work

AI Usage (see policy)

@ynielson-firefly

Copy link
Copy Markdown
Author

Submitted a PR in FireflySpace/fprime.

@thomas-bc
thomas-bc requested a balanced review from Copilot and removed request for Copilot September 18, 2026 17:54
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Coverage report — base devel

Overall (line): 85.11% → 85.12% (+0.01)
Regression threshold: 0.50% (line).

Regressions

Module Line Δ Function Δ Branch Δ
Svc/Ccsds/SpacePacketFramer 95.92 -2.00 100.00 +0.00 86.96 -3.95

Modules changed

Module Line Δ Function Δ Branch Δ
Svc/Ccsds/SpacePacketFramer 95.92 -2.00 100.00 +0.00 86.96 -3.95
Fw/DataStructures 97.87 -0.35 97.14 +0.00 82.48 -0.55
Svc/Ccsds/SpacePacketDeframer 91.53 +0.46 100.00 +0.00 80.00 +0.34
Os/Posix 71.15 +0.51 87.50 +0.00 51.34 +0.67

New modules

Module Line Function Branch
Fw/Prm 0.00 0.00 0.00

Modules without UTs

CFDP/Checksum/GTest, Drv/LinuxGpioDriver, Drv/LinuxI2cDriver, Drv/LinuxSpiDriver, Drv/Ports/DataTypes, Drv/PosixUartDriver, FppTestProject/FppTest/topology/async, FppTestProject/FppTest/topology/components/Comp, FppTestProject/FppTest/topology/components/Framework, FppTestProject/FppTest/topology/components/Receiver, FppTestProject/FppTest/topology/components/Sender, FppTestProject/FppTest/topology/guarded, FppTestProject/FppTest/topology/sync, FppTestProject/FppTest/topology/top_ports, FppTestProject/FppTest/topology/types, Fw/Com, Fw/Comp, Fw/FilePacket/GTest, Fw/Fpy, Fw/Obj, Fw/Port, Fw/Sm, Fw/Test, Fw/Types/GTest, Os/Models, Svc/Ccsds/Types, Svc/Ccsds/Utils, Svc/FatalHandler, Svc/Subtopologies/CdhCore, Svc/Subtopologies/ComCcsds, Svc/Subtopologies/ComCcsdsSdls, Svc/Subtopologies/ComFprime, Svc/Subtopologies/ComLoggerTee, Svc/Subtopologies/DataProducts, Svc/Subtopologies/DpCompression, Svc/Subtopologies/FileHandling, Svc/Subtopologies/FileHandlingCfdp, Svc/Subtopologies/FileHandlingCfdp/FileHandlingCfdpConfig, TestDeploymentsProject/Ref/DpDemo, TestDeploymentsProject/Ref/PingReceiver, TestDeploymentsProject/Ref/RecvBuffApp, TestDeploymentsProject/Ref/SendBuffApp, TestDeploymentsProject/Ref/Top, TestDeploymentsProject/Ref/TypeDemo, cmake/test/data/TestConfigDeployment, cmake/test/data/TestDeployment/TestBuildAutocoder, cmake/test/data/TestDeployment/TestDuplicateDirective, cmake/test/data/TestDeployment/TestHeaderAutocoder, cmake/test/data/TestDeployment/TestLinkDepends, cmake/test/data/TestDeployment/TestRelative, cmake/test/data/test-fprime-library/TestLibrary/TestComponent, cmake/test/data/test-fprime-library2/TestLibrary2/TestComponent

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

FrameContext now carries pktType, but SpacePacketDeframer never extracts that bit into contextCopy. A command packet that is deframed and later reframed therefore falls back to the default telemetry type. The deframer should set it from PktTypeMask too.

@ynielson-firefly

Copy link
Copy Markdown
Author

FrameContext now carries pktType, but SpacePacketDeframer never extracts that bit into contextCopy. A command packet that is deframed and later reframed therefore falls back to the default telemetry type. The deframer should set it from PktTypeMask too.

Packet type is extracted in SpacePacketDeframer and set it in the FrameContext.

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

Rechecked current 023451b. The deframer now extracts PktTypeMask into FrameContext before forwarding the packet, and the existing control-field test now asserts pktType alongside the other recovered primary-header fields. That resolves the command-packet round-trip issue I raised.

@thomas-bc

Copy link
Copy Markdown
Collaborator

We're not able to update your PR cause your fork has not allowed maintainers of nasa/fprime to write to it - but you'll need to pull in latest devel branch for a CI quirk that has been fixed in devel

@ynielson-firefly

ynielson-firefly commented Sep 22, 2026

Copy link
Copy Markdown
Author

We're not able to update your PR cause your fork has not allowed maintainers of nasa/fprime to write to it - but you'll need to pull in latest devel branch for a CI quirk that has been fixed in devel

I merged the latest devel branch into this branch. (building/unit test okay)

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

Rechecked current 83b70fa after merging latest devel. The packet-type fix remains intact: SpacePacketDeframer recovers PktTypeMask into FrameContext and the framer uses that context value when rebuilding the primary header, so the command/telemetry round trip I reviewed remains covered. The current Zephyr failure occurs while downloading the SDK toolchain before the project build starts, so it is unrelated to this change. No remaining blocker from my review.

@thomas-bc

Copy link
Copy Markdown
Collaborator

Can I ask for my own curiosity what the use case is here? Are you ever expecting to receive Space Packets that aren't uplink==telecommand and downlink==telemetry ? Maybe cross-satellite comms ? And what's even the point of checking that value, do APIDs mean something different based on that ?

@ynielson-firefly

Copy link
Copy Markdown
Author

Can I ask for my own curiosity what the use case is here? Are you ever expecting to receive Space Packets that aren't uplink==telecommand and downlink==telemetry ? Maybe cross-satellite comms ? And what's even the point of checking that value, do APIDs mean something different based on that ?

The firefly aerospace uses this packet type to send command (downlink) in the space packet (primary header). And this packet type is defined in the primary header (command/telemetry or data)

This branch has not been deployed

No deployments
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.

4 participants