Add arg names to header and valid#12
Open
Lex-ari wants to merge 47 commits into
Open
Conversation
zimri-leisher
suggested changes
Apr 17, 2026
525bb7b to
31282aa
Compare
* initial document * Improved how-to docs * Add missing cards to how-to/index.md * nits * add tips * fix spelling * typo
* Add Os::File::open overloads for bounded char* and Fw::StringBase Add four new open() overloads to Os::File: - open(const char*, FwSizeType, Mode): bounded path without overwrite - open(const char*, FwSizeType, Mode, OverwriteType): bounded path with overwrite (core) - open(const Fw::StringBase&, Mode): StringBase without overwrite - open(const Fw::StringBase&, Mode, OverwriteType): StringBase with overwrite Refactor existing unbounded char* open() to delegate to the bounded version using FW_FIXED_LENGTH_STRING_SIZE as the default bound. The bounded char* with overwrite is now the core implementation; all other overloads delegate to it. The bounded core asserts that the path is null-terminated within the supplied length using Fw::StringUtils::string_length(). Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Fix off-by-one: use FW_FIXED_LENGTH_STRING_SIZE + 1 as buffer size bound FW_FIXED_LENGTH_STRING_SIZE is the max string length (256), not the buffer size. The length parameter represents buffer capacity including the null terminator, so pass STRING_SIZE + 1 (257) to match the Fw::String buffer size and allow full 256-character paths. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Address PR review: use ConstStringBase and FileNameStringSize - Change open(const Fw::StringBase&, ...) to open(const Fw::ConstStringBase&, ...) to accept string literals directly - Replace FW_FIXED_LENGTH_STRING_SIZE + 1 with FileNameStringSize from FPP config for the default path length bound in unbounded open() - Include config/FppConstantsAc.hpp for FileNameStringSize constant - Run fprime-util format for code style compliance Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Add unit test rules for bounded char* and ConstStringBase open overloads Add OpenFileCreateBounded and OpenFileCreateString rules to test the new Os::File::open() overloads: - OpenFileCreateBounded: tests open(const char*, FwSizeType, Mode, OverwriteType) - OpenFileCreateString: tests open(const Fw::ConstStringBase&, Mode, OverwriteType) Both rules are added to RandomizedInterfaceTesting and RandomizedTesting scenarios, plus dedicated Functionality tests. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Add edge-case death test for unterminated path within bounds Add OpenIllegalBoundedPath rule that verifies FW_ASSERT fires when a path buffer has no null terminator within the specified length bound. This exercises the string_length assertion in the bounded open() core. Added to both RandomizedInterfaceTesting and RandomizedTesting scenarios, plus a dedicated InvalidArguments.OpenBoundedPathUnterminated test. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Remove death test from randomized scenarios, keep as direct test only Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Fix dangling m_path pointer in OpenFileCreateString test rule The ConstStringBase open overload stores m_path pointing to the local Fw::String buffer, which is destroyed when the action returns. Reset m_path to point to the persistent filename string from the FILES vector after a successful open to prevent use-after-free in subsequent rules that call assert_file_consistent(). Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Fix uniqueness loop condition to match OpenBaseRule pattern Use '!this->m_overwrite' guard in the duplicate-filename loop for OpenFileCreateBounded and OpenFileCreateString, matching the existing OpenBaseRule pattern. Without this guard, the loop can exhaust the filename pool (MAX_FILES=100) and fail with 'Failed to generate unique filename' when the FILES vector is full. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Remove uniqueness loop from new open rules to prevent filename exhaustion The filename pool (MAX_FILES=100) can be exhausted when multiple OPEN_CREATE rules with NO_OVERWRITE compete for unique filenames in randomized scenarios. Remove the uniqueness loop from OpenFileCreateBounded and OpenFileCreateString — if the file already exists, both open() and shadow_open() return the same error status, so the test still validates correctly without requiring unique filenames. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Restore uniqueness loop in new open rules to match OpenBaseRule pattern The shadow file (SyntheticFile) uses a fresh filesystem per test while the real file (PosixFile) operates on the persistent disk filesystem. Without the uniqueness loop, filenames from prior tests that still exist on disk cause a mismatch: open() returns FILE_EXISTS but shadow_open() returns OP_OK, failing the ASSERT_EQ check. Restore the same uniqueness loop used by OpenBaseRule::action() in both OpenFileCreateBounded and OpenFileCreateString to ensure filenames are unique on the real filesystem before attempting to create. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Replace uniqueness loop with early-return for new open rules The uniqueness loop can exhaust the filename pool (get_filename caps at 101 entries) when many OPEN_CREATE rules compete in randomized scenarios. Once all cached filenames exist on disk, the loop spins forever. Replace with a simple early-return: if the randomly selected file already exists on the real filesystem, skip this iteration. The synthetic filesystem is reset per test and would not know about the file, causing a status mismatch. Skipping avoids both the mismatch and the exhaustion. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Bump MAX_FILES and gracefully handle filename exhaustion in all open rules Increase MAX_FILES from 100 to 500 in PosixFileTests.cpp to give the filename pool more headroom. The pool caps at MAX_FILES+1 entries because get_filename picks in [0, MAX_FILES]; once FILES.size() exceeds that, only cached filenames are returned and new generation stops. Also change the uniqueness loop in OpenBaseRule, OpenFileCreateBounded, and OpenFileCreateString to gracefully skip the iteration (return) instead of failing with ASSERT_LT when a unique filename cannot be found. This prevents test explosions if the pool is ever exhausted again. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Fix assert trip in unbounded open and improve assert diagnostics The unbounded open(char*, Mode, OverwriteType) was hardcoding FileNameStringSize + 1 (201) as the default length bound, which tripped the string_length assert for any path longer than 200 characters. Fix by measuring the actual string length to compute the bound, restoring the pre-refactoring behavior where this overload accepts any valid null-terminated C string. Also factor the string_length call into a const and pass both the computed string length and the caller-supplied length as arguments to FW_ASSERT for improved diagnostics on assert failures. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Keep FileNameStringSize bound and cap test filenames to fit Revert the unbounded open(char*, Mode, OverwriteType) back to using the correct F Prime bound of FileNameStringSize + 1 (201). Instead, cap the random test filename generation in PosixFileTests.cpp so the full path (BASE_PATH + '/' + random) fits within FileNameStringSize (200 chars). Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Tighten filename cap: subtract 1 for safety margin Reduce MAX_RANDOM_LEN by 1 so full paths are strictly less than FileNameStringSize (max 199 chars instead of 200). This ensures the string_length check in the bounded open core has margin even at the boundary. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Bound test buffers by FileNameStringSize instead of _POSIX_PATH_MAX Replace _POSIX_PATH_MAX with FileNameStringSize + 1 for full_buffer size and snprintf bound. The file path limit in F Prime is FileNameStringSize (200), not _POSIX_PATH_MAX (256). Using sizeof(full_buffer) in snprintf ensures the bound stays in sync with the buffer declaration. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Remove getFileSize check for too-long filename in ActiveTextLogger UT The test at line 255 passed a 201-char filename directly to Os::FileSystem::getFileSize(), which internally calls Os::File::open(). With the new bounded open implementation, this asserts because the path exceeds FileNameStringSize (200). The check was redundant since the test already verifies set_log_file() returned false and m_openFile is false. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Use Os::FileSystem::exists() instead of removing getFileSize check Per reviewer feedback: replace the removed getFileSize() call with Os::FileSystem::exists() to verify the file was not created on disk. exists() uses getPathType() (stat() on POSIX) which does not go through Os::File::open(), so it safely handles paths exceeding FileNameStringSize. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…sa#5068) * Modernize DpCatalog: in-class initializers and =default dtor Resolves several static analysis findings from nasa#4521 in Svc/DpCatalog: - 22x cpp:S3230 ("Do not use the constructor's initializer list for data member ...; use the in-class initializer instead"). All scalar and pointer members previously initialized in the constructor's member-initializer list are now default-initialized in the class body of DpCatalog.hpp. - 1x cpp:S3490 ("Use =default instead of the default implementation of this special member function"). The empty destructor is now declared = default in the header. Behavior is unchanged: members are still initialized in declaration order, which matched the previous initializer-list order, so the construction sequence is identical. Object members (m_directories[], m_fileList[], m_stateFile, m_currXmitFileName) are unchanged because they are default-constructed and were not flagged by SonarQube. Aligned with the in-class initializer style already used elsewhere in the repo (e.g. Svc/DpWriter, Svc/SeqDispatcher, Svc/Ccsds/AosFramer). * DpCatalog: move constructor body off the signature line Addresses CodeQL "More than one statement per line" alert introduced by the previous commit, which collapsed the constructor signature and the empty body onto the same line. Adds an explanatory comment inside the body to keep it on its own line; clang-format would otherwise re-collapse the empty `{}` under the repo's Chromium-based style. Behavior unchanged.
…asa#5014) * Fix ComQueue dropping oldest Fw::Buffer without returning ownership When a buffer queue configured with QUEUE_DROP_OLDEST overflows, the oldest queued Fw::Buffer was silently discarded by Queue::enqueue via CircularBuffer::rotate without returning ownership through bufferReturnOut. This leaked buffer-pool entries, eventually starving dependent communication or telemetry paths. Fix: - Add optional 'discarded' output parameter to Types::Queue::enqueue() that captures the about-to-be-dropped message data via peek before the rotate. - In ComQueue::enqueue(), pass an Fw::Buffer-sized output buffer for buffer queues and invoke bufferReturnOut_out() when DROP_OLDEST fires. Closes nasa#5011 Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Add discarded_size assert to Queue::enqueue, use proper Fw::Buffer serialization, add UTs - Queue::enqueue: added discarded_size parameter with FW_ASSERT validating the output buffer is large enough when discarded != nullptr - ComQueue: switched buffer queues from raw reinterpret_cast<U8*>(&buffer) to proper serializeTo/deserializeFrom using Fw::Buffer::SERIALIZED_SIZE. This affects buffQueueIn_handler (serialize before enqueue), the discard path in enqueue(), processQueue() (deserialize after dequeue), and drainQueue() (deserialize after dequeue). - Added 4 Queue UTs: DropOldestDiscardedOutput, DropOldestNullDiscarded, DropNewestIgnoresDiscarded, LIFODropOldestDiscardedOutput - Added 2 ComQueue UTs: ComQueueDropOldestNoBufferReturn (nullptr discarded path), BufferQueueFlushAfterDropOldest (drainQueue deserialization) All 14 Queue UTs and 25 ComQueue UTs pass. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Run clang-format on changed files Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Revert Queue changes, fix ComQueue drop-oldest with pre-emptive dequeue Reverted all Utils/Types/Queue changes (discarded parameter, UTs) per review feedback — the fix now lives entirely in ComQueue. ComQueue::enqueue() now checks if a buffer queue is full with DROP_OLDEST before calling Queue::enqueue(). When full, it dequeues the oldest entry first, returns buffer ownership via bufferReturnOut, then enqueues the new message into the now-available slot. This prevents buffer-pool leaks without modifying the Queue API. Also reverted the Fw::Buffer serialization changes (to be done in a separate PR) and removed the added ComQueue/Queue UTs. Updated existing testBufferQueueDropOldestMode assertion from 2 to 3 buffer returns to reflect the fix. All 10 Queue UTs and 22 ComQueue UTs pass. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Add Queue::popFront() to fix LIFO+DROP_OLDEST pre-emptive dequeue Queue::dequeue() respects queue mode (LIFO removes from back), but DROP_OLDEST always discards the front entry. The pre-emptive dequeue in ComQueue was using dequeue(), which would remove the wrong entry for LIFO+DROP_OLDEST queues. Added Queue::popFront() — always peeks offset 0 then rotates, matching the rotate-based removal that Queue::enqueue() uses for DROP_OLDEST. ComQueue::enqueue() now calls popFront() instead of dequeue() for the pre-emptive overflow path. Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> * Rename deqStatus to dequeueStatus to fix spelling abbreviation Co-Authored-By: michael.d.starch <michael.d.starch@jpl.nasa.gov> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
nasa#5087) * PR 5086 - Improved frame detector arithmetic against integer underflow and overflow * Fixed git spelling findings --------- Co-authored-by: bitWarrior <contact.bitWarrior@proton.me>
…nasa#5079) * Force Framers to setSize on Fw::Buffer before outputting it * Fix missing test coverage * Remove unused variable --------- Co-authored-by: bitWarrior <contact.bitWarrior@proton.me> Co-authored-by: M Starch <LeStarch@googlemail.com>
…e-save DB file (nasa#5080) * Fixed Potential DB file corruption on longer then shorter re-save DB file * Add note for AI-generated test in PrmDbTester Added a note indicating that the test was generated by AI. --------- Co-authored-by: bitWarrior <contact.bitWarrior@proton.me> Co-authored-by: Thomas Boyer-Chammard <49786685+thomas-bc@users.noreply.github.com>
* PR 5084-Fixed pkt_length integer overflow and replaced ASSERT w/ EVR in SpacePacketDeframer + unit tests * Fixed Git Spelling findings * Removing redundant length case * Fix CI errors * Fix CI again * Formatting * Fix broken UT * Fixing comments * fix comments --------- Co-authored-by: bitWarrior <contact.bitWarrior@proton.me> Co-authored-by: M Starch <LeStarch@googlemail.com> Co-authored-by: Thomas Boyer-Chammard <49786685+thomas-bc@users.noreply.github.com>
* Validate data product files before cataloging * Add DpCatalog tests for invalid FDP files * Formatting * Add DpCatalog invalid file event tests * Reverting .gitignore change Removed unnecessary entries from .gitignore --------- Co-authored-by: M Starch <LeStarch@googlemail.com>
* Fix ApidManager validate-when-table-full bug and reduce complexity
Fixes a bug in validateApidSeqCountIn_handler where the table-full guard
checked the wrong variable. The condition
receivedSeqCount != expectedSequenceCount && receivedSeqCount != SEQUENCE_COUNT_ERROR
is only correct if SEQUENCE_COUNT_ERROR were a valid received sequence
count. It is not: SEQUENCE_COUNT_ERROR is the sentinel returned by
getAndIncrementSeqCount when the APID table is full. The check should
guard on expectedSequenceCount, not receivedSeqCount.
When the table was full and a packet for an untracked APID arrived,
the buggy condition let the handler fall through to setNextSeqCount,
which insert()ed into a full ArrayMap, failed, and tripped the
FW_ASSERT in setNextSeqCount.
Refactor while we are here:
- Make the table-full case an explicit early return instead of a
sentinel comparison hidden inside a compound condition.
- Inline the one-shot setNextSeqCount helper into the validate handler
so the FW_ASSERT and its invariant (the APID was just successfully
tracked, so re-inserting cannot fail) are visible at the call site.
- Light reorder of getAndIncrementSeqCount for clearer flow.
Tests:
- Add ValidateSeqCount__NewTableFull rule covering the previously
untested path (validate against an untracked APID with a full table).
Wire it into the randomized scenario.
- Add a targeted ApidManager.ValidateSequenceCountWhenTableFull test
that fills the table and exercises the bug path. With the buggy code
this test trips the FW_ASSERT.
Docs:
- Update rule-based-testing how-to to include the new behavior in the
ApidManager example table.
Co-Authored-By: thomas.boyer.chammard <thomas.boyer.chammard@jpl.nasa.gov>
* more fixes
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* Replace AI-generated FrameAccumulator UTs with focused tests using existing infra The unit tests added in nasa#5087 had several issues raised in nasa#5089: 1. They re-defined helpers instead of reusing the existing test infrastructure in the *TestMain.cpp files. 2. Several tests duplicated existing coverage (e.g. TestBufferTooSmall vs InsufficientDataReturnsMoreDataNeeded). 3. The CCSDS TC tests built headers with flagsAndScId = 0, which the detector rejects at line 40 before reaching the underflow guard the tests claimed to exercise. This change deletes the two new test files and folds their meaningful intent into the existing TestMain files, using the existing fixture and the EXPECTED_START_TOKEN constant so the size guard is actually exercised: - FprimeFrameDetector: TestRejectsLengthFieldExceedingCapacity and TestHandlesMaximumLengthField verify the overflow guard / capacity check behaviour without crashing on extreme inputs. - CcsdsTcFrameDetector: TestRejectsTooSmallExpectedFrameLength and TestRejectsLengthJustBelowOverhead use a valid flagsAndScId so the underflow guard is what fires; TestMinimumSizedFrameDetected verifies the boundary case (header + trailer, no body) is accepted with a correct CRC. Refs: nasa#5089 * fix tests and comments --------- Co-authored-by: Devin AI <devin-ai-integration[bot]@users.noreply.github.com>
* PR 5036 - Event vs. ASSERT if DP file is corrupted * Fixed formatting errors * Formatting * Add back in commented out opcode * Comment out assertion for FileCorruptedDataError Comment out the assertion for FileCorruptedDataError in DpCatalogTester. This is because DpCatalog is incapable of handling return values from this function. Fixing it is outside the scope of this PR. * Uncomment assertions in DpCatalogTester.cpp --------- Co-authored-by: bitWarrior <contact.bitWarrior@proton.me> Co-authored-by: bitWarrior <bitWarrior@pm.me> Co-authored-by: M Starch <LeStarch@googlemail.com>
bd00ef1 to
48f821a
Compare
48f821a to
101e138
Compare
* Use shared BlockState for sequencer * Fix formatting
* Fix race in CMake chained autocoder test target
Two test targets (`_test_autocode` and `_test_chained_autocode`) both
invoke the `test_target_autocoder`, which registers an
`add_custom_command(OUTPUT ${BASENAME}.test-target.generated.txt
COMMAND ${CMAKE_COMMAND} -E copy ...)` for each module. CMake emits the
same copy rule into each custom target's submakefile, so under
`make -jN` the two rules can fire simultaneously and race on the
destination file, intermittently producing:
Error copying file "...test1.test-target.txt" to
"...test1.test-target.generated.txt".
Make the chained-autocode custom target depend on the `_test_autocode`
sibling. Make then serializes the two, ensuring the copy fires once and
the second build.make sees the destination as up-to-date.
Co-Authored-By: thomas.boyer.chammard <thomas.boyer.chammard@jpl.nasa.gov>
* Expand comment explaining the parallel-build race fix
Co-Authored-By: thomas.boyer.chammard <thomas.boyer.chammard@jpl.nasa.gov>
* Tighten comment on test_chained_autocoder serialization
Co-Authored-By: thomas.boyer.chammard <thomas.boyer.chammard@jpl.nasa.gov>
* Clarify comment
Added manual dependency to prevent race condition between test targets.
* fix spelling
* Differentiate chained-autocoder test inputs to avoid duplicate output filenames
Both target/test_autocoder and target/test_chained_autocoder used to invoke
autocoder/test_target_autocoder on the same module, so CMake emitted the same
`cmake -E copy` rule for *.test-target.generated.txt into both targets'
sub-makefiles. Under `make -jN` the two copies of the rule could fire in
parallel and race on the destination file, producing intermittent
'Error copying file' failures in CI.
Give the chained test its own input suffix (.chained-input.txt) and a dedicated
first-stage autocoder (autocoder/test_chained_input_autocoder) that emits
.chained-input.generated.txt. The standalone test_target_autocoder now only
matches files in TestTargetAutocoder, so no two test targets produce the same
output filename and the race is eliminated at the source.
* Revert "Differentiate chained-autocoder test inputs to avoid duplicate output filenames"
This reverts commit d4df7f9.
* Make test_autocoder and test_chained_autocoder targets non-overlapping
Both targets are registered globally and applied to every module, and both
invoke test_target_autocoder. CMake's Makefile generator then emits the same
`cmake -E copy` rule into both submakefiles for any module reached by both
targets, causing make -jN to race on the destination file ("Error copying
file ...").
Each test fixture only needs one of the two targets, so opt out of the other
via a CMake variable read inside the target's add_module_target hook:
- TestChainedAutocoderModule sets SKIP_TEST_AUTOCODER_TARGET. The chained
target still runs test_target_autocoder as the genuine first stage.
- TestTargetAutocoderModule sets SKIP_TEST_CHAINED_AUTOCODER_TARGET. The
standalone target keeps testing test_target_autocoder by itself.
The flags must be set before register_fprime_module since custom targets are
attached inside that call.
Each cmake -E copy rule is now emitted into exactly one submakefile, so the
race cannot occur.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix spelling
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Devin <devin@cognition.ai>
* Claude updates to use Red/Black tree. Not reviewed yet, but unit tests pass * Fix failing unit tests * Removing erroneous sorting * Have UT handle duplicate values * Formatting * Removing AI planning files * Disable random seed initialization in tests Comment out the random seed initialization for tests. * Fix TODO comment formatting in DpCatalogTestMain.cpp --------- Co-authored-by: M Starch <LeStarch@googlemail.com>
LockGuard in Utils was removed in 2024 and was replaced by Os::ScopeLock. Ref: nasa#2907
* Adding guide for selecting components * Cross-linking * Grammer and spelling * Update docs/user-manual/framework/component-and-port-selection.md Co-authored-by: Thomas Boyer-Chammard <49786685+thomas-bc@users.noreply.github.com> * Fix review recomendations * Fix review II --------- Co-authored-by: Thomas Boyer-Chammard <49786685+thomas-bc@users.noreply.github.com>
* Init AOS (De)Framer from TC & TM * Add AOS Suppport Subset list to sdd * Start Refining AOS Framer User Interface * Move PVN to Types.fpp & Fill out TFVN * AOS Framer Header * Refactor the Virtual Channel as a struct * More work on idle packet serialization * Move PVN back into ComCfg * AosFramer Builds * More Section Number & Refactor Sending, Packing, & Header Writing logic * Start Cleaning up AosFramer w/ Unit Tests * Ensure SPP Idle Fill increments payload offset * Mask & shift (don't shift masks) * Nothing too large & sendNow for buffer ownership test * Fix SeqCount wrap around test * Created, but failing long packet test * Add 2 TODOs * Make AosFrmer Happy on aarch64 * Add lldb vscode launch.json config * Passing striped packet test (Packet across multiple frames) * Correct Author & Prune Outdated content from sdd * Add SPP Idle header asserts * Test many packets per frame (short packets) * Remove (the not yet implemented) AosDeframer * Add more words to expected * More spelling * Docs spelling * Apply a subset of the spelling bot patch * fix `re-entrancy` and fight w/ `aos` spelling pattern * just `aos` * `aos` -> `Aos` * Revert launch.json and macos library duplicate change in other PR * Correct annotations for PVN dict * Make Spacing uniform for ComCfg.fpp annotations * Rename compute_fecf to compute_and_inject_fecf * Correct sdd's description of provided service to VCP * Add a pair of asserts to try to make RHEL compile CICD happy * Add pack_pad_send helper to avoid pack_packet reentrancy * Update sdd to point at version 5 of the AOS SDL Standard * Default FECF on & constant driven AOS Frame Size * More RHEL8 appeasal * RHEL8 UT Appeasement * VC ID default to 1, replay flag gone, & serialize idle spp takes a U16 length * Improve comment about 24 + 4 bit frame count * More RHEL8 UT complaints * expect.txt `bitfield` * Back to expecting `Bitfield` * Shift then Mask to hopefully silence RHEL UT * spelling * Try Unsigned masks * Add back in Darwin linker dupe warning suppression * Promote to unsigned int before masking or shifting * Hope that U32 cast is enough to ward of promotion * Try separate bitwise or and equals for RHEL 8 issues * Add AOS Deframer component with EPP support Implements CCSDS AOS Space Data Link Protocol deframer (CCSDS 732.0-B-5) with support for: - M_PDU data field service with First Header Pointer processing - Frame Error Control Field (FECF/CRC16) validation - Space Packet Protocol (SPP) extraction per CCSDS 133.0-B-2 - Encapsulation Packet Protocol (EPP) extraction per CCSDS 133.1-B-3 - Packet spanning across multiple frames - Virtual Channel ID filtering - Configurable PVN mask for packet type filtering Includes comprehensive unit tests covering: - Frame header validation (TFVN, SCID, VCID) - CRC validation - M_PDU FHP processing (including idle and no-packet-start cases) - Spanning packet assembly across 2-3 frames - SPP/EPP extraction and idle packet filtering - Configuration options (FECF enable/disable, PVN masks) - Telemetry counters https://claude.ai/code/session_01SmSgEGnncHPGTXwvBYNLVt * Make it Compile * Address PR review comments and fix spell check failures Types.fpp: - Rename MPDUSubfields -> AOSMPDUSubfields for consistency - Remove expectedFrameVersion constant; reference Tfvn::AOS from enum - Add explanation comment for AOS deframer masks in AOSHeaderSubfields - Strip lol*/type/protocol constants from EPPSubfields - Add PacketVersionNumber enum (SPP=0, EPP=7) - Add EppPacketType enum (Encapsulation, EncapsulationIdle) - Add EppProtocolId enum (Extended, IPv4Extension, MissionSpecific) - Add EppLengthOfLength enum (Fill, OneOctet, TwoOctets, FourOctets, EightOctets) - Add TODO comment about merging AOS/TC FrameError variants AosDeframer.hpp/.cpp: - Introduce AosDeframerVc per-VC struct mirroring AosFramer::AosVc pattern - Move counters, pvnMask, and spanning packet state into AosDeframerVc - Replace FW_ASSERT on header deserialization with graceful error return - Use Tfvn::AOS enum value for TFVN comparison - Remove replay flag extraction (deferred until needed) - Add VC Frame Count Cycle Count support - Store vcFrameCount in vc struct instead of discarding it - Add appendToSpanningPacket() helper to eliminate duplicated logic - Fix validateFecf() to accept AosDeframerVc& instead of using dummy context - Use PacketVersionNumber and EppPacketType enums throughout - Fix "LSBs"/"MSBs" comments -> "LS bits"/"MS bits" AosDeframerTester.cpp: - Rename MPDUSubfields:: -> AOSMPDUSubfields:: .github/actions/spelling/expect.txt: - Add epp, fhp, mpdu to fix spell check CI failures Co-Authored-By: Will MacCormack <Willmac16@users.noreply.github.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Remove Pvn from ComCfg * EPP is 7 not 3 * Address review feedback: refactor events/telem, fix PvnBitfield, remove acceptAllVcid - Types.fpp: delete obsolete comments, remove duplicate PacketVersionNumber enum (use ComCfg::Pvn instead), fix PvnBitfield::EPP_MASK from 0x8 to 0x80 (EPP PVN=7, so 1<<7=0x80) and VALID_MASK to 0x81 - AosDeframer.fpp: split events into AosDeframerEvents.fppi and telemetry into AosDeframerTelem.fppi; update include directives - AosDeframerEvents.fppi: rename InvalidCrc->InvalidFecf (spec terminology), add vcId to IdleFrame(), update InvalidVcId format to list accepted VCIDs, simplify InvalidEppPacket (remove version arg), add InvalidSppPacket event - AosDeframer.hpp: remove errorNotifyHelper, remove m_acceptAllVcid, move crcErrorCount from per-VC struct to global m_crcErrorCount (FECF errors are a physical-channel concern), update validateFecf signature, fix SpanningPacketState to use static constexpr for HEADER_BUF_SIZE, init pvn to 0xFF (unset) - AosDeframer.cpp: inline errorNotify calls (no helper needed), zero out m_crcErrorCount in configure(), add FW_ASSERT to reject invalid pvnMask bits, replace PacketVersionNumber with ComCfg::Pvn, always filter by VCID (no acceptAllVcid mode), simplify parseAndValidateHeader by using struct accessors directly, use pre-increment for telemetry counters, convert while->for in extractPackets, update IdleFrame event call with vcId - ComCfg.fpp: add pvn: Pvn field to FrameContext (required for packet type context propagation from deframer to downstream consumers) - CMakeLists.txt: add fppi files to AUTOCODER_INPUTS - Testers: rename testInvalidCrc->testInvalidFecf, remove testAcceptAllVcid, fix all configure() calls (drop acceptAllVcid param), update event assertions - expect.txt: add aosmpdu for spell check Co-authored-by: Will MacCormack <Willmac16@users.noreply.github.com> * Just enough to compile * More AosDeframer UTs * Move Test Helpers to separate file * Clean up Types.fpp * NumVcs const * Normalize Transmitted/Actual vs Expected/Configured ordering of mismatch events * Improve clarity on different vc struct counts * Algin telem & member var for 3 types of counts * FECF first & vc as pointer (allow null return) * notifyErrorIfConnected helper * WIP: Gap Detect * Reset Spanning * Steal back the Dyn backed spanning packet logic from 567ec7b * Couple more tweaks from an alt timeline * Minor Cleanup * So Much TODO * Valid but disabled packet warning * Use ApidMask in AosDeframer * Make the tests happy * appendToSpanningPacket is now a void return * Fix EPP & appendToSpanning I'm happy w/ * Actually Builds * Low Effort TODOs * Use the SpacePacketHeader w/ deserialize for sizeSppPacket * Tests fail, but are small & compile * on alloc failure should move to next packet * About to clean slate the tests * Remember how to nominal frame * Mostly Passing * All passing * Remove unused invalid EPP & SPP events * Fix epp length writing (hopefully) and add some more coverage * Reduce duplicate length checking, EPP Length of Length is not a number, & test LoL + header wrapping * Assert we have at least one byte * Packet Abandonment event + test, frame count tlm checks, & fix for packet context overwrite * Spelling + assert on the vcID * Add `lol` to expect.txt * Only check the frame count width we RX-ed * tlm FramesProcessed after we finish frame operations * Separate SPP and EPP cases in sizePacket * Bail Early if PVN is not enabled * Assert rx-ed is less than/eq to HEADER_BUFFER_SIZE * Break vcFrameCount assignment & tlm into 2 lines * Remove the `VCx` aliases * More asserts for memcopies * Add more comments and aserts to the appendToSpanningPacket working from header portion * Fix CCSDS Types Enum values * Add U8 Static Casts for AOS Deframer Tester * Don't drop on OID (Only Idle Data) * Correct UT to expect spanning packets are kept upon Only Idle Data frames * RHEL8 Werror=conversion is my nemesis * Include cstring * appease GCC * formatting * staging * use TEST_DATA_ZONE_SIZE * add opus4.7 findings * Abandon any spanning packets upon OOB FHP * Spelling * Remove those trailing spaces --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Will MacCormack <Willmac16@users.noreply.github.com> Co-authored-by: Thomas Boyer-Chammard <49786685+thomas-bc@users.noreply.github.com> Co-authored-by: thomas-bc <thomas.boyer.chammard@jpl.nasa.gov> Co-authored-by: thomas-bc <thomas.boyerchammard@gmail.com>
* Banishing Ref to a project directory * Fixing CI catches * Change directory to TestDeploymentsProject for build Signed-off-by: M Starch <LeStarch@googlemail.com> --------- Signed-off-by: M Starch <LeStarch@googlemail.com>
Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.6.3 to 2.7.0. - [Release notes](https://github.com/urllib3/urllib3/releases) - [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst) - [Commits](urllib3/urllib3@2.6.3...2.7.0) --- updated-dependencies: - dependency-name: urllib3 dependency-version: 2.7.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change Description
Rationale
Testing/Review Recommendations
Future Work
AI Usage (see policy)