Skip to content

PrmDb: use canonical serialized size for record size#4954

Open
hidirektor wants to merge 4 commits intonasa:develfrom
hidirektor:devel
Open

PrmDb: use canonical serialized size for record size#4954
hidirektor wants to merge 4 commits intonasa:develfrom
hidirektor:devel

Conversation

@hidirektor
Copy link
Copy Markdown

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

Change Description

Updates Svc/PrmDb to use the canonical F Prime serialized size format for the file record size field.

  • Replaces raw U32 record-size serialization/deserialization with serializeSize(...) / deserializeSize(...)
  • Aligns record-size read/write checks with sizeof(FwSizeStoreType)
  • Adds a new FW_HAS_16_BIT == 1 regression test that:
    • writes a 16-bit parameter value,
    • saves the parameter file,
    • verifies the on-disk layout by deserializing the saved bytes,
    • reloads the file and confirms round-trip behavior

Rationale

This fixes the parameter database file format so it is not tied to host U32 assumptions and is more robust across configurations with different type sizes.

The added regression test protects against future format regressions, especially in non-default build configurations such as 16-bit payloads.

Testing/Review Recommendations

  • Review the record-size serialization path in Svc/PrmDb/PrmDbImpl.cpp
  • Review the new 16-bit regression coverage in Svc/PrmDb/test/ut/PrmDbTester.cpp
  • Confirm the read/write error expectations now use FwSizeStoreType-based sizing
  • Validate that the round-trip test still passes in FW_HAS_16_BIT == 1 builds

Future Work

  • Add broader coverage for additional non-default size/type configurations if needed
  • Consider adding a dedicated file-format compatibility test for other payload widths

AI Usage (see policy)

AI was used for review support.

@hidirektor hidirektor marked this pull request as ready for review April 4, 2026 21:04
case 3:
ASSERT_EVENTS_PrmFileReadError_SIZE(1);
ASSERT_EVENTS_PrmFileReadError(0, PrmReadError::PARAMETER_ID_SIZE, 0, sizeof(FwPrmIdType) + 1);
ASSERT_EVENTS_PrmFileReadError(0, PrmReadError::RECORD_SIZE_SIZE, 0, sizeof(FwSizeStoreType) + 1);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why did this change? I believe this loops through each record. Size was in case 2, so my expectation that case 3 remain PARAMETER_ID

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.

That may seem like an odd change at first, but this test fails by read() call index (m_waits), not by field name. With the size-format update, case 3 now hits a record-size read at the next record boundary, so PrmReadError::RECORD_SIZE_SIZE is the correct expectation

@LeStarch
Copy link
Copy Markdown
Collaborator

LeStarch commented Apr 6, 2026

To fix the formatting, you can run this command:

git diff --name-only devel...HEAD | fprime-util format --stdin

@hidirektor hidirektor requested a review from LeStarch April 10, 2026 01:25
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.

2 participants