Zigbee binding

chris · February 18, 2019, 5:38pm

Probably you have a mismatch of versions as I think this class was recently removed.

puzzle-star · February 18, 2019, 5:49pm

Good to hear! . Thanks BTW.

Why do you think it doesn’t ? I think it does, as the TransactionManager, when calling queue.getTransaction() in the loop to poll the queues, is passing a new parameter asking for only “fast” transactions, and in turn the queue is returning null if this parameter is ‘true’ and the next transaction it contains is considered not to be fast.

Unless I coded something wrong, this was the principle I was following (and it is working indeed like this in my network)

Pedro

chris · February 18, 2019, 6:42pm

I think your system will not add any sleepy transactions if the queue size is currently > 5 - is that correct?

This means that if the queue has more than 5 frames, the system will only continue to put non-sleepy frames in the queue - this will slow down frames to sleepy devices, which is also not good, and may cause significant problems with such devices.

Or do I interpret your implementation incorrectly?

puzzle-star · February 18, 2019, 6:51pm

Ah, OK, your concern is about the 5: I just used five to make sure I could see the effects, but it is a configurable setting.

Normal setting would be 1, or 2 if we want to account for the possibility of some devices having gone offline. In the end I think this should end up being a teakable parameter, with a reasonable default.

The implementation is independent on the setting for the thresold.

chris · February 18, 2019, 7:04pm

No, I don’t care about the number, and obviously I can see that this is configurable. My point is that I don’t think it differentiates between sleepy and non-sleepy data, so if the queue fills up with non-sleepy data, and there is a lot of non-sleepy traffic going into the TransactionManager, then I think the implementation would only put non-sleepy frames into the transport queue. This is because it only checks if queue.size > x.

So, if queue.size == 6, then it will not send any sleepy frames, even if there are no sleepy frames in the queue, and it will not send any sleepy frames until the size drops back below 5. It will instead keep the queue full of non-sleepy frames - effectively delaying sleepy frames.

puzzle-star · February 18, 2019, 7:26pm

Ah, I see. I was only considering the queue filling up with slow transactions impacting the fast ones, but I see your point. I had not considererd the opposite case because fast transactions are… fast, but you are right

There needs to be something like a counter of slow transactions already on air, and request “fastOnly” transactions to the queue if “ongoingSlowTransactions >= maxOutstandingTransactions - reservedFastTransactions”.

Would it make sense?

Pedro

chris · February 18, 2019, 8:44pm

Yes, and most of the time your implementation probably works just fine, but if there was a lot of traffic, then we could get into the situation where the queue is always above the threshold, so we only “ever” send the non-sleepy frames.

Yes, that is effectively what I’ve implemented.

chris · February 19, 2019, 11:28pm

@puzzle-star I’ve pushed an update with the sleepy queue handling - please feel free to test it… I’ve only tested it with static test cases as I’m away from home at the moment.

Let me know how it looks

puzzle-star · February 20, 2019, 12:38pm

Hi @chris,

I will check this afternoon. Just a couple of comments (first sight):

I think you have not updated telegesis response timeout to 3s (will check how it goes)
The first time a device is discovered, it may be considered always sleepy, till the next restart, as at the moment of creating the queue maccapabilities are unknown. That is why in my implementation I checked everytime (not efficient).

Regarding this latter one, probably we would need:

queue.isSleepy() return Boolean (default null)
If it returns null, check if the device has already reported the node descriptor:
- If it does not, do we consider it for this transaction sleepy or not?
- If it does, we call setSleepy and act accordingly

Does it make sense?

Pedro

chris · February 20, 2019, 12:57pm

Yes, I did this. It was done in a separate PR for visibility -:

Good point - let’s see how it goes and get can take another look at this.

dbadia · February 20, 2019, 11:11pm

Quick question: does zigbee support unpairing? Or do I just delete the unwanted thing from the OH UI and hard reset the device?

chris · February 20, 2019, 11:12pm

Yes, you can remove a device from the network, but you can also just delete it and reset it. There’s no persistent information held in the coordinator - unlike ZWave.

chris · February 21, 2019, 4:28am

@puzzle-star I’ve pushed an update that should now account for the change to the device type. Again, I’ve not had the chance to test this until I get home at the weekend, but if you get the chance, I welcome any feedback

chris · February 21, 2019, 3:40pm

I’ve created an updated snapshot 1.1.11-SNAPSHOT which includes the latest changes to the transaction manager if anyone fancies giving it a try…

jfrank1845 · February 21, 2019, 6:07pm

I would just need to use the transaction queue branch to get this ? Also curious where we were landing with the whole channel definition thing - I noticed the conversation kindof halted on github.

chris · February 21, 2019, 6:09pm

If you use @5ivers magic download script, then you should just need to specify the version as above and you should get the latest version I compiled earlier.

Was that the the issue about color mode, and UNDEF states etc?

5iver · February 21, 2019, 6:20pm

Just an FYI… the latest version is populated for you as the default, based on the option that was selected (this one being the Zigbee library snapshot).

I just grabbed the latest.

jfrank1845 · February 21, 2019, 7:47pm

Yeah. Going to undef for color or ct when in the opposite mode can work - though you then lose visibility to the current brightness if you are in CT mode - if we added a brightness channel back it would allow us to see everything as the undef would tell you if the bulb was color or ct. The only drawback of that approach is that you wouldn’t be able to switch the bulb back to the previous color state when it was in ct mode - unless sending an undef to ct would cause it to flip back to previous state?

puzzle-star · February 22, 2019, 6:34am

Hi @chris,

I am testing now. But at first sight, is it possible that we need this, to prevent the sleepyTransactions counter from getting out of sync if there are transactions in flight when the node is updated and we change the queue type?

/* Removed code as it has synchronization issues */
/* Will update as soon as I test the new approach */

Pedro

chris · February 22, 2019, 1:37pm

Yes, good point. I’d tried to avoid having to reset this counter, but you’re right, this can probably get out of sync if the queue type changes.

I will try and look at this this afternoon, but I’m travelling homeward again so not sure if I’ll have time today.