Skip to content

Conversation

vincewoo
Copy link
Collaborator

@vincewoo vincewoo commented Oct 9, 2025

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

Change Description

A pre-cursor to the work for #3429

This change introduces a parent class to SerializeBufferBase, named tentatively as SerialBufferBase. The intent is to have SerialBufferBase be the parent of both SerializeBufferBase (for linear buffers) and CircularBuffer.

As part of this change we will be improving the naming of some of the members of SerializeBufferBase that were confusing:

    DEPRECATED(Serializable::SizeType getBuffCapacity() const, "Use getCapacity() instead");
    DEPRECATED(Serializable::SizeType getBuffLength() const, "Use getSize() instead"); 
    DEPRECATED(Serializable::SizeType getBuffLeft(), "Use getDeserializeSizeLeft() instead");

    virtual Serializable::SizeType getCapacity() const = 0;     //!< returns capacity, not current size, of buffer
    Serializable::SizeType getSize() const;                     //!< returns current buffer size
    Serializable::SizeType getDeserializeSizeLeft() const;      //!< returns how much deserialization buffer is left
    Serializable::SizeType getSerializeSizeLeft() const;        //!< returns how much serialization space is left

Rationale

Having a common parent allows CircularBuffer to be used in a more standard way in the future and unlock the ability to use it interchangeably with use-cases where currently only linear buffers can be passed.

Testing/Review Recommendations

This change will require a refactoring of auto-coded classes. In the mean time I have manually edited the auto-coded files in my sandbox to the updated naming and ensured that everything builds properly. Ran F' UTs and everything passed.

Future Work

Since we've renamed some core functions of SerializeBufferBase we will need to update FPP to generate classes with the new names.

Naming scheme should be improved, the current SerializeBufferBase should really be called LinearSerialBuffer.

AI Usage (see policy)

No AI usage for this change.

@timcanham
Copy link
Collaborator

Is there an alternate way to do this now? I think I used it on a previous project to copy a serialized buffer to a new one while reserving space at the beginning of the new one.

@vincewoo
Copy link
Collaborator Author

Is there an alternate way to do this now? I think I used it on a previous project to copy a serialized buffer to a new one while reserving space at the beginning of the new one.

Thanks @timcanham we were hoping to sus out the use-case for this function. Would you happen to have a copy of the code so I can see how it is being used?

@vincewoo
Copy link
Collaborator Author

In the case of copyRaw, I see two uses of it within the fprime project:

// if non-empty, copy data
if (buffer.getBuffLeft()) {
// copy the serialized arguments to the buffer
stat = buffer.copyRaw(this->m_argBuffer, buffer.getBuffLeft());
}

void DpContainer::setDataHash(Utils::HashBuffer hash) {
U8* const buffAddr = this->m_buffer.getData();
const FwSizeType bufferSize = this->m_buffer.getSize();
const FwSizeType dataHashOffset = this->getDataHashOffset();
U8* const dataHashAddr = &buffAddr[dataHashOffset];
FW_ASSERT(dataHashOffset + HASH_DIGEST_LENGTH <= bufferSize,
static_cast<FwAssertArgType>(dataHashOffset + HASH_DIGEST_LENGTH),
static_cast<FwAssertArgType>(bufferSize));
ExternalSerializeBuffer serialBuffer(dataHashAddr, HASH_DIGEST_LENGTH);
hash.resetSer();
const Fw::SerializeStatus status = hash.copyRaw(serialBuffer, HASH_DIGEST_LENGTH);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast<FwAssertArgType>(status));
}

The copyRaw function is different than the copyRawOffset function in that copyRaw resets the destination buffer and copies the data. Note that both copyRaw* functions advances the deserialization pointer.

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