[BUG] sleepyTransaction counter in ZigBeeTransactionManager stucked

[berty2000]

It has happened that no transaction for any of the sleeping devices was processed, because the transaction sleepyTransaction counter was no longer decrements.

After investigation, I noticed that a transaction placed in a sleepy queue (in method sendNextTRansaction), once processed:
In the transactionComplete method, when the queue is retrieved using getTransactionQueue, it is no longer marked as sleepy because it becomes the default queue...?...
Therefore, the counter is not decremented.

Another comment, I was surprised to see that there was only a global counter for all sleeping devices, and not a per-device counter. Wouldn’t it be relevant to have a counter per queue?

[cdjackson]

Handling sleepy devices is a pain. Some routers may not have many buffers available, so having too many outstanding messages sent to SEDs can cause them to get lost, so you'd need to be careful about releasing too many messages to SEDs. This is probably why it was implemented as a global counter... That said - this was written a long time ago so I'm happy to be convinced that there's a better way to manage this if you have some tests that show it working better.

[cdjackson]

I'm trying to work out how to process this.... Looking at your bug report, I assume the issue is that a sleepy transaction effectively looses it's "sleepy" state when it times out and is requeued? Is that correct? If so, I agree this sounds like it needs fixing.

However I'm not sure your fix addresses the actual cause - it might work around the problem...

You state above -:

a transaction placed in a sleepy queue (in method sendNextTRansaction), once processed:
In the transactionComplete method, when the queue is retrieved using getTransactionQueue, it is no longer marked as sleepy because it becomes the default queue...?...

But your fix doesn't address this problem. It sounds like it getTransactionQueue is returning the wrong queue, and the queue is no longer a sleepy queue.

Instead, your fix seems to allow the transaction to be placed back into the default queue, but you have added a sleepy field in the transaction itself. I think that this will cause other problems since the wider management of sleepy devices (if I remember correctly!) manages the frames in queues differently.

Does that make sense to you? Clearly you've spent some time looking at this issue so you probably understand it better than me since it's been a few years since this code was written. I think it would be good to discuss this further so we can merge a fix for this.

For reference below is your proposed fix from the linked PR.