diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 988edf77c..914cefe4d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,7 +3,13 @@ repos: rev: v5.0.0 hooks: - id: trailing-whitespace + # patches/*.patch are machine-generated diffs applied via `git apply`; + # their context/removed lines must byte-match the real upstream files, + # which can legitimately have trailing whitespace. Stripping it here + # silently breaks patch application. + exclude: ^patches/ - id: end-of-file-fixer + exclude: ^patches/ - id: check-yaml exclude: ^mkdocs\.yml$ - id: check-json diff --git a/Makefile b/Makefile index af9825162..a95bbd2f5 100644 --- a/Makefile +++ b/Makefile @@ -11,6 +11,17 @@ help: ## Display this help. submodules: ## Initialize and update git submodules @git submodule foreach --recursive 'git checkout -- . && git clean -fd' || true @git submodule update --init --recursive + @echo "Applying fprime TlmPacketizer I16 packetOffset patch (upstream candidate)..." + @cd lib/fprime && \ + if git apply --check ../../patches/fprime-tlmpacketizer-i16-offset.patch 2>/dev/null; then \ + git apply ../../patches/fprime-tlmpacketizer-i16-offset.patch && \ + echo "✓ Applied TlmPacketizer I16 packetOffset patch"; \ + elif git apply --reverse --check ../../patches/fprime-tlmpacketizer-i16-offset.patch 2>/dev/null; then \ + echo "⚠ Patch already applied"; \ + else \ + echo "❌ Error: Unable to apply TlmPacketizer I16 patch. Run 'cd lib/fprime && git status' to check."; \ + exit 1; \ + fi export VIRTUAL_ENV ?= $(shell pwd)/fprime-venv .PHONY: fprime-venv diff --git a/patches/README.md b/patches/README.md index 09cbaf396..20e3a28c0 100644 --- a/patches/README.md +++ b/patches/README.md @@ -2,6 +2,27 @@ This directory contains patches that are automatically applied to git submodules during the build process. +## fprime-tlmpacketizer-i16-offset.patch + +Shrinks `Svc::TlmPacketizer`'s per-channel `packetOffset` array from `FwSignedSizeType` +(64-bit) to `I16` in `lib/fprime`. The offset is a byte index into a ComBuffer-sized +packet (or the -1 "not in this packet" sentinel), so I16 is ample; the range `FW_ASSERT` +in `setPacketList` is tightened to the I16 max, and a `static_assert` checks at build time +that `FW_COM_BUFFER_MAX_SIZE` fits in I16. + +**Why:** With this project's `MAX_PACKETIZER_CHANNELS=202` and `MAX_PACKETIZER_PACKETS=22`, +the dense channels×packets offset matrix dominates the component: the patch shrinks the +`tlmSend` instance from 60,480 to 33,008 B of BSS (−27,472 B), which flows 1:1 into the +libc malloc arena (free-at-boot measured 11,504 → 38,976 B on a v5e board). + +**Status:** Upstream candidate for nasa/fprime; carried here until accepted (tracked in #469). +Delete this patch (and its `make submodules` step) once the pinned F Prime includes the +upstream fix — do not regenerate it on the next F Prime bump. + +**Application:** Applied automatically by `make submodules` with a reverse-apply +idempotency check. As with the other fprime patches, `lib/fprime` will show as modified — +do not commit the submodule pointer. + ## fprime-gds-version.patch This patch updates the `fprime-gds` version requirement in `lib/fprime/requirements.txt` from 4.1.0 to 4.1.1a2. diff --git a/patches/fprime-tlmpacketizer-i16-offset.patch b/patches/fprime-tlmpacketizer-i16-offset.patch new file mode 100644 index 000000000..04b92ab42 --- /dev/null +++ b/patches/fprime-tlmpacketizer-i16-offset.patch @@ -0,0 +1,44 @@ +diff --git a/Svc/TlmPacketizer/TlmPacketizer.cpp b/Svc/TlmPacketizer/TlmPacketizer.cpp +index a7d00b3..14974ed 100644 +--- a/Svc/TlmPacketizer/TlmPacketizer.cpp ++++ b/Svc/TlmPacketizer/TlmPacketizer.cpp +@@ -20,6 +20,8 @@ namespace Svc { + const TlmPacketizer_TelemetrySendPortMap TlmPacketizer::TELEMETRY_SEND_PORT_MAP = {}; + + static_assert(Svc::TelemetrySection::NUM_SECTIONS >= 1, "At least one telemetry section is required"); ++static_assert(FW_COM_BUFFER_MAX_SIZE <= std::numeric_limits::max(), ++ "TlmEntry::packetOffset is I16; FW_COM_BUFFER_MAX_SIZE must fit"); + + // ---------------------------------------------------------------------- + // Construction, initialization, and destruction +@@ -100,10 +102,10 @@ void TlmPacketizer::setPacketList(const TlmPacketizerPacketList& packetList, + entry.ignored = false; + entry.channelSize = channelSize; + // the offset into the buffer will be the current packet length +- // the offset must fit within FwSignedSizeType to allow for negative values +- FW_ASSERT(packetLen <= static_cast(std::numeric_limits::max()), ++ // the offset must fit within I16 to allow for the -1 sentinel value ++ FW_ASSERT(packetLen <= static_cast(std::numeric_limits::max()), + static_cast(packetLen)); +- entry.packetOffset[pktEntry] = static_cast(packetLen); ++ entry.packetOffset[pktEntry] = static_cast(packetLen); + + packetLen += entry.channelSize; + +diff --git a/Svc/TlmPacketizer/TlmPacketizer.hpp b/Svc/TlmPacketizer/TlmPacketizer.hpp +index f5ed26d..5ffb8f5 100644 +--- a/Svc/TlmPacketizer/TlmPacketizer.hpp ++++ b/Svc/TlmPacketizer/TlmPacketizer.hpp +@@ -170,9 +170,10 @@ class TlmPacketizer final : public TlmPacketizerComponentBase, public Fw::ParamE + + struct TlmEntry { + FwChanIdType id; //!< telemetry id stored in slot +- // Offsets into packet buffers. ++ // Offsets into packet buffers. Offsets are bounded by FW_COM_BUFFER_MAX_SIZE, so a ++ // 16-bit type suffices and keeps this channels x packets table small. + // -1 means that channel is not in that packet +- FwSignedSizeType packetOffset[MAX_PACKETIZER_PACKETS]; ++ I16 packetOffset[MAX_PACKETIZER_PACKETS]; + FwSizeType channelSize; //!< max serialized size of the channel in bytes + bool ignored; //!< ignored channel id + bool hasValue; //!< if the entry has received a value at least once