Skip to content

Commit

Permalink
Rewrite cluster callback implementations to not have per-field argume…
Browse files Browse the repository at this point in the history
…nts.

This commit was generated by running:

  find src/app/clusters examples/tv-app/linux/include/content-launcher examples/window-app/common/src -name "*.cpp" | xargs perl -pi -e 'BEGIN { $/ = undef; } s/(bool emberAf[a-zA-Z0-9]*Cluster[a-zA-Z0-9]*Callback\([^,]+,[^,]+,)[^,]+,([^,)]+ commandData\)\n{\n)/\1 const \2/g;'

  find src/app/clusters examples/tv-app/linux/include/content-launcher examples/window-app/common/src -name "*.cpp" | xargs perl -pi -e 'BEGIN { $/ = undef; } s/(bool emberAf[a-zA-Z0-9]*Cluster[a-zA-Z0-9]*Callback\([^,]+,[^,]+,)[^,)]+,[^,)]+ ([a-zA-Z0-9]+),([^,)]+ commandData\)\n{\n)/\1 const \3    auto & \2 = commandData.\2;\n\n/g;'

  find src/app/clusters examples/tv-app/linux/include/content-launcher examples/window-app/common/src -name "*.cpp" | xargs perl -pi -e 'BEGIN { $/ = undef; } s/(bool emberAf[a-zA-Z0-9]*Cluster[a-zA-Z0-9]*Callback\([^,]+,[^,]+,)[^,)]+,[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),([^,)]+ commandData\)\n{\n)/\1 const \4    auto & \2 = commandData.\2;\n    auto & \3 = commandData.\3;\n\n/g;'

  find src/app/clusters examples/tv-app/linux/include/content-launcher examples/window-app/common/src -name "*.cpp" | xargs perl -pi -e 'BEGIN { $/ = undef; } s/(bool emberAf[a-zA-Z0-9]*Cluster[a-zA-Z0-9]*Callback\([^,]+,[^,]+,)[^,)]+,[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),([^,)]+ commandData\)\n{\n)/\1 const \5    auto & \2 = commandData.\2;\n    auto & \3 = commandData.\3;\n    auto & \4 = commandData.\4;\n\n/g;'

  find src/app/clusters examples/tv-app/linux/include/content-launcher examples/window-app/common/src -name "*.cpp" | xargs perl -pi -e 'BEGIN { $/ = undef; } s/(bool emberAf[a-zA-Z0-9]*Cluster[a-zA-Z0-9]*Callback\([^,]+,[^,]+,)[^,)]+,[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),([^,)]+ commandData\)\n{\n)/\1 const \6    auto & \2 = commandData.\2;\n    auto & \3 = commandData.\3;\n    auto & \4 = commandData.\4;\n    auto & \5 = commandData.\5;\n\n/g;'

  find src/app/clusters examples/tv-app/linux/include/content-launcher examples/window-app/common/src -name "*.cpp" | xargs perl -pi -e 'BEGIN { $/ = undef; } s/(bool emberAf[a-zA-Z0-9]*Cluster[a-zA-Z0-9]*Callback\([^,]+,[^,]+,)[^,)]+,[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),([^,)]+ commandData\)\n{\n)/\1 const \7    auto & \2 = commandData.\2;\n    auto & \3 = commandData.\3;\n    auto & \4 = commandData.\4;\n    auto & \5 = commandData.\5;\n    auto & \6 = commandData.\6;\n\n/g;'

  find src/app/clusters examples/tv-app/linux/include/content-launcher examples/window-app/common/src -name "*.cpp" | xargs perl -pi -e 'BEGIN { $/ = undef; } s/(bool emberAf[a-zA-Z0-9]*Cluster[a-zA-Z0-9]*Callback\([^,]+,[^,]+,)[^,)]+,[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),([^,)]+ commandData\)\n{\n)/\1 const \8    auto & \2 = commandData.\2;\n    auto & \3 = commandData.\3;\n    auto & \4 = commandData.\4;\n    auto & \5 = commandData.\5;\n    auto & \6 = commandData.\6;\n    auto & \7 = commandData.\7;\n\n/g;'

  find src/app/clusters examples/tv-app/linux/include/content-launcher examples/window-app/common/src -name "*.cpp" | xargs perl -pi -e 'BEGIN { $/ = undef; } s/(bool emberAf[a-zA-Z0-9]*Cluster[a-zA-Z0-9]*Callback\([^,]+,[^,]+,)[^,)]+,[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),[^,)]+ ([a-zA-Z0-9]+),([^,)]+ commandData\)\n{\n)/\1 const \9    auto & \2 = commandData.\2;\n    auto & \3 = commandData.\3;\n    auto & \4 = commandData.\4;\n    auto & \5 = commandData.\5;\n    auto & \6 = commandData.\6;\n    auto & \7 = commandData.\7;\n    auto & \8 = commandData.\8;\n\n/g;'

and then restyling.

For all of those regexps the idea is to grab the first two arguments
(command obj and command path) as-is, as well as the last
(commandData) argument.  The endpoint argument is dropped, because
almost nothing uses it right now and it's easier to fix up the things
that do (to use the command path) than to fix up all the things that
don't (to not have it).  For the other arguments, each one is
rewritten into a variable at the top of the function, assigned from
the corresponding member of the field struct.  There are multiple
passes because each pass only deals with functions that have a fixed
number of arguments.

Fixes project-chip#8974
Fixes project-chip#8977
  • Loading branch information
bzbarsky-apple committed Oct 8, 2021
1 parent b024144 commit c09a94c
Show file tree
Hide file tree
Showing 37 changed files with 642 additions and 359 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,11 @@ static void sendResponse(const char * responseName, ContentLaunchResponse launch
}

bool emberAfContentLauncherClusterLaunchContentCallback(
chip::app::CommandHandler * command, const chip::app::ConcreteCommandPath & commandPath, chip::EndpointId endpoint,
bool autoplay, unsigned char * data, chip::app::Clusters::ContentLauncher::Commands::LaunchContent::DecodableType & commandData)
chip::app::CommandHandler * command, const chip::app::ConcreteCommandPath & commandPath,
const chip::app::Clusters::ContentLauncher::Commands::LaunchContent::DecodableType & commandData)
{
auto & autoplay = commandData.autoplay;
auto & data = commandData.data;

string dataString(reinterpret_cast<char *>(data));
list<ContentLaunchParamater> parameterList;
Expand All @@ -118,10 +120,12 @@ bool emberAfContentLauncherClusterLaunchContentCallback(
}

bool emberAfContentLauncherClusterLaunchURLCallback(
chip::app::CommandHandler * command, const chip::app::ConcreteCommandPath & commandPath, chip::EndpointId endpoint,
unsigned char * contentUrl, unsigned char * displayString,
chip::app::Clusters::ContentLauncher::Commands::LaunchURL::DecodableType & commandData)
chip::app::CommandHandler * command, const chip::app::ConcreteCommandPath & commandPath,
const chip::app::Clusters::ContentLauncher::Commands::LaunchURL::DecodableType & commandData)
{
auto & contentUrl = commandData.contentUrl;
auto & displayString = commandData.displayString;

string contentUrlString(reinterpret_cast<char *>(contentUrl));
string displayStringString(reinterpret_cast<char *>(displayString));
ContentLaunchBrandingInformation brandingInformation;
Expand Down
4 changes: 2 additions & 2 deletions examples/window-app/common/src/ZclCallbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ void emberAfPostAttributeChangeCallback(chip::EndpointId endpoint, chip::Cluster
* @brief Cluster StopMotion Command callback (from client)
*/
bool emberAfWindowCoveringClusterStopMotionCallback(chip::app::CommandHandler * commandObj,
const chip::app::ConcreteCommandPath & commandPath, chip::EndpointId endpoint,
Commands::StopMotion::DecodableType & commandData)
const chip::app::ConcreteCommandPath & commandPath,
const Commands::StopMotion::DecodableType & commandData)
{
ChipLogProgress(Zcl, "StopMotion command received");
WindowApp::Instance().PostEvent(WindowApp::Event(WindowApp::EventId::StopMotion, endpoint));
Expand Down
11 changes: 7 additions & 4 deletions src/app/clusters/account-login-server/account-login-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ void sendResponse(app::CommandHandler * command, const char * responseSetupPin)
}

bool emberAfAccountLoginClusterGetSetupPINCallback(app::CommandHandler * command, const app::ConcreteCommandPath & commandPath,
EndpointId endpoint, uint8_t * tempAccountIdentifier,
Commands::GetSetupPIN::DecodableType & commandData)
const Commands::GetSetupPIN::DecodableType & commandData)
{
auto & tempAccountIdentifier = commandData.tempAccountIdentifier;

// TODO: char is not null terminated, verify this code once #7963 gets merged.
std::string tempAccountIdentifierString(reinterpret_cast<char *>(tempAccountIdentifier));
std::string responseSetupPin = accountLoginClusterGetSetupPin(tempAccountIdentifierString, emberAfCurrentEndpoint());
Expand All @@ -64,9 +65,11 @@ bool emberAfAccountLoginClusterGetSetupPINCallback(app::CommandHandler * command
}

bool emberAfAccountLoginClusterLoginCallback(app::CommandHandler * command, const app::ConcreteCommandPath & commandPath,
EndpointId endpoint, uint8_t * tempAccountIdentifier, uint8_t * tempSetupPin,
Commands::Login::DecodableType & commandData)
const Commands::Login::DecodableType & commandData)
{
auto & tempAccountIdentifier = commandData.tempAccountIdentifier;
auto & tempSetupPin = commandData.tempSetupPin;

// TODO: char is not null terminated, verify this code once #7963 gets merged.
std::string tempAccountIdentifierString(reinterpret_cast<char *>(tempAccountIdentifier));
std::string tempSetupPinString(reinterpret_cast<char *>(tempSetupPin));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,16 @@ using namespace chip::app::Clusters::AdministratorCommissioning;
constexpr uint32_t kMaxCommissionioningTimeoutSeconds = 15 * 60;

bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, EndpointId endpoint,
uint16_t commissioningTimeout, ByteSpan pakeVerifier, uint16_t discriminator, uint32_t iterations, ByteSpan salt,
uint16_t passcodeID, Commands::OpenCommissioningWindow::DecodableType & commandData)
app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
const Commands::OpenCommissioningWindow::DecodableType & commandData)
{
auto & commissioningTimeout = commandData.commissioningTimeout;
auto & pakeVerifier = commandData.pakeVerifier;
auto & discriminator = commandData.discriminator;
auto & iterations = commandData.iterations;
auto & salt = commandData.salt;
auto & passcodeID = commandData.passcodeID;

EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;
PASEVerifier verifier;
const uint8_t * verifierData = pakeVerifier.data();
Expand Down Expand Up @@ -74,9 +80,11 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback(
}

bool emberAfAdministratorCommissioningClusterOpenBasicCommissioningWindowCallback(
app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath, EndpointId endpoint,
uint16_t commissioningTimeout, Commands::OpenBasicCommissioningWindow::DecodableType & commandData)
app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
const Commands::OpenBasicCommissioningWindow::DecodableType & commandData)
{
auto & commissioningTimeout = commandData.commissioningTimeout;

EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;
ChipLogProgress(Zcl, "Received command to open basic commissioning window");
VerifyOrExit(!Server::GetInstance().GetCommissioningWindowManager().IsCommissioningWindowOpen(),
Expand All @@ -96,10 +104,9 @@ bool emberAfAdministratorCommissioningClusterOpenBasicCommissioningWindowCallbac
return true;
}

bool emberAfAdministratorCommissioningClusterRevokeCommissioningCallback(app::CommandHandler * commandObj,
const app::ConcreteCommandPath & commandPath,
EndpointId endpoint,
Commands::RevokeCommissioning::DecodableType & commandData)
bool emberAfAdministratorCommissioningClusterRevokeCommissioningCallback(
app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
const Commands::RevokeCommissioning::DecodableType & commandData)
{
ChipLogProgress(Zcl, "Received command to close commissioning window");
Server::GetInstance().GetCommissioningWindowManager().CloseCommissioningWindow();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ using namespace chip::app::Clusters::ApplicationBasic;
bool applicationBasicClusterChangeApplicationStatus(EmberAfApplicationBasicStatus status, EndpointId endpoint);

bool emberAfApplicationBasicClusterChangeStatusCallback(app::CommandHandler * commandObj,
const app::ConcreteCommandPath & commandPath, EndpointId endpoint,
uint8_t newApplicationStatus,
Commands::ChangeStatus::DecodableType & commandData)
const app::ConcreteCommandPath & commandPath,
const Commands::ChangeStatus::DecodableType & commandData)
{
auto & newApplicationStatus = commandData.newApplicationStatus;

bool success = applicationBasicClusterChangeApplicationStatus(static_cast<EmberAfApplicationBasicStatus>(newApplicationStatus),
emberAfCurrentEndpoint());
EmberAfStatus status = success ? EMBER_ZCL_STATUS_SUCCESS : EMBER_ZCL_STATUS_FAILURE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,12 @@ ::ApplicationLauncherApp getApplicationFromCommand(uint16_t catalogVendorId, uin
}

bool emberAfApplicationLauncherClusterLaunchAppCallback(app::CommandHandler * command, const app::ConcreteCommandPath & commandPath,
EndpointId endpoint, uint8_t * requestData,
uint16_t requestApplicationCatalogVendorId, uint8_t * requestApplicationId,
Commands::LaunchApp::DecodableType & commandData)
const Commands::LaunchApp::DecodableType & commandData)
{
auto & requestData = commandData.requestData;
auto & requestApplicationCatalogVendorId = commandData.requestApplicationCatalogVendorId;
auto & requestApplicationId = commandData.requestApplicationId;

::ApplicationLauncherApp application = getApplicationFromCommand(requestApplicationCatalogVendorId, requestApplicationId);
// TODO: Char is not null terminated, verify this code once #7963 gets merged.
std::string reqestDataString(reinterpret_cast<char *>(requestData));
Expand Down
11 changes: 7 additions & 4 deletions src/app/clusters/audio-output-server/audio-output-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,22 @@ bool audioOutputClusterSelectOutput(uint8_t index);
bool audioOutputClusterRenameOutput(uint8_t index, uint8_t * name);

bool emberAfAudioOutputClusterRenameOutputCallback(app::CommandHandler * command, const app::ConcreteCommandPath & commandPath,
EndpointId endpoint, uint8_t index, uint8_t * name,
Commands::RenameOutput::DecodableType & commandData)
const Commands::RenameOutput::DecodableType & commandData)
{
auto & index = commandData.index;
auto & name = commandData.name;

bool success = audioOutputClusterRenameOutput(index, name);
EmberAfStatus status = success ? EMBER_ZCL_STATUS_SUCCESS : EMBER_ZCL_STATUS_FAILURE;
emberAfSendImmediateDefaultResponse(status);
return true;
}

bool emberAfAudioOutputClusterSelectOutputCallback(app::CommandHandler * command, const app::ConcreteCommandPath & commandPath,
EndpointId endpoint, uint8_t index,
Commands::SelectOutput::DecodableType & commandData)
const Commands::SelectOutput::DecodableType & commandData)
{
auto & index = commandData.index;

bool success = audioOutputClusterSelectOutput(index);
EmberAfStatus status = success ? EMBER_ZCL_STATUS_SUCCESS : EMBER_ZCL_STATUS_FAILURE;
emberAfSendImmediateDefaultResponse(status);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,11 +299,12 @@ static void sendDefaultResponse(EmberAfStatus status)
}
}

bool emberAfBarrierControlClusterBarrierControlGoToPercentCallback(app::CommandHandler * commandObj,
const app::ConcreteCommandPath & commandPath,
EndpointId aEndpoint, uint8_t percentOpen,
Commands::BarrierControlGoToPercent::DecodableType & commandData)
bool emberAfBarrierControlClusterBarrierControlGoToPercentCallback(
app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
const Commands::BarrierControlGoToPercent::DecodableType & commandData)
{
auto & percentOpen = commandData.percentOpen;

EndpointId endpoint = emberAfCurrentCommand()->apsFrame->destinationEndpoint;
EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;

Expand Down Expand Up @@ -345,8 +346,8 @@ bool emberAfBarrierControlClusterBarrierControlGoToPercentCallback(app::CommandH
}

bool emberAfBarrierControlClusterBarrierControlStopCallback(app::CommandHandler * commandObj,
const app::ConcreteCommandPath & commandPath, EndpointId aEndpoint,
Commands::BarrierControlStop::DecodableType & commandData)
const app::ConcreteCommandPath & commandPath,
const Commands::BarrierControlStop::DecodableType & commandData)
{
EndpointId endpoint = emberAfCurrentCommand()->apsFrame->destinationEndpoint;
emberAfDeactivateServerTick(endpoint, BarrierControl::Id);
Expand Down
16 changes: 12 additions & 4 deletions src/app/clusters/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,13 @@ EmberStatus getUnusedBindingIndex(uint8_t * bindingIndex)
}

bool emberAfBindingClusterBindCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
EndpointId endpoint, NodeId nodeId, GroupId groupId, EndpointId endpointId,
ClusterId clusterId, Commands::Bind::DecodableType & commandData)
const Commands::Bind::DecodableType & commandData)
{
auto & nodeId = commandData.nodeId;
auto & groupId = commandData.groupId;
auto & endpointId = commandData.endpointId;
auto & clusterId = commandData.clusterId;

ChipLogDetail(Zcl, "RX: BindCallback");

EmberBindingTableEntry bindingEntry;
Expand Down Expand Up @@ -123,9 +127,13 @@ bool emberAfBindingClusterBindCallback(app::CommandHandler * commandObj, const a
}

bool emberAfBindingClusterUnbindCallback(app::CommandHandler * commandObj, const app::ConcreteCommandPath & commandPath,
EndpointId endpoint, NodeId nodeId, GroupId groupId, EndpointId endpointId,
ClusterId clusterId, Commands::Unbind::DecodableType & commandData)
const Commands::Unbind::DecodableType & commandData)
{
auto & nodeId = commandData.nodeId;
auto & groupId = commandData.groupId;
auto & endpointId = commandData.endpointId;
auto & clusterId = commandData.clusterId;

ChipLogDetail(Zcl, "RX: UnbindCallback");

EmberBindingTableEntry bindingEntry;
Expand Down
Loading

0 comments on commit c09a94c

Please sign in to comment.