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

Add timestamp to duplicate command detection #783

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dschall
Copy link
Contributor

@dschall dschall commented Oct 18, 2022

This change keeps track of the timestamp of the last transaction sent by a Tuya wireless button.
Sometimes, these devices send duplicate commands, and we need to ignore these duplicates.
After some inactivity, the transaction ids restart at 1, so we also need to track the timestamp of the last transaction and ignore only those duplicates that happened in a short time (currently set to 5 minutes).

@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/adding-support-for-tuya-zs3l-tz3000-ts0043-ts0042-ts0041-to-zigbee-bindings/108895/61

@cdjackson
Copy link
Contributor

I'm a bit surprised that this is set to minutes. The zigbee library handles duplicate transactions, but that is normally caused by APS retries so it's always in the millisecond range (it's the normal requirement in the APS layer).

I assume this is a bug in the device itself if it's resending the same command after 5 minutes. I guess my other question is how reliable this is, how often it happens, and if the consequences of incorrectly dropping a frame here is worse that the consequences of the original duplicate issue?

@dschall
Copy link
Contributor Author

dschall commented Oct 18, 2022

I guess we can lower it to 1 minute or probably even less. The times I've seen it it was happening right after each other, maybe a second or a few seconds apart.
Could this be a symptom of a different issue where the device does not receive the ACK the first time and resends the tx?
Sorry, not too familiar with Zigbee...

Now that I have a dedicated Zigbee controller only connected to one Tuya button, I can certainly monitor the communication for a while and check what's going on there.

@cdjackson
Copy link
Contributor

The times I've seen it it was happening right after each other, maybe a second or a few seconds apart.

That's a little strange since the current system should filter these out. Can you post some logs of these duplicates please - I want to make sure that the existing mechanism is working ok.

Could this be a symptom of a different issue where the device does not receive the ACK the first time and resends the tx?

Yes, that's the reason for the APS duplicate timer that is implemented in the library. That has a timeout of 5 seconds, which should cover most APS retries. I forget why 5 seconds was used - I can't remember if there's a requirement in the protocol for this or if it just seemed like a good number on the day :)

You could try a search for APS Data: Duplicate frame which is logged by the library when this happens. In theory we can change this time in the library (although I'd need to see if it's accessible - possibly it's not, but that can be changed reasonably easily if necessary).

Now that I have a dedicated Zigbee controller only connected to one Tuya button, I can certainly monitor the communication for a while and check what's going on there.

I think before merging this it would be good to quantify the problem so that it's understood a little better - then we can see if this is the right approach, or if we can modify/extend the existing mechanism.

@dschall
Copy link
Contributor Author

dschall commented Oct 18, 2022

These are my log settings right now (omitting unrelated log settings):

ROOT                                               │ WARN
com.zsmartsystems.zigbee                           │ DEBUG
com.zsmartsystems.zigbee.dongle.ember.internal.ash │ INFO
org.openhab                                        │ INFO
org.openhab.binding.zigbee                         │ DEBUG

Does that cover the data you'd like to see?

@cdjackson
Copy link
Contributor

Yes, that should be fine - the package is com.zsmartsystems.zigbee.aps so that should be at DEBUG based on this.

Thanks.

@dschall
Copy link
Contributor Author

dschall commented Dec 7, 2022

Hey @cdjackson , I finally captured a duplicate command.
It happend after the Tuya device ran out of battery and was left offline for a few days.
When I replaced the batteries and pushed a button, I got two events on OH, see log attached (timestamp 16:27:21.737, look for "TuyaButtonPressCommand")

duplicate command after reconnect.txt

@dschall
Copy link
Contributor Author

dschall commented Dec 7, 2022

PS: These Tuya thingys have other issues, mine eat through batteries like crazy.
I'll follow up on the thread with more logs:
https://community.openhab.org/t/adding-support-for-tuya-zs3l-tz3000-ts0043-ts0042-ts0041-to-zigbee-bindings/108895/61

@cdjackson
Copy link
Contributor

I can't see any duplicate commands here -:

image

What command is duplicated? Is it shown here or am I missing something somewhere?

@cdjackson
Copy link
Contributor

Sorry - I should have read your message better ;)

@cdjackson
Copy link
Contributor

So these commands are not considered duplicated since they are totally different transactions.

@dschall
Copy link
Contributor Author

dschall commented Dec 7, 2022

Yeah, sorry, now that you're pointing it out..
Looks like my change would not even fix that, then.

On the positive side, it never happend during my testing (before the batteries were empty), so maybe we can live with this as a corner case when the buttons are pressed while the device is rejoining after being offline for a few days.
I'll collect logs about the battery drain. I noticed the Tuya device is asking for the TIME cluster every 10 minutes, and it's not implemented on OH side (I think, not sure if I'm reading the logs right).

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

Successfully merging this pull request may close these issues.

3 participants