Skip to content

Assert on channel ID present in both packet and ignore lists in TlmPacketizer#163

Open
devin-ai-integration[bot] wants to merge 2 commits into
develfrom
devin/1781057674-tlmpacketizer-ignore-list-assert
Open

Assert on channel ID present in both packet and ignore lists in TlmPacketizer#163
devin-ai-integration[bot] wants to merge 2 commits into
develfrom
devin/1781057674-tlmpacketizer-ignore-list-assert

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jun 10, 2026

Copy link
Copy Markdown
Related Issue(s) nasa#5133
Has Unit Tests (y/n) n
Documentation Included (y/n) n
Generative AI was used in this contribution (y/n) AI

Change Description

Adds an FW_ASSERT in TlmPacketizer::setPacketList() when processing the ignore list. If a channel ID already exists in m_channelIndices, the assert verifies the existing entry has ignored == true (i.e., it's a duplicate ignore-list entry). If the channel was registered from the packet list (ignored == false), the assert fires at startup, catching the misconfiguration.

 } else {
+    FW_ASSERT(this->m_channels[entryIndex].ignored, static_cast<FwAssertArgType>(id));
 }

Rationale

Without this check, a channel ID appearing in both the packet list and the ignore list silently corrupts telemetry: setPacketList() first registers the channel with a valid packetOffset and ignored = false, then the ignore-list loop overwrites ignored = true. At runtime, TlmRecv_handler sees ignored == true and returns early, leaving the packet buffer slot zeroed. The zeroed slot is then downlinked as valid telemetry — undetectable data corruption.

Testing/Review Recommendations

Existing unit tests pass since the test data has no overlap between packet and ignore channel IDs. The new assert only triggers on the previously-silent misconfiguration case.

Future Work

None.

AI Usage (see policy)

AI was used for code investigation, fix implementation, and PR authoring.

IAMAI

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/d018a7d2e4e54d408baf703a0bd786db


Open in Devin Review

Add an FW_ASSERT in setPacketList() to detect when a channel ID appears in
both the TlmPacketizerPacketList and the ignore list. Previously, this
misconfiguration silently overwrote the channel's ignored flag to true,
causing its packet buffer slot to remain zeroed and be downlinked as
corrupted telemetry data.

Closes nasa#5133

Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>
@devin-ai-integration

Copy link
Copy Markdown
Author
Original prompt from michael.d.starch

Is this actually an issue? nasa#5133. If so, please fix it.

@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

devin-ai-integration[bot]

This comment was marked as resolved.

@github-actions

Copy link
Copy Markdown

Coverage report — base devel

No baseline branch coverage/devel found. This run becomes the seed once it lands on devel.

Overall (line): 81.23% (no baseline)
Regression threshold: 0.50% (line).

Regressions

(none over threshold)

Modules changed

(no measurable change)

New modules

Module Line Function Branch
CFDP/Checksum 71.15 57.14 53.85
Drv/AsyncByteStreamBufferAdapter 100.00 100.00 100.00
Drv/ByteStreamBufferAdapter 100.00 100.00 100.00
Drv/Ip 44.32 57.38 25.36
Drv/TcpClient 75.00 100.00 47.37
Drv/TcpServer 84.72 100.00 60.00
Drv/Udp 68.09 90.91 42.86
Fw/Buffer 81.25 89.47 58.62
Fw/DataStructures 98.48 96.95 83.21
Fw/Dp 94.83 96.67 95.95
Fw/FilePacket 75.24 89.06 54.30
Fw/Log 71.43 72.41 60.00
Fw/Logger 100.00 100.00 100.00
Fw/SerializableFile 90.00 100.00 79.41
Fw/Time 88.62 85.48 86.84
Fw/Tlm 53.76 60.00 37.50
Fw/Types 54.75 57.62 34.96
Os 18.07 19.52 14.52
Os/Generic 89.10 96.30 67.18
Os/Generic/Types 92.45 91.67 82.14
Os/Posix 62.40 84.21 44.10
Svc/ActiveRateGroup 100.00 100.00 92.31
Svc/ActiveTextLogger 79.05 90.00 71.23
Svc/AssertFatalAdapter 94.74 100.00 86.67
Svc/BufferAccumulator 88.00 94.12 74.49
Svc/BufferLogger 92.62 86.96 80.46
Svc/BufferManager 99.05 100.00 83.64
Svc/BufferRepeater 91.67 100.00 75.00
Svc/ChronoTime 100.00 100.00 100.00
Svc/CmdDispatcher 96.97 91.67 91.75
Svc/CmdSequencer 93.89 97.12 84.63
Svc/CmdSplitter 100.00 100.00 100.00
Svc/ComLogger 97.37 91.67 83.91
Svc/ComSplitter 100.00 100.00 100.00
Svc/ComStub 98.51 100.00 85.19
Svc/DpCatalog 78.00 100.00 66.27
Svc/DpManager 97.44 100.00 100.00
Svc/DpWriter 97.58 90.00 97.22
Svc/FileDownlink 84.15 90.91 71.90
Svc/FileManager 88.41 93.33 82.44
Svc/FileUplink 92.35 96.55 80.95
Svc/FileWorker 90.29 100.00 84.87
Svc/FprimeDeframer 100.00 100.00 97.92
Svc/FprimeFramer 100.00 100.00 95.45
Svc/FprimeRouter 88.89 100.00 78.26
Svc/FpySequencer 86.76 99.04 76.89
Svc/GenericHub 100.00 100.00 85.64
Svc/Health 100.00 100.00 88.66
Svc/LinuxTimer 97.06 100.00 82.35
Svc/OsTime 70.00 83.33 60.98
Svc/PassiveRateGroup 100.00 100.00 89.47
Svc/PolyDb 100.00 100.00 53.57
Svc/PosixTime 100.00 100.00 75.00
Svc/PrmDb 92.75 89.47 88.24
Svc/RateGroupDriver 100.00 100.00 68.75
Svc/SeqDispatcher 73.08 80.00 71.05
Svc/StaticMemory 100.00 100.00 100.00
Svc/SystemResources 98.63 100.00 76.19
Svc/TlmChan 77.59 85.71 63.03
Svc/TlmPacketizer 92.18 100.00 77.45
Svc/Version 96.10 100.00 86.96
Utils 20.04 26.44 24.07
Utils/Types 92.11 95.83 77.08

Modules without UTs

CFDP/Checksum/GTest, Drv/ByteStreamDriverModel, Drv/Interfaces, Drv/LinuxGpioDriver, Drv/LinuxI2cDriver, Drv/LinuxSpiDriver, Drv/LinuxUartDriver, Drv/Ports, Drv/Ports/DataTypes, FppTestProject/FppTest/interfaces, 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/ports, FppTestProject/FppTest/topology/sync, FppTestProject/FppTest/topology/top_ports, FppTestProject/FppTest/topology/types, Fw/Cmd, Fw/Com, Fw/Comp, Fw/FilePacket/GTest, Fw/Fpy, Fw/Interfaces, Fw/Obj, Fw/Port, Fw/Ports/CompletionStatus, Fw/Ports/Ready, Fw/Ports/Signal, Fw/Ports/SuccessCondition, Fw/Prm, Fw/SerializableFile/test/TestSerializable, Fw/Sm, Fw/Test, Fw/Types/GTest, Os/Models, Svc/Cycle, Svc/DpPorts, Svc/Fatal, Svc/FatalHandler, Svc/FileDownlinkPorts, Svc/FprimeProtocol, Svc/Interfaces, Svc/PassiveConsoleTextLogger, Svc/Ping, Svc/PolyIf, Svc/Ports/CommsPorts, Svc/Ports/FilePorts, Svc/Ports/OsTimeEpoch, Svc/Ports/TlmPacketizerPorts, Svc/Ports/VersionPorts, Svc/Sched, Svc/Seq, Svc/Subtopologies/CdhCore, Svc/Subtopologies/ComCcsds, Svc/Subtopologies/ComFprime, Svc/Subtopologies/ComLoggerTee, Svc/Subtopologies/DataProducts, Svc/Subtopologies/FileHandling, Svc/Types/TlmPacketizerTypes, Svc/WatchDog, TestDeploymentsProject/Ref/PingReceiver, TestDeploymentsProject/Ref/RecvBuffApp, TestDeploymentsProject/Ref/SendBuffApp, TestDeploymentsProject/Ref/Top, TestDeploymentsProject/Ref/TypeDemo, cmake/test/data/TestDeployment/TestBuildAutocoder, cmake/test/data/TestDeployment/TestChainedAutocoder, cmake/test/data/TestDeployment/TestHeaderAutocoder, cmake/test/data/TestDeployment/TestTargetAutocoder, cmake/test/data/test-fprime-library/TestLibrary/TestComponent, cmake/test/data/test-fprime-library2/TestLibrary2/TestComponent

Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Devin Review found 0 new potential issues.

View 1 additional finding in Devin Review.

Open in Devin Review

@devin-ai-integration

Copy link
Copy Markdown
Author

Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it.

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.

0 participants