Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[petition] return ID of existing active commissioner if one exists #36

Merged
merged 4 commits into from
Mar 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions include/commissioner/commissioner.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,28 +160,39 @@ class Commissioner
/**
* The response handler of a general TMF request.
*
* @param[in] aError A error code.
* @param[in] aError An error code.
*/
using ErrorHandler = std::function<void(Error aError)>;

/**
* The response handler of a general TMF request.
*
* @param[in] aResponseData A response data. nullable.
* @param[in] aError A error code.
* @param[in] aError An error code.
*
* @note @p aResponseData is guaranteed to be not null only when @p aError == Error::kNone.
* Otherwise, @p aResponseData should never be accessed.
*/
template <class T> using Handler = std::function<void(const T *aResponseData, Error aError)>;
template <typename T> using Handler = std::function<void(const T *aResponseData, Error aError)>;

/**
* The petition result handler.
*
* @param[in] aExistingCommissionerId The Existing commissioner Id. nullable.
* @param[in] aError An error code.
*
* @note There is an exiting active commissioner if @p aError != Error::kNone
* and @p aExistingCommissionerId is not null.
*/
using PetitionHandler = std::function<void(const std::string *aExistingCommissionerId, Error aError)>;

/**
* The MGMT_PANID_CONFLICT.ans handler.
*
* @param[in] aPeerAddr A peer address that sent this MGMT_PANID_CONFLICT.ans request.
* @param[in] aChannelMask A channel mask the peer scanned at.
* @param[in] aPanId A PAN ID that has a conflict.
* @param[in] aError A error code.
* @param[in] aError An error code.
*
* @note @p aPeerAddr, aChannelMask and aPanId are guaranteed to
* be not null only when @p aError == Error::kNone. Otherwise,
Expand All @@ -196,7 +207,7 @@ class Commissioner
* @param[in] aPeerAddr A peer address that sent this MGMT_PANID_CONFLICT.ans request.
* @param[in] aChannelMask A channel mask the peer scanned at.
* @param[in] aEnergyList A list of measured energy level in dBm.
* @param[in] aError A error code.
* @param[in] aError An error code.
*
* @note @p aPeerAddr, aChannelMask and aEnergyList are guaranteed to
* be not null only when @p aError == Error::kNone. Otherwise,
Expand Down Expand Up @@ -422,7 +433,7 @@ class Commissioner
*
* @note The commissioner will be first connected to the network if it is not.
*/
virtual void Petition(ErrorHandler aHandler, const std::string &aAddr, uint16_t aPort) = 0;
virtual void Petition(PetitionHandler aHandler, const std::string &aAddr, uint16_t aPort) = 0;

/**
* @brief Synchronously petition to a Thread network.
Expand All @@ -431,12 +442,18 @@ class Commissioner
* If succeed, a keep-alive message will be periodically sent to keep itself active.
* It will not return until errors happened, timeouted or succeed.
*
* @param[in] aAddr A border agent address.
* @param[in] aPort A border agent port.
* @param[out] aExistingCommissionerId The existing active commissioner ID.
* @param[in] aAddr A border agent address.
* @param[in] aPort A border agent port.
*
* @return Error::kNone, succeed; otherwise, failed;
*
* @note @p aExistingCommissionerId is valid only when return value
* is not Error::kNone and itself is not empty. Otherwise,
* its content should never be accessed.
*
*/
virtual Error Petition(const std::string &aAddr, uint16_t aPort) = 0;
virtual Error Petition(std::string &aExistingCommissionerId, const std::string &aAddr, uint16_t aPort) = 0;

/**
* @brief Asynchronously resign from the commissioner role.
Expand Down
2 changes: 1 addition & 1 deletion include/commissioner/error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ enum class Error : int
/**
* @brief Convert Error to printable string.
*
* @param aError A error code.
* @param aError An error code.
*
* @return The printable string of the error.
*/
Expand Down
7 changes: 6 additions & 1 deletion src/app/cli/interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,13 +316,18 @@ Interpreter::Value Interpreter::ProcessStart(const Expression &aExpr)
{
Error error = Error::kInvalidArgs;
std::string msg;
std::string existingCommissionerId;
uint16_t port;

VerifyOrExit(aExpr.size() >= 3, msg = Usage(aExpr));
SuccessOrExit(ParseInteger(port, aExpr[2]), msg = aExpr[2]);
SuccessOrExit(error = mCommissioner->Start(aExpr[1], port));
SuccessOrExit(error = mCommissioner->Start(existingCommissionerId, aExpr[1], port));

exit:
if (!existingCommissionerId.empty())
{
msg = "there is an existing active commissioner: " + existingCommissionerId;
}
return {error, msg};
}

Expand Down
7 changes: 5 additions & 2 deletions src/app/commissioner_app.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,14 @@ const BorderAgent *CommissionerApp::GetBorderAgent(const std::string &aNetworkNa
return nullptr;
}

Error CommissionerApp::Start(const std::string &aBorderAgentAddr, uint16_t aBorderAgentPort)
Error CommissionerApp::Start(std::string & aExistingCommissionerId,
const std::string &aBorderAgentAddr,
uint16_t aBorderAgentPort)
{
Error error = Error::kNone;

SuccessOrExit(error = mCommissioner->Petition(aBorderAgentAddr, aBorderAgentPort));
// We need to report the already active commissioner ID if one exists.
SuccessOrExit(error = mCommissioner->Petition(aExistingCommissionerId, aBorderAgentAddr, aBorderAgentPort));
SuccessOrExit(error = PullNetworkData());

exit:
Expand Down
2 changes: 1 addition & 1 deletion src/app/commissioner_app.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class CommissionerApp
// Set @p aNetworkName to empty to match any network name.
const BorderAgent *GetBorderAgent(const std::string &aNetworkName);

Error Start(const std::string &aBorderAgentAddr, uint16_t aBorderAgentPort);
Error Start(std::string &aExistingCommissionerId, const std::string &aBorderAgentAddr, uint16_t aBorderAgentPort);
void Stop();

void AbortRequests();
Expand Down
26 changes: 18 additions & 8 deletions src/library/commissioner_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,12 @@ void CommissionerImpl::Stop()
event_base_loopbreak(mEventBase);
}

void CommissionerImpl::Petition(ErrorHandler aHandler, const std::string &aAddr, uint16_t aPort)
void CommissionerImpl::Petition(PetitionHandler aHandler, const std::string &aAddr, uint16_t aPort)
{
auto onConnected = [this, aHandler](Error aError) {
if (aError != Error::kNone)
{
aHandler(aError);
aHandler(nullptr, aError);
}
else
{
Expand Down Expand Up @@ -1101,16 +1101,18 @@ void CommissionerImpl::SetEnergyReportHandler(EnergyReportHandler aHandler)
mEnergyReportHandler = aHandler;
}

void CommissionerImpl::SendPetition(ErrorHandler aHandler)
void CommissionerImpl::SendPetition(PetitionHandler aHandler)
{
Error error = Error::kNone;
coap::Request request{coap::Type::kConfirmable, coap::Code::kPost};

auto onResponse = [this, aHandler](const coap::Response *aResponse, Error aError) {
Error error = Error::kNone;
tlv::TlvSet tlvSet;
tlv::TlvPtr stateTlv = nullptr;
tlv::TlvPtr sessionIdTlv = nullptr;
tlv::TlvPtr stateTlv = nullptr;
tlv::TlvPtr sessionIdTlv = nullptr;
tlv::TlvPtr commissionerIdTlv = nullptr;
std::string existingCommissionerId;

SuccessOrExit(error = aError);

Expand All @@ -1121,7 +1123,15 @@ void CommissionerImpl::SendPetition(ErrorHandler aHandler)
VerifyOrExit(stateTlv != nullptr, error = Error::kNotFound);
VerifyOrExit(stateTlv->IsValid(), error = Error::kBadFormat);

VerifyOrExit(stateTlv->GetValueAsInt8() == tlv::kStateAccept, error = Error::kReject);
if (stateTlv->GetValueAsInt8() != tlv::kStateAccept)
{
commissionerIdTlv = tlvSet[tlv::Type::kCommissionerId];
if (commissionerIdTlv != nullptr && commissionerIdTlv->IsValid())
{
existingCommissionerId = commissionerIdTlv->GetValueAsString();
}
ExitNow(error = Error::kReject);
}

sessionIdTlv = tlvSet[tlv::Type::kCommissionerSessionId];
VerifyOrExit(sessionIdTlv != nullptr, error = Error::kNotFound);
Expand All @@ -1141,7 +1151,7 @@ void CommissionerImpl::SendPetition(ErrorHandler aHandler)
{
mState = State::kDisabled;
}
aHandler(error);
aHandler(existingCommissionerId.empty() ? nullptr : &existingCommissionerId, error);
};

VerifyOrExit(mState == State::kDisabled, error = Error::kInvalidState);
Expand All @@ -1163,7 +1173,7 @@ void CommissionerImpl::SendPetition(ErrorHandler aHandler)
exit:
if (error != Error::kNone)
{
aHandler(error);
aHandler(nullptr, error);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/library/commissioner_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ class CommissionerImpl : public Commissioner

void Disconnect() override;

void Petition(ErrorHandler aHandler, const std::string &aAddr, uint16_t aPort) override;
Error Petition(const std::string &, uint16_t) override { return Error::kNotImplemented; }
void Petition(PetitionHandler aHandler, const std::string &aAddr, uint16_t aPort) override;
Error Petition(std::string &, const std::string &, uint16_t) override { return Error::kNotImplemented; }

void Resign(ErrorHandler aHandler) override;
Error Resign() override { return Error::kNotImplemented; }
Expand Down Expand Up @@ -234,7 +234,7 @@ class CommissionerImpl : public Commissioner
static Error EncodeCommissionerDataset(coap::Request &aRequest, const CommissionerDataset &aDataset);
static ByteArray GetCommissionerDatasetTlvs(uint16_t aDatasetFlags);

void SendPetition(ErrorHandler aHandler);
void SendPetition(PetitionHandler aHandler);

// Set @p aKeepAlive to false to resign the commissioner role.
void SendKeepAlive(Timer &aTimer, bool aKeepAlive = true);
Expand Down
12 changes: 9 additions & 3 deletions src/library/commissioner_safe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,15 +190,21 @@ void CommissionerSafe::AbortRequests()
PushAsyncRequest([=]() { mImpl.AbortRequests(); });
}

void CommissionerSafe::Petition(ErrorHandler aHandler, const std::string &aAddr, uint16_t aPort)
void CommissionerSafe::Petition(PetitionHandler aHandler, const std::string &aAddr, uint16_t aPort)
{
PushAsyncRequest([=]() { mImpl.Petition(aHandler, aAddr, aPort); });
}

Error CommissionerSafe::Petition(const std::string &aAddr, uint16_t aPort)
Error CommissionerSafe::Petition(std::string &aExistingCommissionerId, const std::string &aAddr, uint16_t aPort)
{
std::promise<Error> pro;
auto wait = [&pro](Error aError) { pro.set_value(aError); };
auto wait = [&pro, &aExistingCommissionerId](const std::string *existingCommissionerId, Error error) {
if (existingCommissionerId != nullptr)
{
aExistingCommissionerId = *existingCommissionerId;
}
pro.set_value(error);
};

Petition(wait, aAddr, aPort);
return pro.get_future().get();
Expand Down
4 changes: 2 additions & 2 deletions src/library/commissioner_safe.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ class CommissionerSafe : public Commissioner

void Disconnect() override;

void Petition(ErrorHandler aHandler, const std::string &aAddr, uint16_t aPort) override;
Error Petition(const std::string &aAddr, uint16_t aPort) override;
void Petition(PetitionHandler aHandler, const std::string &aAddr, uint16_t aPort) override;
Error Petition(std::string &aExistingCommissionerId, const std::string &aAddr, uint16_t aPort) override;

void Resign(ErrorHandler aHandler) override;
Error Resign() override;
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/test_commissioner_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ TEST_CASE("commissioner-impl-not-implemented-APIs", "[comm-impl]")
std::list<BorderAgent> baList;
REQUIRE(commImpl.Discover(baList) == Error::kNotImplemented);
REQUIRE(commImpl.Connect("::1", 5684) == Error::kNotImplemented);
REQUIRE(commImpl.Petition("::1", 5684) == Error::kNotImplemented);

std::string existingCommissionerId;
REQUIRE(commImpl.Petition(existingCommissionerId, "::1", 5684) == Error::kNotImplemented);
REQUIRE(commImpl.Resign() == Error::kNotImplemented);

CommissionerDataset commDataset;
Expand Down