Union pointers to save space in `CommandSender` (#31609)
* Union pointers to save space in CommandSender
* Restyled by clang-format
---------
Co-authored-by: Restyled.io <commits@restyled.io>
diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp
index 6ab19b2..1066ca9 100644
--- a/src/app/CommandSender.cpp
+++ b/src/app/CommandSender.cpp
@@ -63,7 +63,7 @@
CommandSender::CommandSender(Callback * apCallback, Messaging::ExchangeManager * apExchangeMgr, bool aIsTimedRequest,
bool aSuppressResponse) :
mExchangeCtx(*this),
- mpCallback(apCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse), mTimedRequest(aIsTimedRequest)
+ mCallbackHandle(apCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse), mTimedRequest(aIsTimedRequest)
{
assertChipStackLockedByCurrentThread();
}
@@ -71,8 +71,8 @@
CommandSender::CommandSender(ExtendableCallback * apExtendableCallback, Messaging::ExchangeManager * apExchangeMgr,
bool aIsTimedRequest, bool aSuppressResponse) :
mExchangeCtx(*this),
- mpExtendableCallback(apExtendableCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse),
- mTimedRequest(aIsTimedRequest)
+ mCallbackHandle(apExtendableCallback), mpExchangeMgr(apExchangeMgr), mSuppressResponse(aSuppressResponse),
+ mTimedRequest(aIsTimedRequest), mUseExtendableCallback(true)
{
assertChipStackLockedByCurrentThread();
}
@@ -86,12 +86,6 @@
{
if (!mBufferAllocated)
{
- // We are making sure that both callbacks are not set. This should never happen as the constructors
- // are strongly typed and only one should ever be set, but explicit check is here to ensure that is
- // always the case.
- bool bothCallbacksAreSet = mpExtendableCallback != nullptr && mpCallback != nullptr;
- VerifyOrDie(!bothCallbacksAreSet);
-
mCommandMessageWriter.Reset();
System::PacketBufferHandle commandPacket = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
@@ -417,8 +411,7 @@
// When using ExtendableCallbacks, we are adhering to a different API contract where path
// specific errors are sent to the OnResponse callback. For more information on the history
// of this issue please see https://github.com/project-chip/connectedhomeip/issues/30991
- bool usingExtendableCallbacks = mpExtendableCallback != nullptr;
- if (statusIB.IsSuccess() || usingExtendableCallbacks)
+ if (statusIB.IsSuccess() || mUseExtendableCallback)
{
const ConcreteCommandPath concretePath = ConcreteCommandPath(endpointId, clusterId, commandId);
ResponseData responseData = { concretePath, statusIB };
@@ -456,8 +449,7 @@
//
// We must not be in the middle of preparing a command, and must not have already sent InvokeRequestMessage.
//
- bool usingExtendableCallbacks = mpExtendableCallback != nullptr;
- bool canAddAnotherCommand = (mState == State::AddedCommand && mBatchCommandsEnabled && usingExtendableCallbacks);
+ bool canAddAnotherCommand = (mState == State::AddedCommand && mBatchCommandsEnabled && mUseExtendableCallback);
VerifyOrReturnError(mState == State::Idle || canAddAnotherCommand, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mFinishedCommandCount < mRemoteMaxPathsPerInvoke, CHIP_ERROR_MAXIMUM_PATHS_PER_INVOKE_EXCEEDED);
diff --git a/src/app/CommandSender.h b/src/app/CommandSender.h
index dfcd3e1..41ec5be 100644
--- a/src/app/CommandSender.h
+++ b/src/app/CommandSender.h
@@ -455,7 +455,7 @@
private:
friend class TestCommandInteraction;
- enum class State
+ enum class State : uint8_t
{
Idle, ///< Default state that the object starts out in, where no work has commenced
AddingCommand, ///< In the process of adding a command.
@@ -466,6 +466,14 @@
AwaitingDestruction, ///< The object has completed its work and is awaiting destruction by the application.
};
+ union CallbackHandle
+ {
+ CallbackHandle(Callback * apCallback) : legacyCallback(apCallback) {}
+ CallbackHandle(ExtendableCallback * apExtendableCallback) : extendableCallback(apExtendableCallback) {}
+ Callback * legacyCallback;
+ ExtendableCallback * extendableCallback;
+ };
+
void MoveToState(const State aTargetState);
const char * GetStateStr() const;
@@ -513,46 +521,45 @@
void OnResponseCallback(const ResponseData & aResponseData)
{
// mpExtendableCallback and mpCallback are mutually exclusive.
- if (mpExtendableCallback)
+ if (mUseExtendableCallback && mCallbackHandle.extendableCallback)
{
- mpExtendableCallback->OnResponse(this, aResponseData);
+ mCallbackHandle.extendableCallback->OnResponse(this, aResponseData);
}
- else if (mpCallback)
+ else if (mCallbackHandle.legacyCallback)
{
- mpCallback->OnResponse(this, aResponseData.path, aResponseData.statusIB, aResponseData.data);
+ mCallbackHandle.legacyCallback->OnResponse(this, aResponseData.path, aResponseData.statusIB, aResponseData.data);
}
}
void OnErrorCallback(CHIP_ERROR aError)
{
// mpExtendableCallback and mpCallback are mutually exclusive.
- if (mpExtendableCallback)
+ if (mUseExtendableCallback && mCallbackHandle.extendableCallback)
{
ErrorData errorData = { aError };
- mpExtendableCallback->OnError(this, errorData);
+ mCallbackHandle.extendableCallback->OnError(this, errorData);
}
- else if (mpCallback)
+ else if (mCallbackHandle.legacyCallback)
{
- mpCallback->OnError(this, aError);
+ mCallbackHandle.legacyCallback->OnError(this, aError);
}
}
void OnDoneCallback()
{
// mpExtendableCallback and mpCallback are mutually exclusive.
- if (mpExtendableCallback)
+ if (mUseExtendableCallback && mCallbackHandle.extendableCallback)
{
- mpExtendableCallback->OnDone(this);
+ mCallbackHandle.extendableCallback->OnDone(this);
}
- else if (mpCallback)
+ else if (mCallbackHandle.legacyCallback)
{
- mpCallback->OnDone(this);
+ mCallbackHandle.legacyCallback->OnDone(this);
}
}
Messaging::ExchangeHolder mExchangeCtx;
- Callback * mpCallback = nullptr;
- ExtendableCallback * mpExtendableCallback = nullptr;
+ CallbackHandle mCallbackHandle;
Messaging::ExchangeManager * mpExchangeMgr = nullptr;
InvokeRequestMessage::Builder mInvokeRequestBuilder;
// TODO Maybe we should change PacketBufferTLVWriter so we can finalize it
@@ -564,15 +571,16 @@
Optional<uint16_t> mTimedInvokeTimeoutMs;
TLV::TLVType mDataElementContainerType = TLV::kTLVType_NotSpecified;
- State mState = State::Idle;
chip::System::PacketBufferTLVWriter mCommandMessageWriter;
uint16_t mFinishedCommandCount = 0;
uint16_t mRemoteMaxPathsPerInvoke = 1;
- bool mSuppressResponse = false;
- bool mTimedRequest = false;
- bool mBufferAllocated = false;
- bool mBatchCommandsEnabled = false;
+ State mState = State::Idle;
+ bool mSuppressResponse = false;
+ bool mTimedRequest = false;
+ bool mBufferAllocated = false;
+ bool mBatchCommandsEnabled = false;
+ bool mUseExtendableCallback = false;
};
} // namespace app
