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

feat(split): sync central & peripherals last activity timing #2459

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

angweekiat
Copy link
Contributor

@angweekiat angweekiat commented Sep 5, 2024

Sync central last activity timings to all devices, by having the central emit how long it's inactive, and the peripheral(s) using it to adjust the local last activity time. Prior to this change, key presses on a peripheral keep the central awake, but not vice versa.

This is done by:

  1. Adding a new SYNC_ACTIVITY GATT characteristic for central to sync its inactive time to the other peripheral(s).
  2. Central's activity.c broadcasting its inactivity time to its peripherals via 2 options:
    a. ZMK_SPLIT_SYNC_LAST_ACTIVITY_TIMING_PERIODIC - broadcasting at a regular configurable interval
    b. ZMK_SPLIT_SYNC_LAST_ACTIVITY_TIMING_ON_EVENT - broadcasting when key press / sensors events are received, with a minimum configurable interval between each broadcast
  3. Peripheral's service.c receiving the inactivity time and sending it as an event to its activity.c
  4. Peripheral's activity.c determining its local activity_last_uptime value using the inactivity time as a relative difference.

Additionally:

  • central.c is updated to sync the timers upon a new BLE connection, so new devices don't need to wait for the full interval to get synced.
  • If a peripheral is in IDLE mode when the central comes online, it'll get synced to ACTIVE as well.

Cons:

  • This solution doesn't handle cases where the peripheral(s) is already in deep sleep (ZMK_ACTIVITY_SLEEP), since BLE is turned off. However, the sync-ing can prevent peripheral(s) from going into deep sleep if the central is used.
  • The original intention of periodic broadcast is to prevent unnecessary deep sleeps, not idle, and the default interval was 5 minutes. However, if the interval is greater than the idle time out (ZMK_IDLE_SLEEP_TIMEOUT), the peripheral(s) may go into IDLE before receiving the next sync from central, which may not be desired. See Test steps - Periodic sync below

Test steps - Periodic sync

  1. Build and flash both sides to the target keyboard, ideally with indicators such as backlight, battery, RGB underglow, displays, to detect when one side has gone to idle/active, and with ZMK_SPLIT_SYNC_LAST_ACTIVITY_TIMING_PERIODIC enabled
  2. Allow the peripheral to go to idle state (default timeout: 30s)
  3. Press any key on the central that causes it to switch to active. The peripheral should go to active state, not immediately, but within 30s.

Test steps - Event-based sync

  1. Build and flash both sides to the target keyboard, ideally with indicators such as backlight, battery, RGB underglow, displays, to detect when one side has gone to idle/active, and with ZMK_SPLIT_SYNC_LAST_ACTIVITY_TIMING_ON_EVENT enabled. (_ON_EVENT, not _PERIODIC)
  2. Allow the peripheral to go to idle state (default timeout: 30s)
  3. Press any key on the central that causes it to switch to active. The peripheral should go to active state immediately.

Local test

I added log lines to check the output, that the values are sent and received correctly on both ends. However, if the sync interval is set to a low value (30 seconds), the log lines would end up spamming the console, so I opt'ed to remove them. Let me know if logs can be introduced more nicely, or if there's a better way to check that the code works for the context of reviewing this pull request!

Raised as an issue in #2408

@angweekiat angweekiat requested a review from a team as a code owner September 5, 2024 15:35
@caksoylar caksoylar added core Core functionality/behavior of ZMK split labels Sep 5, 2024
@angweekiat
Copy link
Contributor Author

angweekiat commented Sep 6, 2024

Shifted original comment into PR description

@angweekiat angweekiat force-pushed the feat/sync-sleep-timers-between-central-and-peripherals branch from 85309df to fad0f80 Compare September 8, 2024 07:07
Sync central last activity timings to all devices, by having the
central emit how long it's inactive for, and the peripheral(s) using it
to adjust the local last activity time.

Prior to this commit, key presses on a peripheral keeps the central
awake, but not vice versa. With this commit, key presses on the central
(or hypothetical peripheral B) can keep peripheral A awake.

This is done by:
1. Adding a new `SYNC_ACTIVITY` GATT characteristic for central to sync
it's inactive timer to the other peripheral(s).
2. Central's `activity.c` broadcasting the inactive time at a regular
`ZMK_SPLIT_SYNC_SLEEP_TIMERS_INTERVAL_MS` interval
3. Peripheral's `service.c` receiving the data and sending it to its
`activity.c`
4. Peripheral's `activity.c` determining the new `activity_last_uptime`
value using the inactive duration as a relative difference.

Additionally:
- `central.c` is updated to sync the timers upon a new BLE connection,
so that new devices don't need to wait the full interval to get sync-ed.
- If a peripheral is in IDLE mode when the central comes online, it'll
get sync-ed to ACTIVE as well.

Cons:
- This solution doesn't handle cases where the peripheral(s) is already
in deep sleep, since BLE is turned off. However, the sync-ing can
prevent peripheral(s) from going into deep sleep if the central is used.
- This is a time based sync, so it won't be as accurate as event based
where each key press is sync-ed to the peripheral(s), but that would
take up more of the channel bandwidth.
- If `ZMK_SPLIT_SYNC_SLEEP_TIMERS_INTERVAL_MS` > `ZMK_IDLE_TIMEOUT`,
there can be situations where the peripheral(s) goes into IDLE before
receiving the next sync from central. The original default interval was
5 minutes, primarily to prevent unnecessary deep sleeps, but it would
run into the above case.
@angweekiat angweekiat force-pushed the feat/sync-sleep-timers-between-central-and-peripherals branch from fad0f80 to aa2d3aa Compare September 8, 2024 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core functionality/behavior of ZMK split
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants