Skip to content

Commit

Permalink
[chip-tool] Some CHIP_ERROR returned values are ignored (#25486)
Browse files Browse the repository at this point in the history
  • Loading branch information
vivien-apple authored and pull[bot] committed Jan 18, 2024
1 parent 104a7dc commit 3593004
Show file tree
Hide file tree
Showing 11 changed files with 83 additions and 64 deletions.
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/clusters/CustomArgument.h
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ class CustomArgument
{
chip::TLV::TLVReader reader;
reader.Init(mData, mDataLen);
reader.Next();
ReturnErrorOnFailure(reader.Next());

return writer.CopyElement(tag, reader);
}
Expand Down
23 changes: 7 additions & 16 deletions examples/chip-tool/commands/clusters/DataModelLogger.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,26 +103,20 @@ class DataModelLogger
template <typename X, typename std::enable_if_t<std::is_enum<X>::value, int> = 0>
static CHIP_ERROR LogValue(const char * label, size_t indent, X value)
{
DataModelLogger::LogValue(label, indent, chip::to_underlying(value));
return CHIP_NO_ERROR;
return DataModelLogger::LogValue(label, indent, chip::to_underlying(value));
}

template <typename X>
static CHIP_ERROR LogValue(const char * label, size_t indent, chip::BitFlags<X> value)
{
DataModelLogger::LogValue(label, indent, value.Raw());
return CHIP_NO_ERROR;
return DataModelLogger::LogValue(label, indent, value.Raw());
}

template <typename T>
static CHIP_ERROR LogValue(const char * label, size_t indent, const chip::app::DataModel::DecodableList<T> & value)
{
size_t count = 0;
CHIP_ERROR err = value.ComputeSize(&count);
if (err != CHIP_NO_ERROR)
{
return err;
}
size_t count = 0;
ReturnErrorOnFailure(value.ComputeSize(&count));
DataModelLogger::LogString(label, indent, std::to_string(count) + " entries");

auto iter = value.begin();
Expand All @@ -146,21 +140,18 @@ class DataModelLogger
if (value.IsNull())
{
DataModelLogger::LogString(label, indent, "null");
}
else
{
DataModelLogger::LogValue(label, indent, value.Value());
return CHIP_NO_ERROR;
}

return CHIP_NO_ERROR;
return DataModelLogger::LogValue(label, indent, value.Value());
}

template <typename T>
static CHIP_ERROR LogValue(const char * label, size_t indent, const chip::Optional<T> & value)
{
if (value.HasValue())
{
DataModelLogger::LogValue(label, indent, value.Value());
return DataModelLogger::LogValue(label, indent, value.Value());
}

return CHIP_NO_ERROR;
Expand Down
11 changes: 10 additions & 1 deletion examples/chip-tool/commands/common/CHIPCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,16 @@ CHIP_ERROR CHIPCommand::StartWaiting(chip::System::Clock::Timeout duration)
mWaitingForResponse = true;
}

chip::DeviceLayer::PlatformMgr().ScheduleWork(RunQueuedCommand, reinterpret_cast<intptr_t>(this));
auto err = chip::DeviceLayer::PlatformMgr().ScheduleWork(RunQueuedCommand, reinterpret_cast<intptr_t>(this));
if (CHIP_NO_ERROR != err)
{
{
std::lock_guard<std::mutex> lk(cvWaitingForResponseMutex);
mWaitingForResponse = false;
}
return err;
}

auto waitingUntil = std::chrono::system_clock::now() + std::chrono::duration_cast<std::chrono::seconds>(duration);
{
std::unique_lock<std::mutex> lk(cvWaitingForResponseMutex);
Expand Down
51 changes: 27 additions & 24 deletions examples/chip-tool/commands/common/RemoteDataModelLogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,48 +169,51 @@ CHIP_ERROR LogDiscoveredNodeData(const chip::Dnssd::DiscoveredNodeData & nodeDat
{
VerifyOrReturnError(gDelegate != nullptr, CHIP_NO_ERROR);

if (!chip::CanCastTo<uint8_t>(nodeData.resolutionData.numIPs))
auto & resolutionData = nodeData.resolutionData;
auto & commissionData = nodeData.commissionData;

if (!chip::CanCastTo<uint8_t>(resolutionData.numIPs))
{
ChipLogError(chipTool, "Too many ips.");
return CHIP_ERROR_INVALID_ARGUMENT;
}

if (!chip::CanCastTo<uint64_t>(nodeData.commissionData.rotatingIdLen))
if (!chip::CanCastTo<uint64_t>(commissionData.rotatingIdLen))
{
ChipLogError(chipTool, "Can not convert rotatingId to json format.");
return CHIP_ERROR_INVALID_ARGUMENT;
}

char rotatingId[chip::Dnssd::kMaxRotatingIdLen * 2 + 1] = "";
chip::Encoding::BytesToUppercaseHexString(nodeData.commissionData.rotatingId, nodeData.commissionData.rotatingIdLen, rotatingId,
sizeof(rotatingId));
ReturnErrorOnFailure(chip::Encoding::BytesToUppercaseHexString(commissionData.rotatingId, commissionData.rotatingIdLen,
rotatingId, sizeof(rotatingId)));

Json::Value value;
value["hostName"] = nodeData.resolutionData.hostName;
value["instanceName"] = nodeData.commissionData.instanceName;
value["longDiscriminator"] = nodeData.commissionData.longDiscriminator;
value["shortDiscriminator"] = ((nodeData.commissionData.longDiscriminator >> 8) & 0x0F);
value["vendorId"] = nodeData.commissionData.vendorId;
value["productId"] = nodeData.commissionData.productId;
value["commissioningMode"] = nodeData.commissionData.commissioningMode;
value["deviceType"] = nodeData.commissionData.deviceType;
value["deviceName"] = nodeData.commissionData.deviceName;
value["hostName"] = resolutionData.hostName;
value["instanceName"] = commissionData.instanceName;
value["longDiscriminator"] = commissionData.longDiscriminator;
value["shortDiscriminator"] = ((commissionData.longDiscriminator >> 8) & 0x0F);
value["vendorId"] = commissionData.vendorId;
value["productId"] = commissionData.productId;
value["commissioningMode"] = commissionData.commissioningMode;
value["deviceType"] = commissionData.deviceType;
value["deviceName"] = commissionData.deviceName;
value["rotatingId"] = rotatingId;
value["rotatingIdLen"] = static_cast<uint64_t>(nodeData.commissionData.rotatingIdLen);
value["pairingHint"] = nodeData.commissionData.pairingHint;
value["pairingInstruction"] = nodeData.commissionData.pairingInstruction;
value["supportsTcp"] = nodeData.resolutionData.supportsTcp;
value["port"] = nodeData.resolutionData.port;
value["numIPs"] = static_cast<uint8_t>(nodeData.resolutionData.numIPs);

if (nodeData.resolutionData.mrpRetryIntervalIdle.HasValue())
value["rotatingIdLen"] = static_cast<uint64_t>(commissionData.rotatingIdLen);
value["pairingHint"] = commissionData.pairingHint;
value["pairingInstruction"] = commissionData.pairingInstruction;
value["supportsTcp"] = resolutionData.supportsTcp;
value["port"] = resolutionData.port;
value["numIPs"] = static_cast<uint8_t>(resolutionData.numIPs);

if (resolutionData.mrpRetryIntervalIdle.HasValue())
{
value["mrpRetryIntervalIdle"] = nodeData.resolutionData.mrpRetryIntervalIdle.Value().count();
value["mrpRetryIntervalIdle"] = resolutionData.mrpRetryIntervalIdle.Value().count();
}

if (nodeData.resolutionData.mrpRetryIntervalActive.HasValue())
if (resolutionData.mrpRetryIntervalActive.HasValue())
{
value["mrpRetryIntervalActive"] = nodeData.resolutionData.mrpRetryIntervalActive.Value().count();
value["mrpRetryIntervalActive"] = resolutionData.mrpRetryIntervalActive.Value().count();
}

Json::Value rootValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ void DiscoverCommissionablesCommandBase::OnDiscoveredDevice(const chip::Dnssd::D
if (mDiscoverOnce.ValueOr(true))
{
mCommissioner->RegisterDeviceDiscoveryDelegate(nullptr);
mCommissioner->StopCommissionableDiscovery();
SetCommandExitStatus(CHIP_NO_ERROR);
auto err = mCommissioner->StopCommissionableDiscovery();
SetCommandExitStatus(err);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,10 @@ bool InteractiveCommand::ParseCommand(char * command, int * status)
{
if (strcmp(command, kInteractiveModeStopCommand) == 0)
{
chip::DeviceLayer::PlatformMgr().ScheduleWork(ExecuteDeferredCleanups, 0);
// If scheduling the cleanup fails, there is not much we can do.
// But if something went wrong while the application is leaving it could be because things have
// not been cleaned up properly, so it is still useful to log the failure.
LogErrorOnFailure(chip::DeviceLayer::PlatformMgr().ScheduleWork(ExecuteDeferredCleanups, 0));
return false;
}

Expand Down
34 changes: 24 additions & 10 deletions examples/chip-tool/commands/pairing/PairingCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,22 +269,32 @@ void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err)

void PairingCommand::OnDiscoveredDevice(const chip::Dnssd::DiscoveredNodeData & nodeData)
{
// Ignore nodes with closed comissioning window
// Ignore nodes with closed commissioning window
VerifyOrReturn(nodeData.commissionData.commissioningMode != 0);

const uint16_t port = nodeData.resolutionData.port;
auto & resolutionData = nodeData.resolutionData;

const uint16_t port = resolutionData.port;
char buf[chip::Inet::IPAddress::kMaxStringLength];
nodeData.resolutionData.ipAddress[0].ToString(buf);
resolutionData.ipAddress[0].ToString(buf);
ChipLogProgress(chipTool, "Discovered Device: %s:%u", buf, port);

// Stop Mdns discovery.
CurrentCommissioner().StopCommissionableDiscovery();
auto err = CurrentCommissioner().StopCommissionableDiscovery();

// Some platforms does not implement a mechanism to stop mdns browse, so
// we just ignore CHIP_ERROR_NOT_IMPLEMENTED instead of bailing out.
if (CHIP_NO_ERROR != err && CHIP_ERROR_NOT_IMPLEMENTED != err)
{
SetCommandExitStatus(err);
return;
}

CurrentCommissioner().RegisterDeviceDiscoveryDelegate(nullptr);

Inet::InterfaceId interfaceId =
nodeData.resolutionData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.resolutionData.interfaceId : Inet::InterfaceId::Null();
PeerAddress peerAddress = PeerAddress::UDP(nodeData.resolutionData.ipAddress[0], port, interfaceId);
CHIP_ERROR err = Pair(mNodeId, peerAddress);
auto interfaceId = resolutionData.ipAddress[0].IsIPv6LinkLocal() ? resolutionData.interfaceId : Inet::InterfaceId::Null();
auto peerAddress = PeerAddress::UDP(resolutionData.ipAddress[0], port, interfaceId);
err = Pair(mNodeId, peerAddress);
if (CHIP_NO_ERROR != err)
{
SetCommandExitStatus(err);
Expand Down Expand Up @@ -320,6 +330,10 @@ void PairingCommand::OnDeviceAttestationCompleted(chip::Controller::DeviceCommis
chip::Credentials::AttestationVerificationResult attestationResult)
{
// Bypass attestation verification, continue with success
deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device,
chip::Credentials::AttestationVerificationResult::kSuccess);
auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(
device, chip::Credentials::AttestationVerificationResult::kSuccess);
if (CHIP_NO_ERROR != err)
{
SetCommandExitStatus(err);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,12 @@ CHIP_ERROR SetupPayloadGenerateQRCodeCommand::PopulatePayloadTLVFromBytes(SetupP
{
// First clear out all the existing TVL bits from the payload. Ignore
// errors here, because we don't care if those bits are not present.
payload.removeSerialNumber();
(void) payload.removeSerialNumber();

auto existingVendorData = payload.getAllOptionalVendorData();
for (auto & data : existingVendorData)
{
payload.removeOptionalVendorData(data.tag);
(void) payload.removeOptionalVendorData(data.tag);
}

if (tlvBytes.empty())
Expand Down
3 changes: 1 addition & 2 deletions examples/common/websocket-server/WebSocketServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,8 @@ bool WebSocketServer::OnWebSocketMessageReceived(char * msg)
return shouldContinue;
}

CHIP_ERROR WebSocketServer::Send(const char * msg)
void WebSocketServer::Send(const char * msg)
{
std::lock_guard<std::mutex> lock(gMutex);
gMessageQueue.push_back(msg);
return CHIP_NO_ERROR;
}
2 changes: 1 addition & 1 deletion examples/common/websocket-server/WebSocketServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class WebSocketServer : public WebSocketServerDelegate
{
public:
CHIP_ERROR Run(chip::Optional<uint16_t> port, WebSocketServerDelegate * delegate);
CHIP_ERROR Send(const char * msg);
void Send(const char * msg);

bool OnWebSocketMessageReceived(char * msg) override;

Expand Down
8 changes: 4 additions & 4 deletions examples/placeholder/linux/InteractiveServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ bool InteractiveServer::Command(const chip::app::ConcreteCommandPath & path)

auto valueStr = JsonToString(value);
gInteractiveServerResult.MaybeAddResult(valueStr.c_str());
LogErrorOnFailure(mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str()));
mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str());
gInteractiveServerResult.Reset();
return mIsReady;
}
Expand All @@ -243,7 +243,7 @@ bool InteractiveServer::ReadAttribute(const chip::app::ConcreteAttributePath & p

auto valueStr = JsonToString(value);
gInteractiveServerResult.MaybeAddResult(valueStr.c_str());
LogErrorOnFailure(mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str()));
mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str());
gInteractiveServerResult.Reset();
return mIsReady;
}
Expand All @@ -260,7 +260,7 @@ bool InteractiveServer::WriteAttribute(const chip::app::ConcreteAttributePath &

auto valueStr = JsonToString(value);
gInteractiveServerResult.MaybeAddResult(valueStr.c_str());
LogErrorOnFailure(mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str()));
mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str());
gInteractiveServerResult.Reset();
return mIsReady;
}
Expand All @@ -273,6 +273,6 @@ void InteractiveServer::CommissioningComplete()
Json::Value value = Json::objectValue;
auto valueStr = JsonToString(value);
gInteractiveServerResult.MaybeAddResult(valueStr.c_str());
LogErrorOnFailure(mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str()));
mWebSocketServer.Send(gInteractiveServerResult.AsJsonString().c_str());
gInteractiveServerResult.Reset();
}

0 comments on commit 3593004

Please sign in to comment.