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

Operational Discovery (CON-1306) #1055

Closed
simonFloat opened this issue Aug 15, 2024 · 10 comments
Closed

Operational Discovery (CON-1306) #1055

simonFloat opened this issue Aug 15, 2024 · 10 comments

Comments

@simonFloat
Copy link

How would I perform operational discovery?
I was look at the DiscoveryImplPlatform class which seems to have this functionality. I'm however unsure if there is a better higher level functionality to perform operational discovery. Moreover I get the error
undefined reference to chip::Dnssd::DiscoveryImplPlatform::GetInstance(), and it seems the file is not included when compiling? Other files in the same directory seems to be included so I am a bit confused.

@github-actions github-actions bot changed the title Operational Discovery Operational Discovery (CON-1306) Aug 15, 2024
@simonFloat
Copy link
Author

I have tried to use the chip::Dossed::Resolver instead as it seems the DiscoveryImplPlatform is only for linux?

I have a device advertising as an operational node:

I (16764) chip[DIS]: Advertise operational node 83EAA9679C4F929D-0000000000000010
I (16764) chip[DIS]: CHIP minimal mDNS configured as 'Operational device'; instance name: 83EAA9679C4F929D-0000000000000010.
I (16784) chip[DIS]: mDNS service published: _matter._tcp

I then try to start a operational discovery from another device that has been commissioned:

auto &resolver = chip::Dnssd::Resolver::Instance();
if (! resolver.IsInitialized() ){
    ESP_LOGE(TAG, "Discovery platform not initialized! cannot find nodes.");
    return;
}
DiscoveryFilter filter = DiscoveryFilter(DiscoveryFilterType::kCompressedFabricId, 1);
DiscoveryContext *context = new DiscoveryContext();
DiscoverNodeHandler *discoverDelegate = new DiscoverNodeHandler();
context->SetBrowseIdentifier(1234);
context->SetDiscoveryDelegate(discoverDelegate /* My deletegate */);
ESP_LOGI(TAG, "Starting operational discovery...");
CHIP_ERROR err = resolver.StartDiscovery(chip::Dnssd::DiscoveryType::kOperational, filter, *context);   
ESP_LOGE(TAG, "StartDiscovery errorcode: %s", err.AsString());
void DiscoverNodeHandler::OnNodeDiscovered(const DiscoveredNodeData & nodeData){
        
        if (! nodeData.Is<OperationalNodeBrowseData>()){ //TODO handle wrong type
            ESP_LOGE(TAG, "unhandled node data type: %d", nodeData.GetType());
        }
        /* Cast to type */
        OperationalNodeBrowseData opNodeData = (OperationalNodeBrowseData) nodeData.Get<OperationalNodeBrowseData>();
        ESP_LOGI(TAG,"node discovered! Compressed fabric id: %lld, nodeid: %lld", opNodeData.peerId.GetCompressedFabricId(), opNodeData.peerId.GetNodeId());
        opNodeData.LogDetail();
}

I get the output:

I (69615) discover_delegate: node discovered! Compressed fabric id: -8941147847721512291, nodeid: 16
I (71865) discover_delegate: node discovered! Compressed fabric id: 535107329033360275, nodeid: 1886217941
I (71875) discover_delegate: node discovered! Compressed fabric id: 535107329033360275, nodeid: 1886217941
.
.
.
E (85405) chip[DIS]: Timeout waiting for mDNS resolution.
I (106285) ROUTE_HOOK: Ignore invalid ICMP packet
I (107515) ROUTE_HOOK: Received RIO

I have a strong feeling something is wrong, both with the data I receive and the 'ROUTE_HOOK' packets I start go receive. I would be very happy if anyone could help/clarify this for me.

@simonFloat
Copy link
Author

simonFloat commented Aug 15, 2024

Node-id 1886217941 might be my google nest hub, as when I plug out my other device only node-id 16 disappear. However this confuses me as I specified a fabric id as filter, and I am pretty sure they are not on the same fabric (google nest hub and chip-tool). It seems like the filtering does not work correctly?

DiscoveryFilter(DiscoveryFilterType::kCompressedFabricId, 1);

I tried to change this to: DiscoveryFilter(DiscoveryFilterType::kCompressedFabricId, fabricInfo->GetCompressedFabricId());. But I still get responses from other fabrics.

@wqx6
Copy link
Contributor

wqx6 commented Aug 16, 2024

Looks like the Filter doesn't work for minimal mDNS resolver. This might need a fix for upstream repo of connectedhomeip. I am looking into this issue. Thanks for report this issue!

@simonFloat
Copy link
Author

Thanks, can I also ask about the minimal mDNS? as the context for the resolver says:

/**
 * Node discovery context class.
 *
 * This class enables multiple clients of the global DNS-SD resolver to start simultaneous
 * discovery operations.
 *
 * An object of this class is shared between a resolver client and the concrete resolver
 * implementation. The client is responsible for allocating the context and passing it to
 * the resolver when initiating a discovery operation. The resolver, in turn, is supposed to retain
 * the context until the operation is finished. This allows the client to release the ownership of
 * the context at any time without putting the resolver at risk of using a deleted object.
 */

But in the minimal mDNS resolver StartDiscovery() it says:

 // minmdns currently supports only one discovery context at a time so override the previous context
    SetDiscoveryContext(&context);

What exactly does this mean / what are the effects of this. Do I have to be careful when I perform my own discovery, so that I do not override a context set by another discovery?

@wqx6
Copy link
Contributor

wqx6 commented Aug 19, 2024

What exactly does this mean / what are the effects of this. Do I have to be careful when I perform my own discovery, so that I do not override a context set by another discovery?

That means the resolver can only handle one browsing at a time. You need to stop current discovery when you start a new discovery.

@simonFloat
Copy link
Author

Is it safe to just stop any discovery and is there a safe way to see if a discovery is ongoing?

@wqx6
Copy link
Contributor

wqx6 commented Aug 20, 2024

It is safe the call the stop function even if you don't start any discovery.

@wqx6
Copy link
Contributor

wqx6 commented Aug 20, 2024

project-chip/connectedhomeip#35063 Created a PR to fix this issue, could you try with this patch?

@simonFloat
Copy link
Author

project-chip/connectedhomeip#35063 Created a PR to fix this issue, could you try with this patch?

Yes, fixes the issue for me.

I couldn't build when switching to the PR, some errors occurred from some esp32 component delegate, so I wrote your changes in manually to test it. I just wanted to be sure that this might be due to the compliance of the esp32-matter to the version of the upstream repo.

@wqx6
Copy link
Contributor

wqx6 commented Aug 21, 2024

I just wanted to be sure that this might be due to the compliance of the esp32-matter to the version of the upstream repo.

Yes, the esp-matter should be updated if you update the upstream repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants