Skip to content

Commit

Permalink
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
  • Loading branch information
bzbarsky-apple authored Oct 12, 2021
1 parent c2b7217 commit 4d722f7
Show file tree
Hide file tree
Showing 59 changed files with 1,939 additions and 2,726 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions examples/window-app/common/src/ZclCallbacks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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)
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.status;

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 @@ -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;
Expand Down
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
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
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 4d722f7

Please sign in to comment.