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.

Sign up for GitHub

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

[chip-tool] Some CHIP_ERROR returned values are ignored #25486

Merged
merged 1 commit into from
Mar 7, 2023
Merged

[chip-tool] Some CHIP_ERROR returned values are ignored #25486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/clusters/CustomArgument.h
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp
View file
Open in desktop
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
5 changes: 4 additions & 1 deletion examples/chip-tool/commands/interactive/InteractiveCommands.cpp
View file
Open in desktop
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
View file
Open in desktop
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);
}
}
4 changes: 2 additions & 2 deletions examples/chip-tool/commands/payload/SetupPayloadGenerateCommand.cpp
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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();
}