Skip to content

Commit

Permalink
Cache rotating ID in OnUserDirectedCommissioningRequest
Browse files Browse the repository at this point in the history
  • Loading branch information
@mthiesc
mthiesc committed Jul 6, 2024
1 parent 2b99f24 commit 6a66ce9
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 87 deletions.
24 changes: 13 additions & 11 deletions examples/tv-app/android/include/account-login/AccountLoginManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ AccountLoginManager::AccountLoginManager(ContentAppCommandDelegate * commandDele
CopyString(mSetupPin, sizeof(mSetupPin), setupPin);
}

bool AccountLoginManager::HandleLogin(const CharSpan & tempAccountId, const CharSpan & setupPIN,
bool AccountLoginManager::HandleLogin(const CharSpan & tempAccountIdentifier, const CharSpan & setupPIN,
const chip::Optional<chip::NodeId> & nodeId)
{
ChipLogProgress(DeviceLayer, "AccountLoginManager::HandleLogin called for endpoint %d", mEndpointId);
Expand All @@ -76,16 +76,16 @@ bool AccountLoginManager::HandleLogin(const CharSpan & tempAccountId, const Char

Json::Value response;
bool commandHandled = true;
AccountLogin::Commands::Login::Type cmd = { tempAccountId, setupPIN, nodeId };
AccountLogin::Commands::Login::Type cmd = { tempAccountIdentifier, setupPIN, nodeId };

// Deliberately ignore returned status
mCommandDelegate->InvokeCommand(mEndpointId, AccountLogin::Id,
auto status = mCommandDelegate->InvokeCommand(mEndpointId, AccountLogin::Id,
AccountLogin::Commands::Login::Id,
serializeLoginCommand(cmd), commandHandled, response);

Status status = mCommandDelegate->FormatStatusResponse(response);
if (status == Status::Success) {
// Format status response to verify that response is non-failure.
status = mCommandDelegate->FormatStatusResponse(response);
}
ChipLogProgress(Zcl, "AccountLoginManager::HandleLogin command returned with status: %d", chip::to_underlying(status));

return status == chip::Protocols::InteractionModel::Status::Success;
}

Expand All @@ -108,14 +108,16 @@ bool AccountLoginManager::HandleLogout(const chip::Optional<chip::NodeId> & node
bool commandHandled = true;
AccountLogin::Commands::Logout::Type cmd = { nodeId };

// Deliberately ignore returned status
mCommandDelegate->InvokeCommand(mEndpointId, AccountLogin::Id,
auto status = mCommandDelegate->InvokeCommand(mEndpointId, AccountLogin::Id,
AccountLogin::Commands::Logout::Id,
serializeLogoutCommand(cmd), commandHandled, response);

Status status = mCommandDelegate->FormatStatusResponse(response);
if (status == Status::Success)
{
// Format status response to verify that response is non-failure.
status = mCommandDelegate->FormatStatusResponse(response);
}
ChipLogProgress(Zcl, "AccountLoginManager::HandleLogout command returned with status: %d", chip::to_underlying(status));

return status == chip::Protocols::InteractionModel::Status::Success;
}

Expand Down
12 changes: 7 additions & 5 deletions examples/tv-app/android/java/ContentAppCommandDelegate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ void ContentAppCommandDelegate::InvokeCommand(CommandHandlerInterface::HandlerCo
Status ContentAppCommandDelegate::InvokeCommand(EndpointId epId, ClusterId clusterId, CommandId commandId, std::string payload,
bool & commandHandled, Json::Value & value)
{
// Q1: It seems like this InvokeCommand *never* returns success status.
if (epId >= FIXED_ENDPOINT_COUNT)
{
JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread();
Expand All @@ -121,6 +120,7 @@ Status ContentAppCommandDelegate::InvokeCommand(EndpointId epId, ClusterId clust
ChipLogError(Zcl, "Java exception in ContentAppCommandDelegate::sendCommand");
env->ExceptionDescribe();
env->ExceptionClear();
return chip::Protocols::InteractionModel::Status::Failure;
}
else
{
Expand All @@ -136,7 +136,8 @@ Status ContentAppCommandDelegate::InvokeCommand(EndpointId epId, ClusterId clust
}
env->DeleteLocalRef(resp);

// handle errors from platform-app
// Parse response here in case there is failure response.
// Return non-success error code to indicate to caller it should not parse response.
if (!value[FAILURE_KEY].empty())
{
value = value[FAILURE_KEY];
Expand All @@ -147,8 +148,9 @@ Status ContentAppCommandDelegate::InvokeCommand(EndpointId epId, ClusterId clust
return chip::Protocols::InteractionModel::Status::Failure;
}

// Q: It would seem that this is the successfull return case (?)
return chip::Protocols::InteractionModel::Status::UnsupportedEndpoint;
// Return success to indicate command has been sent, response returned and parsed successfully.
// Caller has to manually parse value input/output parameter to get response status/object.
return chip::Protocols::InteractionModel::Status::Success;
}
else
{
Expand Down Expand Up @@ -342,7 +344,7 @@ GetSetupPINResponseType ContentAppCommandDelegate::FormatGetSetupPINResponse(Jso
}
else
{
getSetupPINresponse.setupPIN = "";
status = chip::Protocols::InteractionModel::Status::Failure;
}
return getSetupPINresponse;
}
Expand Down
16 changes: 10 additions & 6 deletions examples/tv-app/android/java/TVApp-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,13 +252,13 @@ SampleTvAppInstallationService gSampleTvAppInstallationService;

class MyPostCommissioningListener : public PostCommissioningListener
{
void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId, std::string rotatingId, uint32_t passcode,
void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId, CharSpan rotatingId, uint32_t passcode,
Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle) override
{
// read current binding list
chip::Controller::ClusterBase cluster(exchangeMgr, sessionHandle, kTargetBindingClusterEndpointId);

cacheContext(vendorId, productId, nodeId, std::move(rotatingId), passcode, exchangeMgr, sessionHandle);
cacheContext(vendorId, productId, nodeId, rotatingId, passcode, exchangeMgr, sessionHandle);

CHIP_ERROR err =
cluster.ReadAttribute<Binding::Attributes::Binding::TypeInfo>(this, OnReadSuccessResponse, OnReadFailureResponse);
Expand Down Expand Up @@ -342,18 +342,18 @@ class MyPostCommissioningListener : public PostCommissioningListener

Optional<SessionHandle> opt = mSecureSession.Get();
SessionHandle & sessionHandle = opt.Value();
ContentAppPlatform::GetInstance().ManageClientAccess(*mExchangeMgr, sessionHandle, mVendorId, mProductId, localNodeId, mRotatingId, mPasscode,
ContentAppPlatform::GetInstance().ManageClientAccess(*mExchangeMgr, sessionHandle, mVendorId, mProductId, localNodeId, getRotatingIdSpan(), mPasscode,
bindings, OnSuccessResponse, OnFailureResponse);
clearContext();
}

void cacheContext(uint16_t vendorId, uint16_t productId, NodeId nodeId, std::string rotatingId, uint32_t passcode, Messaging::ExchangeManager & exchangeMgr,
void cacheContext(uint16_t vendorId, uint16_t productId, NodeId nodeId, CharSpan rotatingId, uint32_t passcode, Messaging::ExchangeManager & exchangeMgr,
const SessionHandle & sessionHandle)
{
mVendorId = vendorId;
mProductId = productId;
mNodeId = nodeId;
mRotatingId = std::move(rotatingId);
mRotatingId = std::string{ rotatingId.data(), rotatingId.size() }; // Allocates and copies to string instead of storing span to make sure lifetime is valid.
mPasscode = passcode;
mExchangeMgr = &exchangeMgr;
mSecureSession.ShiftToSession(sessionHandle);
Expand All @@ -367,11 +367,15 @@ class MyPostCommissioningListener : public PostCommissioningListener
mExchangeMgr = nullptr;
mSecureSession.SessionReleased();
}
CharSpan getRotatingIdSpan() {
return { mRotatingId.data(), mRotatingId.size() };
}

uint16_t mVendorId = 0;
uint16_t mProductId = 0;
NodeId mNodeId = 0;
std::string mRotatingId;
uint32_t mPasscode = 0;
uint32_t mPasscode = 0;
Messaging::ExchangeManager * mExchangeMgr = nullptr;
SessionHolder mSecureSession;
};
Expand Down
15 changes: 10 additions & 5 deletions examples/tv-app/tv-common/src/AppTv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,13 @@ MyAppInstallationService gMyAppInstallationService;

class MyPostCommissioningListener : public PostCommissioningListener
{
void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId, std::string rotatingId, uint32_t passcode,
void CommissioningCompleted(uint16_t vendorId, uint16_t productId, NodeId nodeId, CharSpan rotatingId, uint32_t passcode,
Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle) override
{
// read current binding list
chip::Controller::ClusterBase cluster(exchangeMgr, sessionHandle, kTargetBindingClusterEndpointId);

cacheContext(vendorId, productId, nodeId, std::move(rotatingId), passcode, exchangeMgr, sessionHandle);
cacheContext(vendorId, productId, nodeId, rotatingId, passcode, exchangeMgr, sessionHandle);

CHIP_ERROR err =
cluster.ReadAttribute<Binding::Attributes::Binding::TypeInfo>(this, OnReadSuccessResponse, OnReadFailureResponse);
Expand Down Expand Up @@ -248,18 +248,18 @@ class MyPostCommissioningListener : public PostCommissioningListener

Optional<SessionHandle> opt = mSecureSession.Get();
SessionHandle & sessionHandle = opt.Value();
ContentAppPlatform::GetInstance().ManageClientAccess(*mExchangeMgr, sessionHandle, mVendorId, mProductId, localNodeId, rotatingId, passcode,
ContentAppPlatform::GetInstance().ManageClientAccess(*mExchangeMgr, sessionHandle, mVendorId, mProductId, localNodeId, getRotatingIdSpan(), passcode,
bindings, OnSuccessResponse, OnFailureResponse);
clearContext();
}

void cacheContext(uint16_t vendorId, uint16_t productId, NodeId nodeId, std::string rotatingId, uint32_t passcode,
void cacheContext(uint16_t vendorId, uint16_t productId, NodeId nodeId, CharSpan rotatingId, uint32_t passcode,
Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle)
{
mVendorId = vendorId;
mProductId = productId;
mNodeId = nodeId;
mRotatingId = std::move(rotatingId);
mRotatingId = std::string{ rotatingId.data(), rotatingId.size() }; // Allocates and copies to string instead of storing span to make sure lifetime is valid.
mPassocde = passcode;
mExchangeMgr = &exchangeMgr;
mSecureSession.ShiftToSession(sessionHandle);
Expand All @@ -273,6 +273,11 @@ class MyPostCommissioningListener : public PostCommissioningListener
mExchangeMgr = nullptr;
mSecureSession.SessionReleased();
}

CharSpan getRotatingIdSpan() {
return { mRotatingId.data(), mRotatingId.size() };
}

uint16_t mVendorId = 0;
uint16_t mProductId = 0;
NodeId mNodeId = 0;
Expand Down
18 changes: 5 additions & 13 deletions src/app/app-platform/ContentAppPlatform.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ CHIP_ERROR ContentAppPlatform::GetACLEntryIndex(size_t * foundIndex, FabricIndex
// and create bindings on the given client so that it knows what it has access to.
CHIP_ERROR ContentAppPlatform::ManageClientAccess(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle,
uint16_t targetVendorId, uint16_t targetProductId, NodeId localNodeId,
const std::string& rotatingId, uint32_t passcode,
CharSpan rotatingId, uint32_t passcode,
std::vector<Binding::Structs::TargetStruct::Type> bindings,
Controller::WriteResponseSuccessCallback successCb,
Controller::WriteResponseFailureCallback failureCb)
Expand Down Expand Up @@ -757,18 +757,10 @@ CHIP_ERROR ContentAppPlatform::ManageClientAccess(Messaging::ExchangeManager & e
// notify content app about this nodeId
app->AddClientNode(subjectNodeId);

auto tempAccountId = CharSpan::fromCharString(rotatingId.c_str());
auto setupPIN = CharSpan::fromCharString(std::to_string(passcode).c_str());
auto nodeId = MakeOptional(subjectNodeId); // Q: Is subjectNodeId correct NodeId to use?

// send login command to content app
if (!tempAccountId.empty() && !setupPIN.empty()) {
// Q: Should we only do this if (commissioner) passcode is not 0, e.g when user was shown PIN prompt?
auto status = app->GetAccountLoginDelegate()->HandleLogin(tempAccountId, setupPIN, nodeId);
ChipLogProgress(Controller, "AccountLogin::Login command sent and returned with status: %d", status);
} else {
ChipLogError(Controller, "Failed to get tempAccountId or setupPIN to set AccountLogin::Login command");
}
// handle login
auto setupPIN = std::to_string(passcode);
auto status = app->GetAccountLoginDelegate()->HandleLogin(rotatingId, { setupPIN.data(), setupPIN.size() }, MakeOptional(subjectNodeId));
ChipLogProgress(Controller, "AccountLogin::Login command sent and returned with status: %d", status);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/app/app-platform/ContentAppPlatform.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ class DLL_EXPORT ContentAppPlatform
*/
CHIP_ERROR ManageClientAccess(Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle, uint16_t targetVendorId,
uint16_t targetProductId, NodeId localNodeId,
const std::string& rotatingId, uint32_t passcode,
chip::CharSpan rotatingId, uint32_t passcode,
std::vector<app::Clusters::Binding::Structs::TargetStruct::Type> bindings,
Controller::WriteResponseSuccessCallback successCb,
Controller::WriteResponseFailureCallback failureCb);
Expand Down
65 changes: 20 additions & 45 deletions src/controller/CommissionerDiscoveryController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,37 +28,9 @@
#include <controller/CommissionerDiscoveryController.h>
#include <platform/CHIPDeviceLayer.h>
#include <setup_payload/AdditionalDataPayloadGenerator.h>
#include <app/app-platform/ContentAppPlatform.h>

#if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY

namespace {

bool fillRotatingIdBuffer(UDCClientState* client, char[] buffer) {
auto rotatingIdLength = client->GetRotatingIdLength();
auto hexBufferByteCount = 2 * rotatingIdLength;
if (sizeof(buffer) < hexBufferByteCount) {
return false;
}
auto err = Encoding::BytesToUppercaseHexBuffer(client->GetRotatingId(), rotatingIdLength, buffer, sizeof(buffer));
return err == CHIP_NO_ERROR;
}

// Returned CharSpan valid lifetime equals buffer lifetime.
CharSpan getRotatingIdSpan(UDCClientState* client, char[] buffer) {
auto ok = fillRotatingIdBuffer(client, buffer);
return ok ? { buffer, hexBufferByteCount } : {};
}

// Allocates memory for rotating ID string and copies to string
std::string getRotatingIdString(UDCClientState* client) {
auto buffer = char[Dnssd::kMaxRotatingIdLen * 2];
auto ok = fillRotatingIdBuffer(client, buffer);
return ok ? { buffer, hexBufferByteCount } : {};
}

}

using namespace ::chip;
using namespace chip::Protocols::UserDirectedCommissioning;

Expand Down Expand Up @@ -193,6 +165,9 @@ void CommissionerDiscoveryController::OnUserDirectedCommissioningRequest(UDCClie
{
ChipLogError(AppServer, "On UDC: could not convert rotating id to hex");
rotatingIdString[0] = '\0';
} else {
// Store rotating ID string. Don't include null terminator character.
mRotatingId = std::string{ rotatingIdString, state.GetRotatingIdLength() * 2 };
}

ChipLogDetail(Controller,
Expand Down Expand Up @@ -273,11 +248,15 @@ void CommissionerDiscoveryController::InternalOk()
}

char rotatingIdBuffer[Dnssd::kMaxRotatingIdLen * 2];
CharSpan rotatingIdSpan = getRotatingIdSpan(client, rotatingIdBuffer);
if (rotatingIdSpan.empty()) {
size_t rotatingIdLength = client->GetRotatingIdLength();
CHIP_ERROR err =
Encoding::BytesToUppercaseHexBuffer(client->GetRotatingId(), rotatingIdLength, rotatingIdBuffer, sizeof(rotatingIdBuffer));
if (err != CHIP_NO_ERROR)
{
ChipLogError(AppServer, "UX InternalOk: could not convert rotating id to hex");
return;
}
CharSpan rotatingIdSpan(rotatingIdBuffer, 2 * rotatingIdLength);

uint8_t targetAppCount = client->GetNumTargetAppInfos();

Expand Down Expand Up @@ -454,11 +433,15 @@ void CommissionerDiscoveryController::InternalHandleContentAppPasscodeResponse()
if (passcode == 0 && client->GetCommissionerPasscode() && client->GetCdPort() != 0)
{
char rotatingIdBuffer[Dnssd::kMaxRotatingIdLen * 2];
CharSpan rotatingIdSpan = getRotatingIdSpan(client, rotatingIdBuffer);
if (rotatingIdSpan.empty()) {
size_t rotatingIdLength = client->GetRotatingIdLength();
CHIP_ERROR err = Encoding::BytesToUppercaseHexBuffer(client->GetRotatingId(), rotatingIdLength, rotatingIdBuffer,
sizeof(rotatingIdBuffer));
if (err != CHIP_NO_ERROR)
{
ChipLogError(AppServer, "UX Ok - HandleContentAppPasscodeResponse: could not convert rotating id to hex");
return;
}
CharSpan rotatingIdSpan(rotatingIdBuffer, 2 * rotatingIdLength);

// first step of commissioner passcode
ChipLogError(AppServer, "UX Ok: commissioner passcode, sending CDC");
Expand Down Expand Up @@ -628,22 +611,14 @@ void CommissionerDiscoveryController::CommissioningSucceeded(uint16_t vendorId,
mProductId = productId;
mNodeId = nodeId;

auto rotatingId = std::string{};
auto passcode = uint32_t{ 0 };

auto client = GetUDCClientState();
if (client != nullptr)
{
// Get rotating ID and cached (commissioner?) passcode to handle AccountLogin
rotatingId = getRotatingIdString(client);
// Q: Should we use mPasscode, client->GetCommissionerPasscode, or client->GetCachedCommissionerPasscode?
passcode = std::to_string(client->GetCachedCommissionerPasscode());
}

if (mPostCommissioningListener != nullptr)
{
ChipLogDetail(Controller, "CommissionerDiscoveryController calling listener");
mPostCommissioningListener->CommissioningCompleted(vendorId, productId, nodeId, std::move(rotatingId), passcode, exchangeMgr, sessionHandle);
// Qs:
// * Is it right to use mPasscode here?
// * Would it not be zero if HandleContentAppPasscodeResponse returns zero passcode?
// * Which client/app does the passcode refer to? Same question for Rotating ID.
mPostCommissioningListener->CommissioningCompleted(vendorId, productId, nodeId, GetRotatingIdSpan(), mPasscode, exchangeMgr, sessionHandle);
}
else
{
Expand Down
Loading

0 comments on commit 6a66ce9

Please sign in to comment.