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

Feature: Soft Off #1942

Closed

Conversation

petejohanson
Copy link
Contributor

@petejohanson petejohanson commented Sep 25, 2023

This PR supersedes #1840 after some useful feedback from @joelspadin in #1840 (comment)

This new branch includes all the previous features, and also:

  • Includes a &soft_off behavior that you can trigger from your keymap, and use a keyboard/controllers reset button to wake from.
  • Supports controlling the hold-time-ms of the behavior, so you can require the behavior be pressed for at least that long in order for soft off to be triggered on release.
  • Adds support for multiple "waker" devices. Not really needed yet, but trying to avoid refactors later.

See the docs deploy for this PR for details on the feature and behavior:

Only item that is a nice-to-have for the future would be some circuit diagrams for the possible hardware designs to leverage this, to complement the text descriptions of them. Don't think that should hold up this work though today.

@petejohanson petejohanson added enhancement New feature or request core Core functionality/behavior of ZMK behaviors labels Sep 25, 2023
@petejohanson petejohanson self-assigned this Sep 25, 2023
@petejohanson petejohanson requested a review from a team as a code owner September 25, 2023 04:45
@petejohanson petejohanson force-pushed the features/soft-off-take-2 branch from 78b84c4 to 2645a67 Compare September 25, 2023 04:59
Copy link
Contributor

@caksoylar caksoylar left a comment

Choose a reason for hiding this comment

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

Note that the descriptions of the nodes (like the properties etc.) can go in a table format in a dedicated page under "Configuration" and linked to from here. It is not crucial but it might have been good for consistency.

docs/docs/features/soft-off.md Show resolved Hide resolved
docs/docs/features/soft-off.md Outdated Show resolved Hide resolved
docs/docs/features/soft-off.md Outdated Show resolved Hide resolved
docs/docs/features/soft-off.md Outdated Show resolved Hide resolved
docs/docs/features/soft-off.md Outdated Show resolved Hide resolved
docs/docs/features/soft-off.md Outdated Show resolved Hide resolved
docs/docs/features/soft-off.md Outdated Show resolved Hide resolved
docs/docs/features/soft-off.md Outdated Show resolved Hide resolved
docs/docs/features/soft-off.md Outdated Show resolved Hide resolved
docs/docs/features/soft-off.md Outdated Show resolved Hide resolved
@petejohanson petejohanson force-pushed the features/soft-off-take-2 branch 3 times, most recently from 683a74a to 3870392 Compare October 3, 2023 08:15
@petejohanson petejohanson force-pushed the features/soft-off-take-2 branch from 3870392 to e526122 Compare October 22, 2023 05:23
@petejohanson petejohanson force-pushed the features/soft-off-take-2 branch 2 times, most recently from eb8dea8 to 8af7f22 Compare October 31, 2023 06:17
@petejohanson petejohanson force-pushed the features/soft-off-take-2 branch from 8af7f22 to 77aa402 Compare November 7, 2023 23:45
@wslfung
Copy link

wslfung commented Nov 26, 2023

Testing this on a nice!nanov2 sofle, I haven't been able to turn the right side off. I have mapped a &soft_off on the right half on the default layer, but that only turns off the left side.

I added .locality in behavior_soft_off_driver_api and moved the target_sources to above the if statement in CMakeLists to make the peripheral power off

@wslfung
Copy link

wslfung commented Nov 28, 2023

another issue I found is the idle sleep now doesn't wake up with key presses anymore.

@petejohanson
Copy link
Contributor Author

another issue I found is the idle sleep now doesn't wake up with key presses anymore.

Did you tag your kscan node with wakeup-source properly for your shield?

@wslfung
Copy link

wslfung commented Nov 30, 2023

another issue I found is the idle sleep now doesn't wake up with key presses anymore.

Did you tag your kscan node with wakeup-source properly for your shield?

Does that affect both the soft_off and CONFIG_ZMK_IDLE_SLEEP_TIMEOUT?

And if I were to add it, would it be the sofle. dtsi since that's the shield I'm using?

@petejohanson
Copy link
Contributor Author

another issue I found is the idle sleep now doesn't wake up with key presses anymore.

Did you tag your kscan node with wakeup-source properly for your shield?

Does that affect both the soft_off and CONFIG_ZMK_IDLE_SLEEP_TIMEOUT?

And if I were to add it, would it be the sofle. dtsi since that's the shield I'm using?

It's needed for wake from deep sleep to work when soft off is enabled.

@wslfung
Copy link

wslfung commented Nov 30, 2023

another issue I found is the idle sleep now doesn't wake up with key presses anymore.

Did you tag your kscan node with wakeup-source properly for your shield?

Does that affect both the soft_off and CONFIG_ZMK_IDLE_SLEEP_TIMEOUT?
And if I were to add it, would it be the sofle. dtsi since that's the shield I'm using?

It's needed for wake from deep sleep to work when soft off is enabled.

happy to report that worked as expected after I added wakeup-source in the sofle shield's kscan

@petejohanson petejohanson force-pushed the features/soft-off-take-2 branch 4 times, most recently from 593fbe4 to 251bc6d Compare December 7, 2023 01:24
@petejohanson
Copy link
Contributor Author

Testing this on a nice!nanov2 sofle, I haven't been able to turn the right side off. I have mapped a &soft_off on the right half on the default layer, but that only turns off the left side.

I added .locality in behavior_soft_off_driver_api and moved the target_sources to above the if statement in CMakeLists to make the peripheral power off

Ok, I've just pushed tweaks for this to do this hopefully correctly, including making it soft off the peripheral side ASAP on press, not on release, so it can be invoked sooner before the central goes away. Can you please test this branch as is?

@caksoylar
Copy link
Contributor

A few further notes on a second review.
Also a remark, tying back to https://github.com/zmkfirmware/zmk/pull/1942/files#r1385777170: wakeup-source; seems now required for deep sleep as well, so getting the board/shield changes will follow soon I am guessing. Might be worth an announcement as well?

It's required only if you turn on ZMK_PM_SOFT_OFF, as that enables PM_DEVICE. Which is part of the reason I don't see it as critical to update existing shields in this PR.

Ah OK then, shouldn't be a breaking change in that case.

@petejohanson petejohanson force-pushed the features/soft-off-take-2 branch 5 times, most recently from f62df55 to ab12762 Compare December 10, 2023 07:25
Copy link
Contributor

@caksoylar caksoylar left a comment

Choose a reason for hiding this comment

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

Fixed a typo on the last update, otherwise docs look good to me.

docs/docs/config/kscan.md Outdated Show resolved Hide resolved
app/boards/shields/zmk_uno/boards/nrf52840dk_nrf52840.conf Outdated Show resolved Hide resolved
@@ -1,6 +1,16 @@
# Copyright (c) 2023 The ZMK Contributors
# SPDX-License-Identifier: MIT

config ZMK_BEHAVIOR_KEY
Copy link
Collaborator

Choose a reason for hiding this comment

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

This may make more sense to me once I get to the relevant files, but I have no idea what "behavior key" and "behavior key scanned" mean. Adding a description to these or finding a more descriptive name would be helpful.

Copy link
Contributor Author

@petejohanson petejohanson Dec 21, 2023

Choose a reason for hiding this comment

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

Renamed per the other review comment. I believe that mostly addresses it.

app/dts/bindings/zmk,behavior-key-scanned.yaml Outdated Show resolved Hide resolved
app/dts/bindings/zmk,behavior-key.yaml Outdated Show resolved Hide resolved
app/src/behaviors/behavior_soft_off.c Outdated Show resolved Hide resolved
app/src/pm.c Outdated Show resolved Hide resolved
size_t device_count;
const struct device *devs;

#if !IS_ENABLED(CONFIG_ZMK_SPLIT) || IS_ENABLED(CONFIG_ZMK_SPLIT_ROLE_CENTRAL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably something for another PR, but there are a lot of these checks for "if split and peripheral" or "if not split or central" in the codebase. Might be useful to create some macros to simplify those.

app/src/pm.c Outdated Show resolved Hide resolved
app/src/wakeup_trigger_key.c Outdated Show resolved Hide resolved
petejohanson and others added 8 commits December 18, 2023 16:12
Initial work on a soft on/off support for ZMK. Triggering soft off
puts the device into deep sleep with only a specific GPIO pin
configured to wake the device, avoiding waking from other key
presses in the matrix like the normal deep sleep.

Co-authored-by: Cem Aksoylar <caksoylar@users.noreply.github.com>
* Add PM device hook to the kscan direct & matrix drivers.
* New soft-off behavior that can be used to force the device
  into  soft-off state with only certain configured wakeup
  devices.
* Use Button 1 for soft off on the nrf52840 when using the ZMK Uno
  shield.
* Add ability for external callers to clear the current endpoint.
* Make sure the connected host has no held HID usages before we sleep.
* Move to explicit enable of `ZMK_PM_SOFT_OFF` to turn
  on the feature and use the behaviors, which matches
  how other features work, and helps with split and
  testing schemes.
Co-authored-by: Cem Aksoylar <caksoylar@users.noreply.github.com>
@petejohanson petejohanson force-pushed the features/soft-off-take-2 branch 3 times, most recently from 7a4892c to 141dbd3 Compare December 21, 2023 02:21
* Better naming for gpio-key behavior triggers.
* Tweaks to scanned behavior trigger to avoid bad semaphore use,
  and reduce chance of issues with slowly scanned matrixes.
* Various code cleanups of style issues.
app/CMakeLists.txt Outdated Show resolved Hide resolved
app/src/gpio_key_wakeup_trigger.c Outdated Show resolved Hide resolved
app/src/gpio_key_wakeup_trigger.c Show resolved Hide resolved
app/src/gpio_key_wakeup_trigger.c Show resolved Hide resolved
* Code style to avoid goto.
* Enable pm.c compilation via dedicated Kconfig flag.
* Comment wakeup trigger PM behavior.
@petejohanson petejohanson mentioned this pull request Dec 30, 2023
@Luerl21
Copy link

Luerl21 commented Jan 29, 2024

Will you do a soft off for charliepex?

@petejohanson
Copy link
Contributor Author

Closing in favor of #2085

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behaviors core Core functionality/behavior of ZMK enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants