Parallelize initializing the node descriptor. #42
Conversation
|
I've tested this with over 30 different devices now and not a single one stumbled over it... should be good to go IMO. |
There was a problem hiding this comment.
The idea behind doing this sequentially was:
First fetch the node information, then fetch sequentially each endpoint, walking down the hierarchy.
Fetching the node information parallel is fine, but fetching the endpoints should start once you are done with the node to have all important information like node type etc before initializing and creating the endpoints.
| @@ -266,20 +268,19 @@ ZigbeeReply *ZigbeeNode::readBindingTableEntries() | |||
|
|
|||
| void ZigbeeNode::initNodeDescriptor() | |||
| { | |||
| qCDebug(dcZigbeeNode()) << "Request node descriptor from" << this; | |||
| qCDebug(dcZigbeeNode()) << "Requesting node descriptor from" << this; | |||
| ZigbeeDeviceObjectReply *reply = deviceObject()->requestNodeDescriptor(); | |||
| connect(reply, &ZigbeeDeviceObjectReply::finished, this, [this, reply](){ | |||
| if (reply->error() != ZigbeeDeviceObjectReply::ErrorNoError) { | |||
| qCWarning(dcZigbeeNode()) << "Error occured during initialization of" << this << "Failed to read node descriptor" << reply->error(); | |||
| m_requestRetry++; | |||
There was a problem hiding this comment.
I think this is messing up the retry mechanism m_requestRetry. If you use a separate variable for each request type I think we should be fine doing it parallel.
|
Also: assuming the endpoints initialization will be finished and the node decriptor is still trying to fetch information, you get in the upper stack the node initialized signal, but you might still not know if this is a router or end device. Doing it sequentially was assuring that all requests have finished (or timeouted) before the node moves into the initialized state. |
|
Agreed, this is not good enough yet (even though in practice it works totally fine). So there's definitely room for improvement, but needs some more effort. |
This speeds up the node initialisation and so far works for every device I tested. In fact, it seems to improve behavior with sleepy devices.
Is there a known reason why this was done sequentially which would prevent this from merging?