use DataRequest instead of DataRequestExt if possible

[dumpfheimer]

According to Docs the DataRequestExt is sequential:

"And any AF_DATA_REQUEST_EXT with a huge data byte count must be completed (or timed-out) before another will be started"

This PR would use the normal data request instead of the extended data request if possible.

[puddly]

The two commands actually lead to the exact same function (ti/zstack/mt/mt_af.c):

    case MT_AF_DATA_REQUEST:
    case MT_AF_DATA_REQUEST_EXT:
      MT_AfDataRequest(pBuf);
      break;

The sequential behavior you mention seems to be triggered when the length of the packet exceeds 230 bytes for _ext and 240 bytes for the normal request and requires a second command to be sent (MT_AF_DATA_STORE) to do something to send it. I believe this is Z-Stack handling fragmentation.

Do you ever see packets larger than 230 bytes at runtime? I believe even with OTA running, the maximum packet length is never greater than 80 bytes. It may be better to guard against this codepath with an explicit if len(data) > 230: raise ValueError("Packet is too large to send without fragmentation") before even trying to send it.

[dumpfheimer]

True, they do 😮‍💨. Inside that function they do differetiate between the two requests, to be fair, but it seems to not make a noticable difference anyways.

But while looking at the code I found something interesting. It seems like DataRequest can be sent as both, synchronous and asynchronous request which could greatly reduces time spent in the sync lock in api.py

Do you have any thoughts about this?

[puddly]

Interesting. My understanding of the request flow is that you queue up a request, Z-Stack responds with the Rsp once it has been sent (or errors out), and then sends back a Callback once a confirmation is received or fails. Is there a more asynchronous way to send requests?

[dumpfheimer]

Not sure if there is a callback later on. This is the end of the MT_AfDataRequest function:

if (MT_RPC_CMD_SREQ == (cmd0 & MT_RPC_CMD_TYPE_MASK))
{
  MT_BuildAndSendZToolResponse(((uint8_t)MT_RPC_CMD_SRSP|(uint8_t)MT_RPC_SYS_AF), cmd1, 1, &retValue);
}

I testet this with changing api.py (added to the beginning of async def request(...) ):

        if type(request) is c.AF.DataRequest.Req and request.Callback is None:
            LOGGER.info("Sending DataRequest as AREQ %s " % type(request))
            request.header.with_type(t.CommandType.AREQ)

This right now ignores the response but I guess it would not be difficult to add a Callback to the firmware.

I think its worth checking this out, toggling a bunch of lights gets really fast especially when using scenes that change not only the state but also color. I think it might be possible to get out even more when turn_on creates a task instead of running asynchronously (maybe HA waits for the first light to finish its state change before it starts with the second?)

[dumpfheimer]

Hmm, not sure if this is the only relevant change atm.

[puddly]

Not waiting for DataConfirm.Callback I think will lead to memory errors. You can send more quickly by ignoring it, but you'll have no indication of receipt (since that's the command that relays it) and no flow control.

Setting the request concurrency to 9999 would have the same effect. If I remember correctly, Z-Stack's API reference notes that concurrency should be capped at one, hence the reason for Req/Rsp/Callback as opposed to just Req/Rsp.

[dumpfheimer]

Do you know if the coordinator returns before or after transmitting the frame?
If it sends the frame before answering, aren't we effectively limited to a single request at a time? Does the max concurrent setting then even make a difference?

Memory could be an issue at some point but that might be managed by reserving memory for n messages. Zigpy coulf load and use that number as max concurrent.

[puddly]

Do you know if the coordinator returns before or after transmitting the frame?

There are two responses:

1. The Rsp: it's sent after the packet is enqueued or no route exists. This should happen almost instantly.
2. The Callback: this is sent after the packet is sent (or ACK'ed, depending on the type).

If it sends the frame before answering, aren't we effectively limited to a single request at a time?

No. zigpy-znp has two concurrency limits:

1. A global synchronous command lock (i.e. you must receive a Rsp before sending another Req). This is limited to a single request, as per the spec.
2. A request concurrency limit, which enforces no more than 16 in-flight requests without a corresponding DataConfirm.Callbacks.

Does the max concurrent setting then even make a difference?

Of course. Try setting max_concurrent_requests: 1 and max_concurrent_requests: 9999 and see what happens to your network as you rapidly send requests.

[dumpfheimer]

Okay, thanks, so turning eg a light on will:

1. Send a message to the controller.
2. The controller will respond with success

Now the controller should send the France OTA and receive an ACK from the light

3. The controller sends a callback message to zigpy

Are only 1 and 2 in the sync lock? Or is 3 blocking it?

[puddly]

No, 3 is not blocking it. The device can send Callback messages at any point but the application can't send a second Req message without receiving a Rsp:

The Z-Tool program must send one message at a time and wait for either the expected response message to a timeout before sending the next message or resending the current message.

Just pretend Req/Rsp/Callback are called Req/Ack/Rsp in this case and I think it will make more sense.

[dumpfheimer]

Okay nice, I am starting to understand the bigger picture.
Thanks for taking the time to explain 🙏