Skip to content

optimisation again ;-)#389

Open
RealByron wants to merge 4 commits intoromasku:mainfrom
RealByron:optimisation_again
Open

optimisation again ;-)#389
RealByron wants to merge 4 commits intoromasku:mainfrom
RealByron:optimisation_again

Conversation

@RealByron
Copy link
Copy Markdown
Contributor

@RealByron RealByron commented Apr 4, 2026

Reduce battery wake-ups on end devices

Problem

Battery-powered end devices (e.g. REMOTE_MOES_SWITCH_TS0044) suffered from unnecessary wake-ups after every button event, draining the battery significantly faster than needed.

Two SDK mechanisms caused extra wake-ups after each ZCL report:

  1. Quick data polls (AUTO_QUICK_DATA_POLL_ENABLE): After every af_dataSend(), the SDK schedules 3 additional MAC polls at 250ms intervals via tl_zbNwkQuickDataPollCb. In long-poll mode with deep retention, each of these polls triggers a full wake from deep retention (~3.5 mA for ~8 ms each).

  2. SDK reporting timers (reportingTimerCb): Calling report_handler() in the main loop and in hal_zigbee_notify_attribute_changed() causes the SDK to schedule reporting timers that don't survive deep retention, leading to spurious timer wake-ups.

Before (main branch)

Short press

main_short

Long press

main_long

After (this PR)

Short press

my_short

Long press

my_long

Results

-46% battery consumption on short press
-58% battery consumption on long press

Changes

1. Disable quick data polls in long-poll mode (zigbee_network.c)

hal_zigbee_set_poll_rate_ms() now toggles AUTO_QUICK_DATA_POLL_ENABLE based on the poll rate. When the poll rate exceeds 10 seconds (i.e. the device is in long-poll / idle mode), quick data polls are disabled — eliminating the ~487ms post-report wake-ups.

In fast-poll mode (≤10s, during joining or interview), quick data polls remain enabled for proper responsiveness.

All BDB callbacks now go through hal_zigbee_set_poll_rate_ms() instead of calling zb_setPollRate() directly, ensuring the flag is always in sync.

2. Bypass SDK report timers on battery devices (zigbee_zcl.c, main.c)

For battery devices, hal_zigbee_notify_attribute_changed() now sends reports directly via hal_zigbee_send_report_attr() instead of going through report_handler(). This avoids the SDK scheduling reportingTimerCb timers that cause extra wake-ups after each attribute change.

The periodic report_handler() call in the main loop is also skipped for battery devices, since reports are sent immediately on change.

Router devices continue to use the SDK reporting mechanism unchanged.

3. Keep fast poll during network join (poll_control_cluster.c)

The poll control cluster now extends the fast-poll window while the device has not yet joined a network. This prevents the initial 10-second fast-poll timeout from expiring during steering, which would drop the device to long-poll before the join completes.

Once joined, on_zcl_activity() naturally keeps the device responsive during the Z2M interview by re-entering fast poll on each incoming ZCL message.

4. Minor: fix printf format specifiers (poll_control_cluster.c)

Replace %lu / (unsigned long) with %d / (int) for compatibility with the Telink printf implementation.

Testing

  • All 184 existing tests pass
  • Verified on hardware (REMOTE_MOES_SWITCH_TS0044) with current profiling
  • Network joining and Z2M interview complete successfully
  • Button press/release/long-press events report correctly

Copy link
Copy Markdown
Owner

@romasku romasku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good, and thank you for the explanation in the description. One issue with reporting: please see the comment.

Comment thread src/telink/hal/zigbee_zcl.c Outdated
Comment on lines +244 to +245
// Battery device: send report directly, SDK reporting timers
// don't survive deep retention.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct. Reporting, like timers in general, is a regular RAM state, so it survives deep-retention sleep.

From a battery-use perspective, this version of hal_zigbee_notify_attribute_changed() will transmit every time it is called, without checking whether the value has actually changed, so can actually increase total power usage. It also breaks minInterval (the minimum time between reports) and maxInterval (periodic reporting even when the value has not changed). These parameters are not very useful for buttons, but they do matter for battery reporting. They will also matter if the firmware later supports real sensors, such as temperature, which should respect these reporting parameters.

So, please keep report_handler() for battery devices as well. With a reasonable maxInterval, automatic reporting will happen very infrequently and should not meaningfully impact battery life.

Copy link
Copy Markdown
Contributor Author

@RealByron RealByron Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about having both : direct send for button events and report_handler otherwise ?
I'm worried about dimming binding (ie: Dim Up at long press event and stop at release)

100ms delay is noticeable I think.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dimming is done via commands, not reporting, so it isn't affected by this delay. But I agree with you here, we can eliminate this 100ms delay for multistate value for more response HA automations. Probably the cleanest option is to call hal_zigbee_send_report_attr from switch_cluster and cover_switch_cluster whenever we change cluster->multistate_state.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean Dimming for another device through binding or coordinator. (not quite aware of all of that). anyway I just push a proposal that combines both send behaviours.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe an option on hal_zigbee_notify_attribute_changed like immediate_send could be a cleaner approach....

flash_indicator is done after direct send for better responsiveness
@RealByron RealByron force-pushed the optimisation_again branch from 699616a to 314ea63 Compare April 5, 2026 07:55
@RealByron RealByron force-pushed the optimisation_again branch from 4e32594 to 1c1fe21 Compare April 5, 2026 08:11
@t3hk0d3
Copy link
Copy Markdown

t3hk0d3 commented Apr 5, 2026

I am hyped. Good job!

@RealByron RealByron force-pushed the optimisation_again branch from 141455b to 5afba98 Compare April 5, 2026 19:06
@RealByron
Copy link
Copy Markdown
Contributor Author

Final results :
Short press :
my_short
Long press :
my_long

@RealByron
Copy link
Copy Markdown
Contributor Author

Something else to do?
Why PRs are so long to integrate?

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