Skip to content

Commit

Permalink
[indirect-sender] remove callbacks to its sub-components (openthread#…
Browse files Browse the repository at this point in the history 
…10955)

This commit removes the `Callbacks` class defined for use between
`IndirectSender` and its underlying `DataPollHandler` and
`CslTxScheduler` components. This model was added earlier in openthread#3952 to
make all interactions between these modules asynchronous
(callback-based). The objective was to move the handling of data
polls and indirect transmissions closer to the MAC layer and allow
`DataPollHandler` logic to be moved to RCP in an host and RCP
architecture. With the introduction of CSL, this approach is no
longer viable, but the callback style was still used. This commit
removes this and simplifies the code.
  • Loading branch information
@abtink
abtink authored Nov 22, 2024
1 parent 025d84b commit 35cc162
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 215 deletions.
50 changes: 5 additions & 45 deletions src/core/mac/data_poll_handler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,38 +41,10 @@ namespace ot {

RegisterLogModule("DataPollHandlr");

DataPollHandler::Callbacks::Callbacks(Instance &aInstance)
: InstanceLocator(aInstance)
{
}

inline Error DataPollHandler::Callbacks::PrepareFrameForChild(Mac::TxFrame &aFrame,
FrameContext &aContext,
Child &aChild)
{
return Get<IndirectSender>().PrepareFrameForChild(aFrame, aContext, aChild);
}

inline void DataPollHandler::Callbacks::HandleSentFrameToChild(const Mac::TxFrame &aFrame,
const FrameContext &aContext,
Error aError,
Child &aChild)
{
Get<IndirectSender>().HandleSentFrameToChild(aFrame, aContext, aError, aChild);
}

inline void DataPollHandler::Callbacks::HandleFrameChangeDone(Child &aChild)
{
Get<IndirectSender>().HandleFrameChangeDone(aChild);
}

//---------------------------------------------------------

DataPollHandler::DataPollHandler(Instance &aInstance)
: InstanceLocator(aInstance)
, mIndirectTxChild(nullptr)
, mFrameContext()
, mCallbacks(aInstance)
{
}

Expand All @@ -89,18 +61,6 @@ void DataPollHandler::Clear(void)
mIndirectTxChild = nullptr;
}

void DataPollHandler::HandleNewFrame(Child &aChild)
{
OT_UNUSED_VARIABLE(aChild);

// There is no need to take any action with current data poll
// handler implementation, since the preparation of the frame
// happens after receiving of a data poll from the child. This
// method is included for use by other data poll handler models
// (e.g., in RCP/host model if the handling of data polls is
// delegated to RCP).
}

void DataPollHandler::RequestFrameChange(FrameChange aChange, Child &aChild)
{
if ((mIndirectTxChild == &aChild) && Get<Mac::Mac>().IsPerformingIndirectTransmit())
Expand All @@ -119,7 +79,7 @@ void DataPollHandler::RequestFrameChange(FrameChange aChange, Child &aChild)
else
{
ResetTxAttempts(aChild);
mCallbacks.HandleFrameChangeDone(aChild);
Get<IndirectSender>().HandleFrameChangeDone(aChild);
}
}

Expand Down Expand Up @@ -183,7 +143,7 @@ Mac::TxFrame *DataPollHandler::HandleFrameRequest(Mac::TxFrames &aTxFrames)
frame = &aTxFrames.GetTxFrame();
#endif

VerifyOrExit(mCallbacks.PrepareFrameForChild(*frame, mFrameContext, *mIndirectTxChild) == kErrorNone,
VerifyOrExit(Get<IndirectSender>().PrepareFrameForChild(*frame, mFrameContext, *mIndirectTxChild) == kErrorNone,
frame = nullptr);

#if OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
Expand Down Expand Up @@ -234,7 +194,7 @@ void DataPollHandler::HandleSentFrame(const Mac::TxFrame &aFrame, Error aError,
aChild.SetFramePurgePending(false);
aChild.SetFrameReplacePending(false);
ResetTxAttempts(aChild);
mCallbacks.HandleFrameChangeDone(aChild);
Get<IndirectSender>().HandleFrameChangeDone(aChild);
ExitNow();
}

Expand All @@ -261,7 +221,7 @@ void DataPollHandler::HandleSentFrame(const Mac::TxFrame &aFrame, Error aError,
{
aChild.SetFrameReplacePending(false);
ResetTxAttempts(aChild);
mCallbacks.HandleFrameChangeDone(aChild);
Get<IndirectSender>().HandleFrameChangeDone(aChild);
ExitNow();
}

Expand Down Expand Up @@ -295,7 +255,7 @@ void DataPollHandler::HandleSentFrame(const Mac::TxFrame &aFrame, Error aError,
OT_ASSERT(false);
}

mCallbacks.HandleSentFrameToChild(aFrame, mFrameContext, aError, aChild);
Get<IndirectSender>().HandleSentFrameToChild(aFrame, mFrameContext, aError, aChild);

exit:
return;
Expand Down
96 changes: 7 additions & 89 deletions src/core/mac/data_poll_handler.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,70 +132,6 @@ class DataPollHandler : public InstanceLocator, private NonCopyable
static_assert(kMaxPollTriggeredTxAttempts < (1 << 5), "mIndirectTxAttempts cannot fit max!");
};

/**
* Defines the callbacks used by the `DataPollHandler`.
*/
class Callbacks : public InstanceLocator
{
friend class DataPollHandler;

private:
/**
* Defines the frame context associated with a prepared frame.
*
* Data poll handler treats `FrameContext` as an opaque data type. Data poll handler provides the buffer/object
* for the context when a new frame is prepared (from the callback `PrepareFrameForChild()`). It ensures
* to save the context along with the prepared frame and provide the same context back in the callback
* `HandleSentFrameToChild()` when the indirect transmission of the frame is finished.
*/
typedef IndirectSenderBase::FrameContext FrameContext;

/**
* Initializes the callbacks object.
*
* @param[in] aInstance A reference to the OpenThread instance.
*/
explicit Callbacks(Instance &aInstance);

/**
* This callback method requests a frame to be prepared for indirect transmission to a given sleepy child.
*
* @param[out] aFrame A reference to a MAC frame where the new frame would be placed.
* @param[out] aContext A reference to a `FrameContext` where the context for the new frame would be placed.
* @param[in] aChild The child for which to prepare the frame.
*
* @retval kErrorNone Frame was prepared successfully.
* @retval kErrorAbort Indirect transmission to child should be aborted (no frame for the child).
*/
Error PrepareFrameForChild(Mac::TxFrame &aFrame, FrameContext &aContext, Child &aChild);

/**
* This callback method notifies the end of indirect frame transmission to a child.
*
* @param[in] aFrame The transmitted frame.
* @param[in] aContext The context associated with the frame when it was prepared.
* @param[in] aError kErrorNone when the frame was transmitted successfully,
* kErrorNoAck when the frame was transmitted but no ACK was received,
* kErrorChannelAccessFailure tx failed due to activity on the channel,
* kErrorAbort when transmission was aborted for other reasons.
* @param[in] aChild The child to which the frame was transmitted.
*/
void HandleSentFrameToChild(const Mac::TxFrame &aFrame,
const FrameContext &aContext,
Error aError,
Child &aChild);

/**
* This callback method notifies that a requested frame change from `RequestFrameChange()` is processed.
*
* This callback indicates to the next layer that the indirect frame/message for the child can be safely
* updated.
*
* @param[in] aChild The child to update.
*/
void HandleFrameChangeDone(Child &aChild);
};

/**
* Initializes the data poll handler object.
*
Expand All @@ -208,19 +144,6 @@ class DataPollHandler : public InstanceLocator, private NonCopyable
*/
void Clear(void);

/**
* Informs data poll handler that there is a new frame for a given child.
*
* After this call, the data poll handler can use the `Callbacks::PrepareFrameForChild()` method to request the
* frame to be prepared. A subsequent call to `Callbacks::PrepareFrameForChild()` should ensure to prepare the same
* frame (this is used for retransmissions of frame by data poll handler). If/When the frame transmission is
* finished, the data poll handler will invoke the `Callbacks::HandleSentFrameToChild()` to indicate the status of
* the frame transmission.
*
* @param[in] aChild The child which has a new frame.
*/
void HandleNewFrame(Child &aChild);

/**
* Requests a frame change for a given child.
*
Expand All @@ -231,7 +154,7 @@ class DataPollHandler : public InstanceLocator, private NonCopyable
*
* If there is no ongoing indirect frame transmission to the child, the request will be handled immediately and the
* callback `HandleFrameChangeDone()` is called directly from this method itself. This callback notifies the next
* layer that the indirect frame/message for the child can be safely updated.
* layer (`IndirectSender`) that the indirect frame/message for the child can be safely updated.
*
* If there is an ongoing indirect frame transmission to this child, the request can not be handled immediately.
* The following options can happen based on the request type:
Expand All @@ -240,8 +163,8 @@ class DataPollHandler : public InstanceLocator, private NonCopyable
* callback `HandleFrameChangeDone()` is invoked.
*
* 2) In case of "replace" request, the ongoing indirect transmission is allowed to finish (current tx attempt).
* 2.a) If the tx attempt is successful, the `Callbacks::HandleSentFrameToChild()` in invoked which indicates
* the "replace" could not happen (in this case the `HandleFrameChangeDone()` is no longer called).
* 2.a) If the tx attempt is successful, the `IndirectSender::HandleSentFrameToChild()` in invoked which
* indicates the "replace" could not happen (in this case `HandleFrameChangeDone()` is no longer called).
* 2.b) If the ongoing tx attempt is unsuccessful, then callback `HandleFrameChangeDone()` is invoked to allow
* the next layer to update the frame/message for the child.
*
Expand All @@ -255,6 +178,8 @@ class DataPollHandler : public InstanceLocator, private NonCopyable
void RequestFrameChange(FrameChange aChange, Child &aChild);

private:
typedef IndirectSenderBase::FrameContext FrameContext;

// Callbacks from MAC
void HandleDataPoll(Mac::RxFrame &aFrame);
Mac::TxFrame *HandleFrameRequest(Mac::TxFrames &aTxFrames);
Expand All @@ -264,15 +189,8 @@ class DataPollHandler : public InstanceLocator, private NonCopyable
void ProcessPendingPolls(void);
void ResetTxAttempts(Child &aChild);

// In the current implementation of `DataPollHandler`, we can have a
// single indirect tx operation active at MAC layer at each point of
// time. `mIndirectTxChild` indicates the child being handled (`nullptr`
// indicates no active indirect tx). `mFrameContext` tracks the
// context for the prepared frame for the current indirect tx.

Child *mIndirectTxChild;
Callbacks::FrameContext mFrameContext;
Callbacks mCallbacks;
Child *mIndirectTxChild; // The child being handled (`nullptr` indicates no active indirect tx).
FrameContext mFrameContext; // Context for the prepared frame for the current indirect tx (if any)
};

/**
Expand Down
28 changes: 3 additions & 25 deletions src/core/thread/csl_tx_scheduler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,34 +36,11 @@ namespace ot {

RegisterLogModule("CslTxScheduler");

CslTxScheduler::Callbacks::Callbacks(Instance &aInstance)
: InstanceLocator(aInstance)
{
}

inline Error CslTxScheduler::Callbacks::PrepareFrameForChild(Mac::TxFrame &aFrame,
FrameContext &aContext,
Child &aChild)
{
return Get<IndirectSender>().PrepareFrameForChild(aFrame, aContext, aChild);
}

inline void CslTxScheduler::Callbacks::HandleSentFrameToChild(const Mac::TxFrame &aFrame,
const FrameContext &aContext,
Error aError,
Child &aChild)
{
Get<IndirectSender>().HandleSentFrameToChild(aFrame, aContext, aError, aChild);
}

//---------------------------------------------------------

CslTxScheduler::CslTxScheduler(Instance &aInstance)
: InstanceLocator(aInstance)
, mCslTxChild(nullptr)
, mCslTxMessage(nullptr)
, mFrameContext()
, mCallbacks(aInstance)
{
UpdateFrameRequestAhead();
}
Expand Down Expand Up @@ -190,7 +167,8 @@ Mac::TxFrame *CslTxScheduler::HandleFrameRequest(Mac::TxFrames &aTxFrames)
frame = &aTxFrames.GetTxFrame();
#endif

VerifyOrExit(mCallbacks.PrepareFrameForChild(*frame, mFrameContext, *mCslTxChild) == kErrorNone, frame = nullptr);
VerifyOrExit(Get<IndirectSender>().PrepareFrameForChild(*frame, mFrameContext, *mCslTxChild) == kErrorNone,
frame = nullptr);
mCslTxMessage = mCslTxChild->GetIndirectMessage();
VerifyOrExit(mCslTxMessage != nullptr, frame = nullptr);

Expand Down Expand Up @@ -328,7 +306,7 @@ void CslTxScheduler::HandleSentFrame(const Mac::TxFrame &aFrame, Error aError, C
OT_UNREACHABLE_CODE(break);
}

mCallbacks.HandleSentFrameToChild(aFrame, mFrameContext, aError, aChild);
Get<IndirectSender>().HandleSentFrameToChild(aFrame, mFrameContext, aError, aChild);

exit:
return;
Expand Down
56 changes: 6 additions & 50 deletions src/core/thread/csl_tx_scheduler.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,51 +156,6 @@ class CslTxScheduler : public InstanceLocator, private NonCopyable
static_assert(kMaxCslTriggeredTxAttempts < (1 << 7), "mCslTxAttempts cannot fit max!");
};

/**
* Defines the callbacks used by the `CslTxScheduler`.
*/
class Callbacks : public InstanceLocator
{
friend class CslTxScheduler;

private:
typedef IndirectSenderBase::FrameContext FrameContext;

/**
* Initializes the callbacks object.
*
* @param[in] aInstance A reference to the OpenThread instance.
*/
explicit Callbacks(Instance &aInstance);

/**
* This callback method requests a frame to be prepared for CSL transmission to a given SSED.
*
* @param[out] aFrame A reference to a MAC frame where the new frame would be placed.
* @param[out] aContext A reference to a `FrameContext` where the context for the new frame would be placed.
* @param[in] aChild The child for which to prepare the frame.
*
* @retval kErrorNone Frame was prepared successfully.
* @retval kErrorAbort CSL transmission should be aborted (no frame for the child).
*/
Error PrepareFrameForChild(Mac::TxFrame &aFrame, FrameContext &aContext, Child &aChild);

/**
* This callback method notifies the end of CSL frame transmission to a child.
*
* @param[in] aFrame The transmitted frame.
* @param[in] aContext The context associated with the frame when it was prepared.
* @param[in] aError kErrorNone when the frame was transmitted successfully,
* kErrorNoAck when the frame was transmitted but no ACK was received,
* kErrorChannelAccessFailure tx failed due to activity on the channel,
* kErrorAbort when transmission was aborted for other reasons.
* @param[in] aChild The child to which the frame was transmitted.
*/
void HandleSentFrameToChild(const Mac::TxFrame &aFrame,
const FrameContext &aContext,
Error aError,
Child &aChild);
};
/**
* Initializes the CSL tx scheduler object.
*
Expand Down Expand Up @@ -232,6 +187,8 @@ class CslTxScheduler : public InstanceLocator, private NonCopyable
// Guard time in usec to add when checking delay while preparing the CSL frame for tx.
static constexpr uint32_t kFramePreparationGuardInterval = 1500;

typedef IndirectSenderBase::FrameContext FrameContext;

void RescheduleCslTx(void);

uint32_t GetNextCslTransmissionDelay(const Child &aChild, uint32_t &aDelayFromLastRx, uint32_t aAheadUs) const;
Expand All @@ -242,11 +199,10 @@ class CslTxScheduler : public InstanceLocator, private NonCopyable

void HandleSentFrame(const Mac::TxFrame &aFrame, Error aError, Child &aChild);

uint32_t mCslFrameRequestAheadUs;
Child *mCslTxChild;
Message *mCslTxMessage;
Callbacks::FrameContext mFrameContext;
Callbacks mCallbacks;
uint32_t mCslFrameRequestAheadUs;
Child *mCslTxChild;
Message *mCslTxMessage;
FrameContext mFrameContext;
};

/**
Expand Down
3 changes: 0 additions & 3 deletions src/core/thread/indirect_sender.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,6 @@ void IndirectSender::UpdateIndirectMessage(Child &aChild)
{
Mac::Address childAddress;

mDataPollHandler.HandleNewFrame(aChild);

aChild.GetMacAddress(childAddress);
Get<MeshForwarder>().LogMessage(MeshForwarder::kMessagePrepareIndirect, *message, kErrorNone, &childAddress);
}
Expand Down Expand Up @@ -460,7 +458,6 @@ void IndirectSender::HandleSentFrameToChild(const Mac::TxFrame &aFrame,
if ((message != nullptr) && (nextOffset < message->GetLength()))
{
aChild.SetIndirectFragmentOffset(nextOffset);
mDataPollHandler.HandleNewFrame(aChild);
#if OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
mCslTxScheduler.Update();
#endif
Expand Down
Loading

0 comments on commit 35cc162

Please sign in to comment.