Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Stop passing individual field arguments to server cluster command cal…
Browse files Browse the repository at this point in the history
…lbacks (#10354)

* Update server-side command codegen to not output arguments for each field.

* Regenerate generated files

* Rewrite cluster callback implementations to not have per-field arguments.

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 #8974
Fixes #8977

* Manual fixups to deal with missing endpoint arguments.

* Manually remove declarations for arguments that are unused.

* Manually fix cases where the argument name did not match the struct member name.

In some of these cases the argument is actually unused so can just be
removed.

* Manual changes to deal with CHAR_STRING command fields.

The type of these has changed, and this actually fixes a bunch of bugs.

* Manual changes to make the scenes cluster compile for now.

It doesn't work right, because we don't have the right types for scene
field sets.

* Manual fixups for lists in groups cluster.

* Manual changes for const-correctness for door lock.

* Manual last fixups

* Restore the GetGroupMembership group id logging.

* Apply suggestions about endpoint id from code review
bzbarsky-apple authored and pull[bot] committed Oct 26, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 5ea3b52 commit 1623554
Showing 59 changed files with 1,939 additions and 2,726 deletions.
Original file line number Diff line number Diff line change
@@ -63,7 +63,7 @@ bool audioOutputClusterSelectOutput(uint8_t index)
// TODO: Insert code here
return true;
}
bool audioOutputClusterRenameOutput(uint8_t index, uint8_t * name)
bool audioOutputClusterRenameOutput(uint8_t index, const chip::CharSpan & name)
{
// TODO: Insert code here
return true;
Original file line number Diff line number Diff line change
@@ -106,24 +106,28 @@ 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));
string dataString(data.data(), data.size());
list<ContentLaunchParamater> parameterList;
ContentLaunchResponse response = ContentLauncherManager().proxyLaunchContentRequest(parameterList, autoplay, dataString);
sendResponse("LaunchContent", response, ZCL_LAUNCH_CONTENT_RESPONSE_COMMAND_ID);
return true;
}

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)
{
string contentUrlString(reinterpret_cast<char *>(contentUrl));
string displayStringString(reinterpret_cast<char *>(displayString));
auto & contentUrl = commandData.contentURL;
auto & displayString = commandData.displayString;

string contentUrlString(contentUrl.data(), contentUrl.size());
string displayStringString(displayString.data(), displayString.size());
ContentLaunchBrandingInformation brandingInformation;
ContentLaunchResponse response =
ContentLauncherManager().proxyLaunchUrlRequest(contentUrlString, displayStringString, brandingInformation);
6 changes: 3 additions & 3 deletions examples/window-app/common/src/ZclCallbacks.cpp
Original file line number Diff line number Diff line change
@@ -98,11 +98,11 @@ void MatterPostAttributeChangeCallback(const app::ConcreteAttributePath & attrib
* @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));
WindowApp::Instance().PostEvent(WindowApp::Event(WindowApp::EventId::StopMotion, commandPath.mEndpointId));
emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS);
return true;
}
19 changes: 10 additions & 9 deletions src/app/clusters/account-login-server/account-login-server.cpp
Original file line number Diff line number Diff line change
@@ -53,23 +53,24 @@ 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)
{
// TODO: char is not null terminated, verify this code once #7963 gets merged.
std::string tempAccountIdentifierString(reinterpret_cast<char *>(tempAccountIdentifier));
auto & tempAccountIdentifier = commandData.tempAccountIdentifier;

std::string tempAccountIdentifierString(tempAccountIdentifier.data(), tempAccountIdentifier.size());
std::string responseSetupPin = accountLoginClusterGetSetupPin(tempAccountIdentifierString, emberAfCurrentEndpoint());
sendResponse(command, responseSetupPin.c_str());
return true;
}

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)
{
// 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));
auto & tempAccountIdentifier = commandData.tempAccountIdentifier;
auto & tempSetupPin = commandData.setupPIN;

std::string tempAccountIdentifierString(tempAccountIdentifier.data(), tempAccountIdentifier.size());
std::string tempSetupPinString(tempSetupPin.data(), tempSetupPin.size());
bool isLoggedIn = accountLoginClusterIsUserLoggedIn(tempAccountIdentifierString, tempSetupPinString);
EmberAfStatus status = isLoggedIn ? EMBER_ZCL_STATUS_SUCCESS : EMBER_ZCL_STATUS_NOT_AUTHORIZED;
if (!isLoggedIn)
Original file line number Diff line number Diff line change
@@ -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();
@@ -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(),
@@ -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();
Original file line number Diff line number Diff line change
@@ -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.status;

bool success = applicationBasicClusterChangeApplicationStatus(static_cast<EmberAfApplicationBasicStatus>(newApplicationStatus),
emberAfCurrentEndpoint());
EmberAfStatus status = success ? EMBER_ZCL_STATUS_SUCCESS : EMBER_ZCL_STATUS_FAILURE;
Original file line number Diff line number Diff line change
@@ -64,22 +64,24 @@ void sendResponse(app::CommandHandler * command, ApplicationLauncherResponse res
}
}

::ApplicationLauncherApp getApplicationFromCommand(uint16_t catalogVendorId, uint8_t * applicationId)
::ApplicationLauncherApp getApplicationFromCommand(uint16_t catalogVendorId, CharSpan applicationId)
{
::ApplicationLauncherApp application = {};
application.applicationId = applicationId;
application.catalogVendorId = catalogVendorId;
// TODO: Need to figure out what types we're using here.
// application.applicationId = applicationId;
application.catalogVendorId = catalogVendorId;
return application;
}

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.data;
auto & requestApplicationCatalogVendorId = commandData.catalogVendorId;
auto & requestApplicationId = commandData.applicationId;

::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));
std::string reqestDataString(requestData.data(), requestData.size());
ApplicationLauncherResponse response = applicationLauncherClusterLaunchApp(application, reqestDataString);
sendResponse(command, response);
return true;
13 changes: 8 additions & 5 deletions src/app/clusters/audio-output-server/audio-output-server.cpp
Original file line number Diff line number Diff line change
@@ -31,22 +31,25 @@ using namespace chip;
using namespace chip::app::Clusters::AudioOutput;

bool audioOutputClusterSelectOutput(uint8_t index);
bool audioOutputClusterRenameOutput(uint8_t index, uint8_t * name);
bool audioOutputClusterRenameOutput(uint8_t index, const CharSpan & 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);
Original file line number Diff line number Diff line change
@@ -299,12 +299,13 @@ 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)
{
EndpointId endpoint = emberAfCurrentCommand()->apsFrame->destinationEndpoint;
auto & percentOpen = commandData.percentOpen;

EndpointId endpoint = commandPath.mEndpointId;
EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;

emberAfBarrierControlClusterPrintln("RX: GoToPercentCallback p=%d", percentOpen);
@@ -345,10 +346,10 @@ 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;
EndpointId endpoint = commandPath.mEndpointId;
emberAfDeactivateServerTick(endpoint, BarrierControl::Id);
setMovingState(endpoint, EMBER_ZCL_BARRIER_CONTROL_MOVING_STATE_STOPPED);
sendDefaultResponse(EMBER_ZCL_STATUS_SUCCESS);
16 changes: 12 additions & 4 deletions src/app/clusters/bindings/bindings.cpp
Original file line number Diff line number Diff line change
@@ -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;
@@ -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;
Loading

0 comments on commit 1623554

Please sign in to comment.