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

device: Expose "sleepy" configuration #453

Merged
merged 1 commit into from
Nov 14, 2021
Merged

Conversation

kennylevinsen
Copy link
Contributor

When set, all endpoints sends will default to sendWhenActive: true,
avoiding the need for manually setting this for every request.

@kennylevinsen
Copy link
Contributor Author

An earlier implementation of this embedded the heuristic for enabling sleepy mode (battery-powered end device), but I thought it made sense to merge the manual version first - we want to be able to overrule regardless.

@Koenkk
Copy link
Owner

Koenkk commented Nov 11, 2021

I guess this changes after we choose to go for #445 (comment) ?

@kennylevinsen
Copy link
Contributor Author

We can have the interview flip this setting for all the endpoints. Is it guaranteed that a device object will go through interview after being created/read from database? If so, we could remove the database record and just have interview be responsible for flipping the bit.

@kennylevinsen kennylevinsen marked this pull request as draft November 12, 2021 07:18
@Koenkk
Copy link
Owner

Koenkk commented Nov 12, 2021

@kennylevinsen

  • Is it guaranteed that if the device supports genPollCtrl, sendWhenActive is not needed (can be false)?
  • Instead of introducing a new term; maybe defaultSendWhenActive instead of sleepy is better
  • In case we do not set sleepy in the interview, where do you plan to set this from? (from the configure()?)

@kennylevinsen
Copy link
Contributor Author

@kennylevinsen

  • Is it guaranteed that if the device supports genPollCtrl, sendWhenActive is not needed (can be false)?

The opposite. genPollCtrl is the zigbee-official way of powering a sendWhenActive behavior, by having the end device actively call us specifically to empty any request or configuration queues.

As it specifically controls sleepy end device behavior, it should be pretty safe to use as indicator for a device being sleepy.

  • Instead of introducing a new term; maybe defaultSendWhenActive instead of sleepy is better

Will do.

  • In case we do not set sleepy in the interview, where do you plan to set this from? (from the configure()?)

Indeed, known sleepy devices without genPollCtrl may wish to manually set it in configure.

However, we could add the end device + battery powered heuristic as fallback when genPollCtrl isn't there. Not sure if we'll end up needing any overrides in that case.

@Koenkk
Copy link
Owner

Koenkk commented Nov 13, 2021

@kennylevinsen

However, we could add the end device + battery powered heuristic as fallback when genPollCtrl isn't there. Not sure if we'll end up needing any overrides in that case.

Not sure about that, I know that e.g. many Xiaomi end devices like the smoke detector and vibration sensor can be send messages to without sendWhenActive (they do a dataRequest every 30 seconds)

@kennylevinsen
Copy link
Contributor Author

kennylevinsen commented Nov 13, 2021

they do a dataRequest every 30 seconds

I believe that is a good example of where sendWhenActive is necessary, but where chance probably make things work often enough to not notice.

The parent will keep 1 packet for the device for 7.68 seconds. If the device long polls at 30s intervals, then delivery failure is guaranteed if:

  1. More than one packet is sent in a 30s timespan.
  2. Any packet is sent less than 22.32 seconds after the device polled last. If we add 10 seconds of retry logic, this becomes 12.32 seconds, or slightly more than 1/3rd risk.

Occasional fast poll sessions drop the risk a bit depending on how often they happen, so maybe 1/4th average risk of failure due to request timing.

So I believe that those Xiaomi end devices should be using sendWhenActive. The updated version that tries a plain send first is very unintrusive though, and effectively only changes behavior it is needed.

@kennylevinsen
Copy link
Contributor Author

Added tests for having interview respect a user setting - we'll probably need the user setting regardless of whether we add a fallback heuristic later.

Ready for review.

When set, all endpoints sends will default to sendWhenActive: true,
avoiding the need for manually setting this for every request.
@Koenkk
Copy link
Owner

Koenkk commented Nov 14, 2021

@kennylevinsen sounds good indeed

@Koenkk Koenkk merged commit 5149d17 into Koenkk:master Nov 14, 2021
@kennylevinsen kennylevinsen deleted the sleepy branch November 14, 2021 15:23
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.

None yet

2 participants