Make CommandHandler a pure interface class, have actual implementat…

…ion in `CommandHandlerImpl` (project-chip#33736)

* First pass: renames only and moved things around. It should not yet compile

* Move CommandHandler::Callback to CommandHandlerImpl::Callback

* Prepare some compile fixes. Uses of preparation of responses is not ok yet

* More replace fixes

* More compile updates

* Try to implement the addResponse properly

* Many more updates for compilation

* More compile tests - getting closer

* Things compile

* Restyle

* Split out CommandHandler (the interface) from CommandHandlerImpl (actual IM implementation)

* Restyle

* make GetExchangeContext also be virtual

* Fix some includes that were previously part of CommandHandler.h

* Restyle

* No need for casts and private APIs: Exchange context is actually available

* Code review feedback

* Restyle

* Upgrading docs

* Slight example fix

---------

Co-authored-by: Andrei Litvin <[email protected]>

docs/index.md

@@ -22,6 +22,7 @@ BUG_REPORT
 code_generation
 zap_clusters
 spec_clusters
+upgrading
 ERROR_CODES
 
 ```

docs/upgrading.md

@@ -0,0 +1,62 @@
+# Upgrading notes
+
+## API changes and code migration
+
+### `CommandHandler`
+
+`CommandHandler` ability to directly invoke `Prepare/TLV-Write/Finish` cycles
+has been changed to only expose `AddResponse/AddStatus/AddClusterSpecific*`.
+
+Original versions of `CommandHandler` exposed the following low-level
+implementation-specific methods: `PrepareCommand`,
+`PrepareInvokeResponseCommand`, `GetCommandDataIBTLVWriter` and `FinishCommand`.
+These are not exposed anymore and instead one should use `AddResponse` or
+`AddResponseData`. When using an `EncodableToTLV` argument, the same
+functionality should be achievable.
+
+Example
+
+Before:
+
+```cpp
+
+const CommandHandler::InvokeResponseParameters prepareParams(requestPath);
+ReturnOnFailure(commandHandler->PrepareInvokeResponseCommand(path, prepareParams));
+
+TLV::TLVWriter *writer = commandHandler->GetCommandDataIBTLVWriter();
+ReturnOnFailure(writer->Put(chip::TLV::ContextTag(1), 123));
+ReturnOnFailure(writer->Put(chip::TLV::ContextTag(2), 234));
+return commandHandler->FinishCommand();
+```
+
+After:
+
+```cpp
+
+class ReplyEncoder : public DataModel::EncodableToTLV
+{
+public:
+    CHIP_ERROR EncodeTo(TLV::TLVWriter & writer, TLV::Tag tag) const override
+    {
+        TLV::TLVType outerType;
+        ReturnErrorOnFailure(writer.StartContainer(tag, TLV::kTLVType_Structure, outerType));
+
+        ReturnOnFailure(writer.Put(chip::TLV::ContextTag(1), 123));
+        ReturnOnFailure(writer.Put(chip::TLV::ContextTag(2), 234));
+
+        return writer.EndContainer(outerType);
+    }
+};
+
+// ...
+ReplyEncoder replyEncoder;
+commandHandler->AddResponse(path, kReplyCommandId, replyEncoder);
+
+// or if error handling is implemented:
+//
+// ReturnErrorOnFailure(commandHandler->AddResponseData(path, kReplyCommandId, replyEncoder));
+//
+// In many cases error recovery from not being able to send a reply is not easy or expected,
+// so code does AddResponse rather than AddResponseData.
+
+```

examples/lighting-app/tizen/src/DBusInterface.cpp

@@ -22,6 +22,7 @@
 #include <app-common/zap-generated/cluster-enums.h>
 #include <app-common/zap-generated/ids/Attributes.h>
 #include <app-common/zap-generated/ids/Clusters.h>
+#include <app/CommandHandlerImpl.h>
 #include <app/ConcreteAttributePath.h>
 #include <app/clusters/color-control-server/color-control-server.h>
 #include <app/clusters/level-control/level-control.h>
@@ -36,13 +37,13 @@ using namespace chip::app;
 
 namespace example {
 
-// Dummy class to satisfy the CommandHandler::Callback interface.
-class CommandHandlerCallback : public CommandHandler::Callback
+// Dummy class to satisfy the CommandHandlerImpl::Callback interface.
+class CommandHandlerImplCallback : public CommandHandlerImpl::Callback
 {
 public:
     using Status = Protocols::InteractionModel::Status;
-    void OnDone(CommandHandler & apCommandObj) {}
-    void DispatchCommand(CommandHandler & apCommandObj, const ConcreteCommandPath & aCommandPath, TLV::TLVReader & apPayload) {}
+    void OnDone(CommandHandlerImpl & apCommandObj) {}
+    void DispatchCommand(CommandHandlerImpl & apCommandObj, const ConcreteCommandPath & aCommandPath, TLV::TLVReader & apPayload) {}
     Status CommandExists(const ConcreteCommandPath & aCommandPath) { return Status::Success; }
 };
 
@@ -188,8 +189,10 @@ gboolean DBusInterface::OnColorTemperatureChanged(LightAppColorControl * colorCo
     // Do not handle on-change event if it was triggered by internal set
     VerifyOrReturnValue(!self->mInternalSet, G_DBUS_METHOD_INVOCATION_HANDLED);
 
-    CommandHandlerCallback callback;
-    CommandHandler handler(&callback);
+    // TODO: creating such a complex object seems odd here
+    //       as handler seems not used to send back any response back anywhere.
+    CommandHandlerImplCallback callback;
+    CommandHandlerImpl handler(&callback);
 
     ConcreteCommandPath path{ self->mEndpointId, Clusters::ColorControl::Id, 0 };
 

src/app/BUILD.gn

@@ -338,6 +338,8 @@ source_set("command-handler") {
     "CommandHandler.cpp",
     "CommandHandler.h",
     "CommandHandlerExchangeInterface.h",
+    "CommandHandlerImpl.cpp",
+    "CommandHandlerImpl.h",
   ]
 
   public_deps = [