Skip to content

PRIVATE -> private for Utils/Types/CircularBuffer.hpp#3743

Merged
LeStarch merged 4 commits intonasa:develfrom
m-aleem:devel-3446-t
Jun 18, 2025
Merged

PRIVATE -> private for Utils/Types/CircularBuffer.hpp#3743
LeStarch merged 4 commits intonasa:develfrom
m-aleem:devel-3446-t

Conversation

@m-aleem
Copy link
Contributor

@m-aleem m-aleem commented Jun 17, 2025

Related Issue(s) #3446
Has Unit Tests (y/n) Yes - existing
Documentation Included (y/n) N/A

Change Description

Updates to Utils/Types/CircularBuffer.hpp* to change:

  • PRIVATE -> private
  • PROTECTED -> protected
  • Add friend tester, as needed

Rationale

#3446

Testing/Review Recommendations

N/A

Future Work

Additional PRs to be opened for further updates corresponding to #3446

Copy link
Collaborator

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

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

One minor fix. Also, CI is failing because of unrelated problems. We have a fix in progress here: #3744

(void)generate_random_fprime_frame(circular_buffer);
circular_buffer.m_allocated_size--; // Remove 1 byte from the end of the frame to trigger "more data needed"
// Remove 1 byte from the end of the frame to trigger "more data needed"
Types::CircularBufferTester::tester_m_allocated_size_decrement(circular_buffer);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thomas-bc brought up a good discussion point that we could alternatively make FrameAccumulatorTester directly a friend of CircularBuffer instead of using CircularBufferTester here (since FrameAccumulator uses the TEST (fixture-less) macro with calls to FrameAccumulatorTester).

In this case, is there's a reason to prefer that approach over this (or vice-versa)? I think I recall some discussion of the friendship pattern (limit?) .e.g. for X have XTester be the friend, so I guess that is an argument for the approach already shown here, but want to double check. @LeStarch ?

@thomas-bc
Copy link
Collaborator

CI should be fixed by merging latest devel in your branch

@m-aleem m-aleem requested a review from LeStarch June 17, 2025 19:21
@LeStarch LeStarch merged commit 5a04997 into nasa:devel Jun 18, 2025
52 of 53 checks passed
@m-aleem m-aleem deleted the devel-3446-t branch June 18, 2025 16:50
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.

3 participants