Skip to content

Commit

Permalink
Optionally tag CHIP_ERROR with source location (#8470)
Browse files Browse the repository at this point in the history
* Optionally tag CHIP_ERROR with source location

#### Problem

It can be difficult to identify the specific code that returned an error.

#### Change overview

Added a configuration option, `CHIP_CONFIG_ERROR_SOURCE`. When this is
enabled (and assuming `CHIP_ERROR` is a class, which should eventually
be true for all builds), `…_ERROR_…` constant macros record the source
`__FILE__` and `__LINE__` where they are used, in the `CHIP_ERROR` object.

Unless `CHIP_CONFIG_SHORT_ERROR_STR` is configured, the source file and
line appear in the result of `ErrorStr()`.

Fixes #8340 _Optionally tag CHIP_ERROR with source location_

#### Testing

- revised `src/lib/support/tests/TestErrorStr.cpp`

* Convert python interface

* review

* review

* Modify ReturnErrorOnFailure et al to accept CHIP_ERROR class or integer

* Fix flaky unit test

* restyle
  • Loading branch information
kpschoedel authored Jul 21, 2021
1 parent db0e079 commit 57874d4
Show file tree
Hide file tree
Showing 23 changed files with 3,638 additions and 2,820 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,46 +28,48 @@
#include <support/BytesToHex.h>
#include <support/logging/CHIPLogging.h>

#include <type_traits>

using namespace chip;
using namespace chip::Controller;

static_assert(std::is_same<uint32_t, ChipError::StorageType>::value, "python assumes CHIP_ERROR maps to c_uint32");

extern "C" {
CHIP_ERROR
ChipError::StorageType
pychip_CommissionableNodeController_NewController(chip::Controller::CommissionableNodeController ** outCommissionableNodeCtrl);
CHIP_ERROR
ChipError::StorageType
pychip_CommissionableNodeController_DeleteController(chip::Controller::CommissionableNodeController * commissionableNodeCtrl);

CHIP_ERROR
ChipError::StorageType
pychip_CommissionableNodeController_DiscoverCommissioners(chip::Controller::CommissionableNodeController * commissionableNodeCtrl);
void pychip_CommissionableNodeController_PrintDiscoveredCommissioners(
chip::Controller::CommissionableNodeController * commissionableNodeCtrl);
}

CHIP_ERROR
ChipError::StorageType
pychip_CommissionableNodeController_NewController(chip::Controller::CommissionableNodeController ** outCommissionableNodeCtrl)
{
CHIP_ERROR err = CHIP_NO_ERROR;
*outCommissionableNodeCtrl = new chip::Controller::CommissionableNodeController();
VerifyOrExit(*outCommissionableNodeCtrl != NULL, err = CHIP_ERROR_NO_MEMORY);
exit:
return err;
VerifyOrReturnError(*outCommissionableNodeCtrl != NULL, ChipError::AsInteger(CHIP_ERROR_NO_MEMORY));
return ChipError::AsInteger(CHIP_NO_ERROR);
}

CHIP_ERROR
ChipError::StorageType
pychip_CommissionableNodeController_DeleteController(chip::Controller::CommissionableNodeController * commissionableNodeCtrl)
{
if (commissionableNodeCtrl != NULL)
{
delete commissionableNodeCtrl;
}

return CHIP_NO_ERROR;
return ChipError::AsInteger(CHIP_NO_ERROR);
}

CHIP_ERROR
ChipError::StorageType
pychip_CommissionableNodeController_DiscoverCommissioners(chip::Controller::CommissionableNodeController * commissionableNodeCtrl)
{
return commissionableNodeCtrl->DiscoverCommissioners();
return ChipError::AsInteger(commissionableNodeCtrl->DiscoverCommissioners());
}

void pychip_CommissionableNodeController_PrintDiscoveredCommissioners(
Expand Down
259 changes: 136 additions & 123 deletions src/controller/python/ChipDeviceController-ScriptBinding.cpp

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace Controller {
void ScriptDeviceAddressUpdateDelegate::OnAddressUpdateComplete(NodeId nodeId, CHIP_ERROR error)
{
if (mOnAddressUpdateComplete != nullptr)
mOnAddressUpdateComplete(nodeId, error);
mOnAddressUpdateComplete(nodeId, ChipError::AsInteger(error));
}

} // namespace Controller
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
namespace chip {
namespace Controller {

extern "C" using DeviceAddressUpdateDelegate_OnUpdateComplete = void(*)(NodeId, CHIP_ERROR);
extern "C" using DeviceAddressUpdateDelegate_OnUpdateComplete = void(*)(NodeId, ChipError::StorageType);

class ScriptDeviceAddressUpdateDelegate final : public Controller::DeviceAddressUpdateDelegate
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ void ScriptDevicePairingDelegate::OnPairingComplete(CHIP_ERROR error)
{
if (mOnPairingCompleteCallback != nullptr)
{
mOnPairingCompleteCallback(error);
mOnPairingCompleteCallback(ChipError::AsInteger(error));
}
}

void ScriptDevicePairingDelegate::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR error)
{
if (mOnCommissioningCompleteCallback != nullptr)
{
mOnCommissioningCompleteCallback(nodeId, error);
mOnCommissioningCompleteCallback(nodeId, ChipError::AsInteger(error));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ namespace chip {
namespace Controller {

extern "C" {
typedef void (*DevicePairingDelegate_OnPairingCompleteFunct)(CHIP_ERROR err);
typedef void (*DevicePairingDelegate_OnCommissioningCompleteFunct)(NodeId nodeId, CHIP_ERROR err);
typedef void (*DevicePairingDelegate_OnPairingCompleteFunct)(ChipError::StorageType err);
typedef void (*DevicePairingDelegate_OnCommissioningCompleteFunct)(NodeId nodeId, ChipError::StorageType err);
}

class ScriptDevicePairingDelegate final : public Controller::DevicePairingDelegate
Expand Down
2 changes: 1 addition & 1 deletion src/controller/python/chip/ChipStack.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ def PostTaskOnChipThread(self, callFunct) -> AsyncCallableHandle:
return callObj

def ErrorToException(self, err, devStatusPtr=None):
if err == 4044 and devStatusPtr:
if err == 0x2C and devStatusPtr:
devStatus = devStatusPtr.contents
msg = ChipUtility.CStringToString(
(
Expand Down
5,828 changes: 3,258 additions & 2,570 deletions src/controller/python/chip/clusters/CHIPClusters.cpp

Large diffs are not rendered by default.

10 changes: 7 additions & 3 deletions src/controller/python/chip/discovery/NodeResolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@
#include <platform/CHIPDeviceLayer.h>
#include <support/CodeUtils.h>

#include <type_traits>

using namespace chip;
using namespace chip::Mdns;

static_assert(std::is_same<uint32_t, ChipError::StorageType>::value, "python assumes CHIP_ERROR maps to c_uint32");

namespace {

// callback types shared with python code (see ptyhon code in chip.discovery.types)
using DiscoverSuccessCallback = void (*)(uint64_t fabricId, uint64_t nodeId, uint32_t interfaceId, const char * ip, uint16_t port);
using DiscoverFailureCallback = void (*)(uint64_t fabricId, uint64_t nodeId, CHIP_ERROR error_code);
using DiscoverFailureCallback = void (*)(uint64_t fabricId, uint64_t nodeId, ChipError::StorageType error_code);

constexpr uint16_t kMdnsPort = 5353;

Expand Down Expand Up @@ -58,7 +62,7 @@ class PythonResolverDelegate : public ResolverDelegate
{
if (mFailureCallback != nullptr)
{
mFailureCallback(peerId.GetFabricId(), peerId.GetNodeId(), error);
mFailureCallback(peerId.GetFabricId(), peerId.GetNodeId(), ChipError::AsInteger(error));
}
else
{
Expand Down Expand Up @@ -86,7 +90,7 @@ extern "C" void pychip_discovery_set_callbacks(DiscoverSuccessCallback success,
gPythonResolverDelegate.SetFailureCallback(failure);
}

extern "C" int32_t pychip_discovery_resolve(uint64_t fabricId, uint64_t nodeId)
extern "C" ChipError::StorageType pychip_discovery_resolve(uint64_t fabricId, uint64_t nodeId)
{
CHIP_ERROR result = CHIP_NO_ERROR;

Expand Down
4 changes: 2 additions & 2 deletions src/controller/python/chip/discovery/library_handle.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import chip.native
import ctypes
from ctypes import c_void_p, c_int32, c_uint64
from ctypes import c_void_p, c_int32, c_uint32, c_uint64
from chip.discovery.types import DiscoverSuccessCallback_t, DiscoverFailureCallback_t

def _GetDiscoveryLibraryHandle() -> ctypes.CDLL:
Expand All @@ -33,7 +33,7 @@ def _GetDiscoveryLibraryHandle() -> ctypes.CDLL:
if not handle.pychip_discovery_resolve.argtypes:
setter = chip.native.NativeLibraryHandleMethodArguments(handle)

setter.Set('pychip_discovery_resolve', c_int32, [c_uint64, c_uint64])
setter.Set('pychip_discovery_resolve', c_uint32, [c_uint64, c_uint64])
setter.Set('pychip_discovery_set_callbacks', None, [DiscoverSuccessCallback_t, DiscoverFailureCallback_t])

return handle
11 changes: 7 additions & 4 deletions src/controller/python/chip/interaction_model/Delegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,15 @@ PythonInteractionModelDelegate & PythonInteractionModelDelegate::Instance()

extern "C" {

CHIP_ERROR pychip_InteractionModel_GetCommandSenderHandle(uint64_t * commandSender)
static_assert(std::is_same<uint32_t, chip::ChipError::StorageType>::value, "python assumes CHIP_ERROR maps to c_uint32");

chip::ChipError::StorageType pychip_InteractionModel_GetCommandSenderHandle(uint64_t * commandSender)
{
chip::app::CommandSender * commandSenderObj = nullptr;
VerifyOrReturnError(commandSender != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->NewCommandSender(&commandSenderObj));
VerifyOrReturnError(commandSender != nullptr, chip::ChipError::AsInteger(CHIP_ERROR_INVALID_ARGUMENT));
CHIP_ERROR err = chip::app::InteractionModelEngine::GetInstance()->NewCommandSender(&commandSenderObj);
VerifyOrReturnError(err == CHIP_NO_ERROR, chip::ChipError::AsInteger(err));
*commandSender = reinterpret_cast<uint64_t>(commandSenderObj);
return CHIP_NO_ERROR;
return chip::ChipError::AsInteger(CHIP_NO_ERROR);
}
}
12 changes: 7 additions & 5 deletions src/controller/python/chip/internal/CommissionerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,11 @@ extern "C" chip::Controller::DeviceCommissioner * pychip_internal_Commissioner_N
return result.release();
}

static_assert(std::is_same<uint32_t, chip::ChipError::StorageType>::value, "python assumes CHIP_ERROR maps to c_uint32");

/// Returns CHIP_ERROR corresponding to an UnpairDevice call
extern "C" uint32_t pychip_internal_Commissioner_Unpair(chip::Controller::DeviceCommissioner * commissioner,
uint64_t remoteDeviceId)
extern "C" chip::ChipError::StorageType pychip_internal_Commissioner_Unpair(chip::Controller::DeviceCommissioner * commissioner,
uint64_t remoteDeviceId)
{
CHIP_ERROR err;

Expand All @@ -134,9 +136,9 @@ extern "C" uint32_t pychip_internal_Commissioner_Unpair(chip::Controller::Device
return chip::ChipError::AsInteger(err);
}

extern "C" uint32_t pychip_internal_Commissioner_BleConnectForPairing(chip::Controller::DeviceCommissioner * commissioner,
uint64_t remoteNodeId, uint32_t pinCode,
uint16_t discriminator)
extern "C" chip::ChipError::StorageType
pychip_internal_Commissioner_BleConnectForPairing(chip::Controller::DeviceCommissioner * commissioner, uint64_t remoteNodeId,
uint32_t pinCode, uint16_t discriminator)
{

CHIP_ERROR err;
Expand Down
11 changes: 7 additions & 4 deletions src/controller/python/chip/native/StackInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,18 @@ void * PlatformMainLoop(void *)

extern "C" {

CHIP_ERROR pychip_BLEMgrImpl_ConfigureBle(uint32_t bluetoothAdapterId)
static_assert(std::is_same<uint32_t, chip::ChipError::StorageType>::value, "python assumes CHIP_ERROR maps to c_uint32");

chip::ChipError::StorageType pychip_BLEMgrImpl_ConfigureBle(uint32_t bluetoothAdapterId)
{
#if CHIP_DEVICE_LAYER_TARGET_LINUX && CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE
// By default, Linux device is configured as a BLE peripheral while the controller needs a BLE central.
sBluetoothAdapterId = bluetoothAdapterId;
ReturnErrorOnFailure(
chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureBle(/* BLE adapter ID */ bluetoothAdapterId, /* BLE central */ true));
CHIP_ERROR err =
chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureBle(/* BLE adapter ID */ bluetoothAdapterId, /* BLE central */ true);
VerifyOrReturnError(err == CHIP_NO_ERROR, chip::ChipError::AsInteger(err));
#endif
return CHIP_NO_ERROR;
return chip::ChipError::AsInteger(CHIP_NO_ERROR);
}

void pychip_native_init()
Expand Down
19 changes: 12 additions & 7 deletions src/controller/python/chip/setup_payload/Generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@
#include <support/CodeUtils.h>

#include <string>
#include <type_traits>

using namespace chip;

extern "C" CHIP_ERROR pychip_SetupPayload_PrintOnboardingCodes(uint32_t passcode, uint16_t vendorId, uint16_t productId,
uint16_t discriminator, uint8_t customFlow, uint8_t capabilities,
uint8_t version)
static_assert(std::is_same<uint32_t, ChipError::StorageType>::value, "python assumes CHIP_ERROR maps to c_uint32");

extern "C" ChipError::StorageType pychip_SetupPayload_PrintOnboardingCodes(uint32_t passcode, uint16_t vendorId, uint16_t productId,
uint16_t discriminator, uint8_t customFlow,
uint8_t capabilities, uint8_t version)
{
std::string QRCode;
std::string manualPairingCode;
Expand All @@ -52,14 +55,16 @@ extern "C" CHIP_ERROR pychip_SetupPayload_PrintOnboardingCodes(uint32_t passcode
break;
default:
ChipLogError(SetupPayload, "Invalid Custom Flow");
return CHIP_ERROR_INVALID_ARGUMENT;
return ChipError::AsInteger(CHIP_ERROR_INVALID_ARGUMENT);
}

ReturnErrorOnFailure(ManualSetupPayloadGenerator(payload).payloadDecimalStringRepresentation(manualPairingCode));
CHIP_ERROR err = ManualSetupPayloadGenerator(payload).payloadDecimalStringRepresentation(manualPairingCode);
VerifyOrReturnError(err == CHIP_NO_ERROR, ChipError::AsInteger(err));
ChipLogProgress(SetupPayload, "Manual pairing code: [%s]", manualPairingCode.c_str());

ReturnErrorOnFailure(QRCodeSetupPayloadGenerator(payload).payloadBase38Representation(QRCode));
err = QRCodeSetupPayloadGenerator(payload).payloadBase38Representation(QRCode);
VerifyOrReturnError(err == CHIP_NO_ERROR, ChipError::AsInteger(err));
ChipLogProgress(SetupPayload, "SetupQRCode: [%s]", QRCode.c_str());

return CHIP_NO_ERROR;
return ChipError::AsInteger(CHIP_NO_ERROR);
}
22 changes: 14 additions & 8 deletions src/controller/python/chip/setup_payload/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@
#include <support/CodeUtils.h>

#include <string>
#include <type_traits>

using namespace chip;

static_assert(std::is_same<uint32_t, ChipError::StorageType>::value, "python assumes CHIP_ERROR maps to c_uint32");

namespace {

using AttributeVisitor = void (*)(const char * attrName, const char * attrValue);
Expand Down Expand Up @@ -56,22 +59,25 @@ void YieldSetupPayloadAttributes(const SetupPayload & payload, AttributeVisitor

} // namespace

extern "C" CHIP_ERROR pychip_SetupPayload_ParseQrCode(const char * qrCode, AttributeVisitor attrVisitor,
VendorAttributeVisitor vendorAttrVisitor)
extern "C" ChipError::StorageType pychip_SetupPayload_ParseQrCode(const char * qrCode, AttributeVisitor attrVisitor,
VendorAttributeVisitor vendorAttrVisitor)
{
SetupPayload payload;
ReturnErrorOnFailure(QRCodeSetupPayloadParser(qrCode).populatePayload(payload));
CHIP_ERROR err = QRCodeSetupPayloadParser(qrCode).populatePayload(payload);
VerifyOrReturnError(err == CHIP_NO_ERROR, ChipError::AsInteger(err));

YieldSetupPayloadAttributes(payload, attrVisitor, vendorAttrVisitor);
return CHIP_NO_ERROR;
return ChipError::AsInteger(CHIP_NO_ERROR);
}

extern "C" CHIP_ERROR pychip_SetupPayload_ParseManualPairingCode(const char * manualPairingCode, AttributeVisitor attrVisitor,
VendorAttributeVisitor vendorAttrVisitor)
extern "C" ChipError::StorageType pychip_SetupPayload_ParseManualPairingCode(const char * manualPairingCode,
AttributeVisitor attrVisitor,
VendorAttributeVisitor vendorAttrVisitor)
{
SetupPayload payload;
ReturnErrorOnFailure(ManualSetupPayloadParser(manualPairingCode).populatePayload(payload));
CHIP_ERROR err = ManualSetupPayloadParser(manualPairingCode).populatePayload(payload);
VerifyOrReturnError(err == CHIP_NO_ERROR, ChipError::AsInteger(err));

YieldSetupPayloadAttributes(payload, attrVisitor, vendorAttrVisitor);
return CHIP_NO_ERROR;
return ChipError::AsInteger(CHIP_NO_ERROR);
}
Loading

0 comments on commit 57874d4

Please sign in to comment.