Skip to content

Commit

Permalink
Move command handler registered list out of InteractionModelEngine an…
Browse files Browse the repository at this point in the history
…d into a separate `Registry` (#34414)

* Starting to define a commandhandlerregistry

* Add missing files and replace some deprecated files usage

* Replace all usages of IME to the command handler registry direct calls

* Restyle

* Fix wrong comment paste

* Fix include

* Remove unused include

* Fix linter error

* Update src/app/util/attribute-storage.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/controller/tests/TestServerCommandDispatch.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/controller/tests/TestServerCommandDispatch.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

---------

Co-authored-by: Andrei Litvin <andreilitvin@google.com>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
  • Loading branch information
3 people authored and pull[bot] committed Oct 11, 2024
1 parent 801ce7a commit ef923b1
Show file tree
Hide file tree
Showing 27 changed files with 266 additions and 155 deletions.
1 change: 0 additions & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ jobs:
--known-failure controller/ExamplePersistentStorage.cpp \
--known-failure controller/ExamplePersistentStorage.h \
--known-failure app/AttributeAccessToken.h \
--known-failure app/CommandHandlerInterface.h \
--known-failure app/CommandResponseSender.h \
--known-failure app/CommandSenderLegacyCallback.h \
--known-failure app/ReadHandler.h \
Expand Down
16 changes: 16 additions & 0 deletions docs/upgrading.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,19 @@ commandHandler->AddResponse(path, kReplyCommandId, replyEncoder);
// so code does AddResponse rather than AddResponseData.
```

### `CommandHandlerInterface` in `chip::app::InteractionModelEngine`

Command handler lists were placed in a separate registry class that is
independent of the InteractionModelEngine class.

The following replacements exist:

- `chip::app::InteractionModelEngine::RegisterCommandHandler` replaced by
`chip::app::CommandHandlerInterfaceRegistry::RegisterCommandHandler`
- `chip::app::InteractionModelEngine::UnregisterCommandHandler` replaced by
`chip::app::CommandHandlerInterfaceRegistry::UnregisterCommandHandler`
- `chip::app::InteractionModelEngine::FindCommandHandler` replaced by
`chip::app::CommandHandlerInterfaceRegistry::GetCommandHandler`
- `chip::app::InteractionModelEngine::UnregisterCommandHandlers` replaced by
`chip::app::CommandHandlerInterfaceRegistry::UnregisterAllCommandHandlersForEndpoint`
3 changes: 2 additions & 1 deletion examples/fabric-bridge-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "DeviceManager.h"

#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/CommandHandlerInterfaceRegistry.h>

#if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE
#include "RpcClient.h"
Expand Down Expand Up @@ -173,7 +174,7 @@ void ApplicationInit()
{
ChipLogDetail(NotSpecified, "Fabric-Bridge: ApplicationInit()");

InteractionModelEngine::GetInstance()->RegisterCommandHandler(&gAdministratorCommissioningCommandHandler);
CommandHandlerInterfaceRegistry::RegisterCommandHandler(&gAdministratorCommissioningCommandHandler);

#if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE
InitRpcServer(kFabricBridgeServerPort);
Expand Down
3 changes: 2 additions & 1 deletion examples/log-source-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <platform/CHIPDeviceLayer.h>
#include <platform/PlatformManager.h>

#include <app/CommandHandlerInterfaceRegistry.h>
#include <app/clusters/diagnostic-logs-server/diagnostic-logs-server.h>
#include <app/server/Server.h>
#include <app/util/util.h>
Expand Down Expand Up @@ -109,7 +110,7 @@ int main(int argc, char * argv[])
// Initialize device attestation config
SetDeviceAttestationCredentialsProvider(chip::Credentials::Examples::GetExampleDACProvider());

chip::app::InteractionModelEngine::GetInstance()->RegisterCommandHandler(&GetLogProvider());
CommandHandlerInterfaceRegistry::RegisterCommandHandler(&GetLogProvider());

chip::DeviceLayer::PlatformMgr().RunEventLoop();

Expand Down
4 changes: 2 additions & 2 deletions examples/tv-app/android/java/AppImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include <app-common/zap-generated/ids/Attributes.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/CommandHandler.h>
#include <app/InteractionModelEngine.h>
#include <app/CommandHandlerInterfaceRegistry.h>
#include <app/reporting/reporting.h>
#include <app/server/Dnssd.h>
#include <app/server/Server.h>
Expand Down Expand Up @@ -572,7 +572,7 @@ CHIP_ERROR InitVideoPlayerPlatform(jobject contentAppEndpointManager)
{
ContentAppCommandDelegate * delegate =
new ContentAppCommandDelegate(contentAppEndpointManager, contentAppClusters[i].clusterId);
chip::app::InteractionModelEngine::GetInstance()->RegisterCommandHandler(delegate);
chip::app::CommandHandlerInterfaceRegistry::RegisterCommandHandler(delegate);
ChipLogProgress(AppServer, "Registered command handler delegate for cluster %d", contentAppClusters[i].clusterId);
}

Expand Down
3 changes: 3 additions & 0 deletions src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,9 @@ source_set("command-handler-interface") {
"CommandHandler.cpp",
"CommandHandler.h",
"CommandHandlerExchangeInterface.h",
"CommandHandlerInterface.h",
"CommandHandlerInterfaceRegistry.cpp",
"CommandHandlerInterfaceRegistry.h",
]

public_deps = [
Expand Down
2 changes: 0 additions & 2 deletions src/app/CommandHandlerInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@
#include <app/ConcreteCommandPath.h>
#include <app/data-model/Decode.h>
#include <app/data-model/List.h> // So we can encode lists
#include <functional>
#include <lib/core/DataModelTypes.h>
#include <lib/support/Iterators.h>
#include <type_traits>

namespace chip {
namespace app {
Expand Down
139 changes: 139 additions & 0 deletions src/app/CommandHandlerInterfaceRegistry.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/**
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <app/CommandHandlerInterfaceRegistry.h>

using namespace chip::app;

namespace {

CommandHandlerInterface * gCommandHandlerList = nullptr;

}

namespace chip {
namespace app {
namespace CommandHandlerInterfaceRegistry {

void UnregisterAllHandlers()
{

CommandHandlerInterface * handlerIter = gCommandHandlerList;

//
// Walk our list of command handlers and de-register them, before finally
// nulling out the list entirely.
//
while (handlerIter)
{
CommandHandlerInterface * nextHandler = handlerIter->GetNext();
handlerIter->SetNext(nullptr);
handlerIter = nextHandler;
}

gCommandHandlerList = nullptr;
}

CHIP_ERROR RegisterCommandHandler(CommandHandlerInterface * handler)
{
VerifyOrReturnError(handler != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

for (auto * cur = gCommandHandlerList; cur; cur = cur->GetNext())
{
if (cur->Matches(*handler))
{
ChipLogError(InteractionModel, "Duplicate command handler registration failed");
return CHIP_ERROR_INCORRECT_STATE;
}
}

handler->SetNext(gCommandHandlerList);
gCommandHandlerList = handler;

return CHIP_NO_ERROR;
}

void UnregisterAllCommandHandlersForEndpoint(EndpointId endpointId)
{

CommandHandlerInterface * prev = nullptr;

for (auto * cur = gCommandHandlerList; cur; cur = cur->GetNext())
{
if (cur->MatchesEndpoint(endpointId))
{
if (prev == nullptr)
{
gCommandHandlerList = cur->GetNext();
}
else
{
prev->SetNext(cur->GetNext());
}

cur->SetNext(nullptr);
}
else
{
prev = cur;
}
}
}

CHIP_ERROR UnregisterCommandHandler(CommandHandlerInterface * handler)
{
VerifyOrReturnError(handler != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
CommandHandlerInterface * prev = nullptr;

for (auto * cur = gCommandHandlerList; cur; cur = cur->GetNext())
{
if (cur->Matches(*handler))
{
if (prev == nullptr)
{
gCommandHandlerList = cur->GetNext();
}
else
{
prev->SetNext(cur->GetNext());
}

cur->SetNext(nullptr);

return CHIP_NO_ERROR;
}

prev = cur;
}

return CHIP_ERROR_KEY_NOT_FOUND;
}

CommandHandlerInterface * GetCommandHandler(EndpointId endpointId, ClusterId clusterId)
{
for (auto * cur = gCommandHandlerList; cur; cur = cur->GetNext())
{
if (cur->Matches(endpointId, clusterId))
{
return cur;
}
}

return nullptr;
}

} // namespace CommandHandlerInterfaceRegistry
} // namespace app
} // namespace chip
47 changes: 47 additions & 0 deletions src/app/CommandHandlerInterfaceRegistry.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#pragma once

#include <app/CommandHandlerInterface.h>

namespace chip {
namespace app {
namespace CommandHandlerInterfaceRegistry {

/// Remove the entire linked list of handlers
void UnregisterAllHandlers();

/// Add a new handler to the list of registered command handlers
///
/// At most one command handler can exist for a given endpoint/cluster combination. Trying
/// to register conflicting handlers will result in a `CHIP_ERROR_INCORRECT_STATE` error.
CHIP_ERROR RegisterCommandHandler(CommandHandlerInterface * handler);

/// Unregister all commandHandlers that `MatchesEndpoint` for the given endpointId.
void UnregisterAllCommandHandlersForEndpoint(EndpointId endpointId);

/// Unregister a single handler.
///
/// If the handler is not registered, a `CHIP_ERROR_KEY_NOT_FOUND` is returned.
CHIP_ERROR UnregisterCommandHandler(CommandHandlerInterface * handler);

/// Find the command handler for the given endpoint/cluster combination or return
/// nullptr if no such command handler exists.
CommandHandlerInterface * GetCommandHandler(EndpointId endpointId, ClusterId clusterId);

} // namespace CommandHandlerInterfaceRegistry
} // namespace app
} // namespace chip
Loading

0 comments on commit ef923b1

Please sign in to comment.