Skip to content

Conversation

@valdaarhun
Copy link
Contributor

Add new component that can be used by other components to resend a message on failure.

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

Change Description

Add a new component - ComRetry.

Rationale

Other components such as ComQueue can be connected to ComRetry to resend a message on failure.

Testing/Review Recommendations

Future Work

[ ] Add tests
[ ] Add CMakeLists.txt
[ ] Add docs

AI Usage (see policy)

NA

Add new component that can be used by other components to resend
a message on failure.
@valdaarhun
Copy link
Contributor Author

Hi. Apologies, it took much longer to draft this change. A lot has to be refined and the tests have not been added. I had a few questions and I thought I would get them clarified before continuing.

  • I am not sure if this is the right way to store the buffer in dataReturnIn_handler.
this->m_buffer = buffer

I was initially thinking of using DpContainer. Based on what I have understood from the documentation though, it would be an overkill to use it here.

Should Fw::ExternalSerializeBufferWithMemberCopy be used instead to store the buffer (similar to what's done in DpContainer::setBuffer())?

  • Once I get more clarity on the question above, I'll add in a check in comStatusIn_handler to test the validity of the buffer. I have marked it as TODO for now.
  • I am not sure how the following would work:
    else {
        this->dataReturnOut_out(0, this->m_buffer, this->m_context);
        this->m_retry_count = 0;
        Fw::Success condition = Fw::Success::FAILURE;
        this->comStatusOut_out(0, condition);
    }

Based on this comment:

  1. Will dataReturnOut here be connected to dataReturnIn in the ComQueue?
  2. Similarly, will comStatusOut be connected to ComQueue's comStatusIn?
  3. Given that comStatus is an async port while dataReturn is a sync port, can it be said that dataReturnIn will always be called before comStatusIn?
  4. How does this work under the hood? I tried reading through the relevant functions in ComQueueComponentAc.cpp but couldn't really grasp it.
  5. If I placed comStatusOut_out before dataReturnOut_out in the snippet above, would that make a difference?

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.

Minor changes, but this is what I was expecting.

void ComRetry ::comStatusIn_handler(FwIndexType portNum, Fw::Success& condition) {
// TODO: Check if m_buffer is valid before attempting delivery
if (condition == Fw::Success::SUCCESS) {
this->dataReturnOut_out(0, this->m_buffer, this->m_context);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should reset retry_count in the success case too.

@@ -0,0 +1,14 @@
module Svc {
@ A component for retrying message delivery on failure
active component ComRetry {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This need not be active. Make it passive

active component ComRetry {
import Svc.Framer

# One async command/port is required for active components
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the comments and below.

@LeStarch
Copy link
Collaborator

LeStarch commented Nov 3, 2025

First, you are storing things as I would expect. No need to do more. Second, your validity check isn't necessary either, as this component doesn't alter the validity.

To answer your enumerated questions:

  1. yes
  2. also yes
  3. yes, given that you have comstatus after data return in.
  4. You know that dataReturnOut finishes before comstatus is ever sent because dataReturnOut is synchronous and thus finishes all the work before the end of the function call.
  5. Yes it would. You have the expected order and should not reorder those calls.

@valdaarhun
Copy link
Contributor Author

First, you are storing things as I would expect. No need to do more. Second, your validity check isn't necessary either, as this component doesn't alter the validity.

To answer your enumerated questions:

  1. yes
  2. also yes
  3. yes, given that you have comstatus after data return in.
  4. You know that dataReturnOut finishes before comstatus is ever sent because dataReturnOut is synchronous and thus finishes all the work before the end of the function call.
  5. Yes it would. You have the expected order and should not reorder those calls.

Hi. Thank you for the answers.

Regarding the validity check, I was thinking that would take care of the case when ComRetry receives Fw::Success, but there's no buffer stored in the component.

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