Skip to content

Commit 16d6475

Browse files
authored
Minimize API surface that is templated for CommandHandler (#33620)
* Define a EncodeToTLV inteface for the command handler * Slight documentation update * Comments update
1 parent 6b83689 commit 16d6475

File tree

1 file changed

+70
-22
lines changed

1 file changed

+70
-22
lines changed

src/app/CommandHandler.h

+70-22
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,32 @@
5757
namespace chip {
5858
namespace app {
5959

60+
/// Defines an abstract class of something that can be encoded
61+
/// into a TLV with a given data tag
62+
class EncoderToTLV
63+
{
64+
public:
65+
virtual ~EncoderToTLV() = default;
66+
67+
virtual CHIP_ERROR Encode(TLV::TLVWriter &, TLV::Tag tag) = 0;
68+
};
69+
70+
/// An `EncoderToTLV` the uses `DataModel::Encode` to encode things.
71+
///
72+
/// Generally useful to encode things like <ClusterName>::Commands::<CommandName>::Type
73+
/// structures.
74+
template <typename T>
75+
class DataModelEncoderToTLV : public EncoderToTLV
76+
{
77+
public:
78+
DataModelEncoderToTLV(const T & value) : mValue(value) {}
79+
80+
virtual CHIP_ERROR Encode(TLV::TLVWriter & writer, TLV::Tag tag) { return DataModel::Encode(writer, tag, mValue); }
81+
82+
private:
83+
const T & mValue;
84+
};
85+
6086
class CommandHandler
6187
{
6288
public:
@@ -325,14 +351,37 @@ class CommandHandler
325351
* @param [in] aRequestCommandPath the concrete path of the command we are
326352
* responding to.
327353
* @param [in] aData the data for the response.
354+
*
355+
* NOTE: this is a convenience function for `AddResponseDataViaEncoder`
328356
*/
329357
template <typename CommandData>
330-
CHIP_ERROR AddResponseData(const ConcreteCommandPath & aRequestCommandPath, const CommandData & aData)
358+
inline CHIP_ERROR AddResponseData(const ConcreteCommandPath & aRequestCommandPath, const CommandData & aData)
359+
{
360+
DataModelEncoderToTLV<CommandData> encoder(aData);
361+
return AddResponseDataViaEncoder(aRequestCommandPath, CommandData::GetCommandId(), encoder);
362+
}
363+
364+
/**
365+
* API for adding a data response. The encoded is generally expected to encode
366+
* a ClusterName::Commands::CommandName::Type struct, but any
367+
* object should work.
368+
*
369+
* @param [in] aRequestCommandPath the concrete path of the command we are
370+
* responding to.
371+
* @param [in] commandId the command whose content is being encoded.
372+
* @param [in] encoder - an encoder that places the command data structure for `commandId`
373+
* into a TLV Writer.
374+
*
375+
* Most applications are likely to use `AddResponseData` as a more convenient
376+
* one-call that auto-sets command ID and creates the underlying encoders.
377+
*/
378+
CHIP_ERROR AddResponseDataViaEncoder(const ConcreteCommandPath & aRequestCommandPath, CommandId commandId,
379+
EncoderToTLV & encoder)
331380
{
332381
// Return early when response should not be sent out.
333382
VerifyOrReturnValue(ResponsesAccepted(), CHIP_NO_ERROR);
334-
335-
return TryAddingResponse([&]() -> CHIP_ERROR { return TryAddResponseData(aRequestCommandPath, aData); });
383+
return TryAddingResponse(
384+
[&]() -> CHIP_ERROR { return TryAddResponseDataViaEncoder(aRequestCommandPath, commandId, encoder); });
336385
}
337386

338387
/**
@@ -350,9 +399,21 @@ class CommandHandler
350399
* @param [in] aData the data for the response.
351400
*/
352401
template <typename CommandData>
353-
void AddResponse(const ConcreteCommandPath & aRequestCommandPath, const CommandData & aData)
402+
inline void AddResponse(const ConcreteCommandPath & aRequestCommandPath, const CommandData & aData)
354403
{
355-
if (AddResponseData(aRequestCommandPath, aData) != CHIP_NO_ERROR)
404+
DataModelEncoderToTLV<CommandData> encoder(aData);
405+
return AddResponseViaEncoder(aRequestCommandPath, CommandData::GetCommandId(), encoder);
406+
}
407+
408+
/**
409+
* API for adding a response with a given encoder of TLV data.
410+
*
411+
* The encoder would generally encode a ClusterName::Commands::CommandName::Type with
412+
* the corresponding `GetCommandId` call.
413+
*/
414+
void AddResponseViaEncoder(const ConcreteCommandPath & aRequestCommandPath, CommandId commandId, EncoderToTLV & encoder)
415+
{
416+
if (AddResponseDataViaEncoder(aRequestCommandPath, commandId, encoder) != CHIP_NO_ERROR)
356417
{
357418
AddStatus(aRequestCommandPath, Protocols::InteractionModel::Status::Failure);
358419
}
@@ -630,7 +691,6 @@ class CommandHandler
630691
return PrepareInvokeResponseCommand(aResponseCommandPath, prepareParams);
631692
}
632693

633-
// TODO(#31627): It would be awesome if we could remove this template all together.
634694
/**
635695
* If this function fails, it may leave our TLV buffer in an inconsistent state.
636696
* Callers should snapshot as needed before calling this function, and roll back
@@ -640,26 +700,14 @@ class CommandHandler
640700
* responding to.
641701
* @param [in] aData the data for the response.
642702
*/
643-
template <typename CommandData>
644-
CHIP_ERROR TryAddResponseData(const ConcreteCommandPath & aRequestCommandPath, const CommandData & aData)
703+
CHIP_ERROR TryAddResponseDataViaEncoder(const ConcreteCommandPath & aRequestCommandPath, CommandId commandId,
704+
EncoderToTLV & encoder)
645705
{
646-
// This method, templated with CommandData, captures all the components needs
647-
// from CommandData with as little code as possible.
648-
//
649-
// Previously, non-essential code was unnecessarily templated, leading to
650-
// compilation and duplication N times. By isolating only the code segments
651-
// that genuinely require templating, minimizes duplicate compiled code.
652-
ConcreteCommandPath responseCommandPath = { aRequestCommandPath.mEndpointId, aRequestCommandPath.mClusterId,
653-
CommandData::GetCommandId() };
706+
ConcreteCommandPath responseCommandPath = { aRequestCommandPath.mEndpointId, aRequestCommandPath.mClusterId, commandId };
654707
ReturnErrorOnFailure(TryAddResponseDataPreEncode(aRequestCommandPath, responseCommandPath));
655708
TLV::TLVWriter * writer = GetCommandDataIBTLVWriter();
656709
VerifyOrReturnError(writer != nullptr, CHIP_ERROR_INCORRECT_STATE);
657-
ReturnErrorOnFailure(DataModel::Encode(*writer, TLV::ContextTag(CommandDataIB::Tag::kFields), aData));
658-
659-
// FinishCommand technically should be refactored out as it is not a command that needs templating.
660-
// But, because there is only a single function call, keeping it here takes less code. If there is
661-
// ever more code between DataModel::Encode and the end of this function, it should be broken out into
662-
// TryAddResponseDataPostEncode.
710+
ReturnErrorOnFailure(encoder.Encode(*writer, TLV::ContextTag(CommandDataIB::Tag::kFields)));
663711
return FinishCommand(/* aEndDataStruct = */ false);
664712
}
665713

0 commit comments

Comments
 (0)