Skip to content

Commit

Permalink
[python/dns-sd] Fix "resolve" command in Python CHIP Controller
Browse files Browse the repository at this point in the history
After project-chip#5956 has been merged, the device controller works as
a Resolver delegate, so no separate DeviceAddressUpdater is
needed. In fact, they interfere which each other.

Switch Python CHIP controller to using the mechanisms
introduced by project-chip#5956, fix a couple of issues and remove no
longer necessary code.
  • Loading branch information
Damian-Nordic committed Apr 15, 2021
1 parent 672cf76 commit cd38710
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 136 deletions.
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/discover/Commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#pragma once

#include "DiscoverCommand.h"
#include <controller/DeviceAddressUpdater.h>
#include <controller/DeviceAddressUpdateDelegate.h>
#include <mdns/Resolver.h>

class Resolve : public DiscoverCommand, public chip::Mdns::ResolverDelegate
Expand Down
3 changes: 1 addition & 2 deletions src/controller/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ static_library("controller") {
"CHIPDevice.h",
"CHIPDeviceController.cpp",
"CHIPDeviceController.h",
"DeviceAddressUpdater.cpp",
"DeviceAddressUpdater.h",
"DeviceAddressUpdateDelegate.h",
"EmptyDataModelHandler.cpp",
]

Expand Down
7 changes: 6 additions & 1 deletion src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,12 @@ void DeviceController::OnNodeIdResolved(NodeId nodeId, const chip::Mdns::Resolve

void DeviceController::OnNodeIdResolutionFailed(NodeId nodeId, CHIP_ERROR error)
{
ChipLogError(Controller, "Error resolving node %" PRIu64 ": %s", ErrorStr(error));
ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error));

if (mDeviceAddressUpdateDelegate != nullptr)
{
mDeviceAddressUpdateDelegate->OnAddressUpdateComplete(nodeId, error);
}
};
#endif // CHIP_DEVICE_CONFIG_ENABLE_MDNS

Expand Down
3 changes: 2 additions & 1 deletion src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@
#include <transport/raw/UDP.h>

#if CHIP_DEVICE_CONFIG_ENABLE_MDNS
#include <controller/DeviceAddressUpdater.h>
#include <controller/DeviceAddressUpdateDelegate.h>
#include <mdns/Resolver.h>
#endif

namespace chip {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,12 @@

#pragma once

#include <mdns/Resolver.h>
#include <support/DLLUtil.h>
#include <transport/raw/MessageHeader.h>

namespace chip {
namespace Controller {

class DeviceController;

/// Callbacks for CHIP device address resolution
class DLL_EXPORT DeviceAddressUpdateDelegate
{
Expand All @@ -35,20 +32,5 @@ class DLL_EXPORT DeviceAddressUpdateDelegate
virtual void OnAddressUpdateComplete(NodeId nodeId, CHIP_ERROR error) = 0;
};

/// Class for updating CHIP devices' addresses based on responses from mDNS Resolver
class DLL_EXPORT DeviceAddressUpdater : public Mdns::ResolverDelegate
{
public:
CHIP_ERROR Init(DeviceController * controller, DeviceAddressUpdateDelegate * delegate = nullptr);

private:
// Mdns::ResolverDelegate Implementation
void OnNodeIdResolved(NodeId nodeId, const Mdns::ResolvedNodeData & nodeData) override;
void OnNodeIdResolutionFailed(NodeId nodeId, CHIP_ERROR error) override;

DeviceController * mController = nullptr;
DeviceAddressUpdateDelegate * mDelegate = nullptr;
};

} // namespace Controller
} // namespace chip
65 changes: 0 additions & 65 deletions src/controller/DeviceAddressUpdater.cpp

This file was deleted.

38 changes: 4 additions & 34 deletions src/controller/python/ChipDeviceController-ScriptBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
#include <app/InteractionModelEngine.h>
#include <controller/CHIPDevice.h>
#include <controller/CHIPDeviceController.h>
#include <controller/DeviceAddressUpdater.h>
#include <mdns/Resolver.h>
#include <support/CHIPMem.h>
#include <support/CodeUtils.h>
Expand Down Expand Up @@ -80,8 +79,7 @@ chip::NodeId kRemoteDeviceId = chip::kTestDeviceNodeId;
extern "C" {
CHIP_ERROR pychip_DeviceController_NewDeviceController(chip::Controller::DeviceCommissioner ** outDevCtrl,
chip::NodeId localDeviceId);
CHIP_ERROR pychip_DeviceController_DeleteDeviceController(chip::Controller::DeviceCommissioner * devCtrl,
chip::Controller::DeviceAddressUpdater * addressUpdater);
CHIP_ERROR pychip_DeviceController_DeleteDeviceController(chip::Controller::DeviceCommissioner * devCtrl);
CHIP_ERROR
pychip_DeviceController_GetAddressAndPort(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeId, char * outAddress,
uint64_t maxAddressLen, uint16_t * outPort);
Expand All @@ -103,10 +101,6 @@ CHIP_ERROR
pychip_ScriptDevicePairingDelegate_SetKeyExchangeCallback(chip::Controller::DeviceCommissioner * devCtrl,
chip::Controller::DevicePairingDelegate_OnPairingCompleteFunct callback);

// Discovery
CHIP_ERROR pychip_DeviceAddressUpdater_New(chip::Controller::DeviceAddressUpdater ** outAddressUpdater,
chip::Controller::DeviceCommissioner * devCtrl);
void pychip_DeviceAddressUpdater_Delete(chip::Controller::DeviceAddressUpdater * addressUpdater);
void pychip_ScriptDeviceAddressUpdateDelegate_SetOnAddressUpdateComplete(
chip::Controller::DeviceAddressUpdateDelegate_OnUpdateComplete callback);
CHIP_ERROR pychip_Resolver_ResolveNode(uint64_t fabricid, chip::NodeId nodeid);
Expand All @@ -131,9 +125,8 @@ CHIP_ERROR pychip_DeviceController_NewDeviceController(chip::Controller::DeviceC
chip::NodeId localDeviceId)
{
CHIP_ERROR err = CHIP_NO_ERROR;
ControllerInitParams initParams{
.storageDelegate = &sStorageDelegate,
};
ControllerInitParams initParams{ .storageDelegate = &sStorageDelegate,
.mDeviceAddressUpdateDelegate = &sDeviceAddressUpdateDelegate };

*outDevCtrl = new chip::Controller::DeviceCommissioner();
VerifyOrExit(*outDevCtrl != NULL, err = CHIP_ERROR_NO_MEMORY);
Expand Down Expand Up @@ -164,8 +157,7 @@ CHIP_ERROR pychip_BLEMgrImpl_ConfigureBle(uint32_t bluetoothAdapterId)
return CHIP_NO_ERROR;
}

CHIP_ERROR pychip_DeviceController_DeleteDeviceController(chip::Controller::DeviceCommissioner * devCtrl,
chip::Controller::DeviceAddressUpdater * addressUpdater)
CHIP_ERROR pychip_DeviceController_DeleteDeviceController(chip::Controller::DeviceCommissioner * devCtrl)
{
if (devCtrl != NULL)
{
Expand Down Expand Up @@ -285,28 +277,6 @@ pychip_ScriptDevicePairingDelegate_SetKeyExchangeCallback(chip::Controller::Devi
return CHIP_NO_ERROR;
}

CHIP_ERROR pychip_DeviceAddressUpdater_New(chip::Controller::DeviceAddressUpdater ** outAddressUpdater,
chip::Controller::DeviceCommissioner * devCtrl)
{
auto addressUpdater = std::make_unique<chip::Controller::DeviceAddressUpdater>();

VerifyOrReturnError(addressUpdater.get() != nullptr, CHIP_ERROR_NO_MEMORY);
ReturnErrorOnFailure(addressUpdater->Init(devCtrl, &sDeviceAddressUpdateDelegate));
ReturnErrorOnFailure(Mdns::Resolver::Instance().SetResolverDelegate(addressUpdater.get()));

*outAddressUpdater = addressUpdater.release();
return CHIP_NO_ERROR;
}

void pychip_DeviceAddressUpdater_Delete(chip::Controller::DeviceAddressUpdater * addressUpdater)
{
if (addressUpdater != nullptr)
{
Mdns::Resolver::Instance().SetResolverDelegate(nullptr);
delete addressUpdater;
}
}

void pychip_ScriptDeviceAddressUpdateDelegate_SetOnAddressUpdateComplete(
chip::Controller::DeviceAddressUpdateDelegate_OnUpdateComplete callback)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

#pragma once

#include <controller/DeviceAddressUpdater.h>
#include <controller/DeviceAddressUpdateDelegate.h>

namespace chip {
namespace Controller {
Expand Down
13 changes: 0 additions & 13 deletions src/controller/python/chip/ChipDeviceCtrl.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,11 @@ def __init__(self, startNetworkThread=True, controllerNodeId=0, bluetoothAdapter
self._InitLib()

devCtrl = c_void_p(None)
addressUpdater = c_void_p(None)
res = self._dmLib.pychip_DeviceController_NewDeviceController(pointer(devCtrl), controllerNodeId)
if res != 0:
raise self._ChipStack.ErrorToException(res)

res = self._dmLib.pychip_DeviceAddressUpdater_New(pointer(addressUpdater), devCtrl)
if res != 0:
raise self._ChipStack.ErrorToException(res)

self.devCtrl = devCtrl
self.addressUpdater = addressUpdater
self._ChipStack.devCtrl = devCtrl

self._Cluster = ChipClusters(self._ChipStack)
Expand Down Expand Up @@ -121,7 +115,6 @@ def HandleAddressUpdateComplete(nodeid, err):

def __del__(self):
if self.devCtrl != None:
self._dmLib.pychip_DeviceAddressUpdater_Delete(self.addressUpdater)
self._dmLib.pychip_DeviceController_DeleteDeviceController(self.devCtrl)
self.devCtrl = None

Expand Down Expand Up @@ -252,12 +245,6 @@ def _InitLib(self):
self._dmLib.pychip_ScriptDevicePairingDelegate_SetKeyExchangeCallback.argtypes = [c_void_p, _DevicePairingDelegate_OnPairingCompleteFunct]
self._dmLib.pychip_ScriptDevicePairingDelegate_SetKeyExchangeCallback.restype = c_uint32

self._dmLib.pychip_DeviceAddressUpdater_New.argtypes = [POINTER(c_void_p), c_void_p]
self._dmLib.pychip_DeviceAddressUpdater_New.restype = c_uint32

self._dmLib.pychip_DeviceAddressUpdater_Delete.argtypes = [c_void_p]
self._dmLib.pychip_DeviceAddressUpdater_Delete.restype = None

self._dmLib.pychip_ScriptDeviceAddressUpdateDelegate_SetOnAddressUpdateComplete.argtypes = [_DeviceAddressUpdateDelegate_OnUpdateComplete]
self._dmLib.pychip_ScriptDeviceAddressUpdateDelegate_SetOnAddressUpdateComplete.restype = None

Expand Down

0 comments on commit cd38710

Please sign in to comment.