modified: Drv/TcpClient/TcpClientComponentImpl.cpp

[menaman123]

Related Issue(s)	#2074
Has Unit Tests (y/n)	no
Documentation Included (y/n)	no

---

Change Description

Changed the getBuffer function to have a tunable size. There is a default size of 1024 so that it does not break any existing code.

Rationale

This removes the need of having to fork off of the fprime repo or autopatch everytime.

Testing/Review Recommendations

There is no unit tests to see if it'll break anything. Unsure where to begin to test to see.

Future Work

[LeStarch]

@menaman123 sorry, I didn't catch your question. I do like that you have defaulted this, but I'd move the allocation size to the configure method. Store the values a a member (e.g. m_allocation_size) and then pass that member to the call.

Note: We should do separate PRs for TcpServer and UDP too (once this one is approved)!

[menaman123]

@LeStarch Should i modify the getBuffer such that it returns the member allocate(0, m_allocation_size)?

[menaman123]

@LeStarch I changed the header file, I was wondering should the default buffer size be hardcoded as 1024 or a variable called DEFAULT_BUFFER_SIZE should be created instead?

[LeStarch]

One last change: use FwSizeType instead of size_t for these sizes. In F´ we use FwSizeType as the type of sizes.

I am ok with the hardcoded 1024. It is a default after all, and easily changed. Thanks for this change, we've needed it for some time!

[LeStarch]

You missed the member variable change to FwSizeType too. I made that edit for you so you don't have to keep chasing nits.

Your code looks good. Normally, we'd prefix all member variables with this->m_allocation_type .... but this file does not do that anywhere and you were correct to maintain style within this file.

Once CI passes, we'll merge this!

[menaman123]

Sweet thank you so much for the insight and teaching me the way!! @LeStarch

[LeStarch]

The issue with CI is this: F´ is in the process of transitioning from fixed-width types (e.g. U32) to project configurable types (e.g. FwSizeType). In this case Fw::Buffer uses an older style (U32) and your changes now update to the new style.

Here is how to fix this. Add a cast to the allocation call to convert types:

    return allocate_out(0, static_cast<U32>(m_allocation_size));

Add this check to the configure method:

    FW_ASSERT(buffer_size <= std::numeric_limits<U32>::max(), buffer_size);

This check ensures that the configured buffer size fits within the limit of the old structure. We do this in configure so that the assert trips as early as possible.

You may need to include these headers (if not already done so):

#include <limits>
#include <Fw/Types/Assert.hpp>