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

refactor!: Remove deprecated label property #2028

Merged
merged 10 commits into from
Dec 5, 2023

Conversation

joelspadin
Copy link
Collaborator

Zephyr has deprecated the label property in favor of using the devicetree node name as a node identifier. This PR contains several changes to remove all instances of label from the codebase. The property is still supported, however, so most user keymaps will not need to be changed.

The most significant changes are:

  • Layers have a new name property to replace label when determining the layer name shown on displays.
  • Node names for all predefined behaviors have been shortened.
  • Behaviors now use BEHAVIOR_*_DEFINE() macros instead of DEVICE_*_DEFINE(). These define the device as before, but also add it to a new list of behavior devices.
  • Looking up a behavior by name is now done with zmk_behavior_get_binding() instead of device_get_binding(). This searches only the list of devices, so it is faster, and there is no possibility of returning the wrong device if one of the shortened behavior names collides with another node defined by the SoC.
  • If logging is enabled, an additional check will be run at system init, which will log an error if any two behaviors have the same name.

For the most part, these changes are non-breaking. However, because the names of predefined behavior nodes have been changed, if any keymap changes the settings on a behavior like

/ {
  behaviors {
    behavior_mod_tap {
      tapping-term-ms = <150>;
    };
  };
};

instead of the more common

&mt {
  tapping-term-ms = <150>;
};

then it will need to be updated to use the new name.

@joelspadin joelspadin requested a review from a team as a code owner November 19, 2023 02:14
@joelspadin joelspadin force-pushed the remove-label branch 3 times, most recently from 492e421 to 6244be5 Compare November 19, 2023 03:37
@caksoylar caksoylar added core Core functionality/behavior of ZMK refactor labels Nov 23, 2023
Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

Just a couple super minor things. On a whole this looks great! Thanks for tackling it.

docs/docs/config/keymap.md Outdated Show resolved Hide resolved
@@ -176,15 +176,13 @@ this to all of the `.dts` files for the boards that were affected by this issue.

```diff
code_partition: partition@26000 {
label = "code_partition";
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed in Discord, adding a small "Changes" or "Edits" list to any changed blog posts would be good for posterity whenever changing old posts.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added. Not sure I picked the best heading for it, so feel free to suggest a better one.

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

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

Thanks! Header/section for the updated blog posts is great.

@petejohanson
Copy link
Contributor

@joelspadin Needs a small base to fix a conflict, then we can merge right away. Thanks!

Changed all code (except for layer names) which used the label property
to use DEVICE_DT_NAME() instead, which uses the label if set or falls
back to the full node name. This matches how Zephyr determines the node
names used with device_get_binding() and allows us to start removing the
deprecated label property from things.
Changed the label property on zmk,ext-power-generic to be optional and
removed it from existing uses. Renamed the nodes for all non-development
boards to "EXT_POWER" to preserve user settings.

rgb_underglow.c now finds the correct device by finding the first
instance of zmk,ext-power-generic instead of looking for a node named
"EXT_POWER".
Removed "label" properties which no longer have any function.

Labels are still used as layer names and as identifiers for sending
behaviors between sides of a split keyboard, so those have been left
alone for now.
Changed the property used to define a layer name for displays from
"label" (which affects other things in Zephyr and is deprecated) to
"display-name". (It cannot be named simply "name", because that has
special meaning in newer versions of the devicetree compiler.)

"label" is still supported as a fallback, so no changes need to be made
to existing keymaps.
Added BEHAVIOR_DT_DEFINE() and BEHAVIOR_DT_INST_DEFINE(), which work
exactly like the DEVICE_*_DEFINE() macros, except they also register the
device as a behavior by adding a pointer to it to a memory section.

Added zmk_behavior_get_binding(), which works like device_get_binding()
except that it only searches the devices that have been registered as
behaviors. This ensures that behaviors cannot have name collisions with
other devices defined by the SoC, which will be important when we remove
the label property from behaviors so they are given their node names.

As an added benefit, this is faster since it searches a smaller list.
Some basic benchmark code I wrote indicates it takes 30-70% as long,
depending on where the behavior is in the list and whether the name
string is an exact pointer match.

From now on, behaviors should use BEHAVIOR_*_DEFINe() instead of
DEVICE_*_DEFINE(), and any code that looks up a behavior by name should
use zmk_behavior_get_binding() instead of device_get_binding().
Removed the label property from built-in behaviors, custom behaviors
defined in a few keymaps, and macros generated with ZMK_MACRO().

Now that node names are used to identify behaviors, and names only need
to be unique within the set of behaviors, the names of all behaviors
have been shortened to be similar to their original labels.

This means that any keymaps which reference behavior nodes by name
instead of by label will need to be updated. Keymaps typically use the
labels though, so most keymaps should be unaffected by this change.
Added a section to the new behavior guide to document that the names of
behaviors invoked on the peripheral side of a split must be at most 8
characters long.
Removed new uses of the "label" property which were introduced after
the previous commits on this branch were written.
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 refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants