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.

Sign up for GitHub

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

Jump to bottom

Core encoder/sensor refactor #1039

Merged
merged 10 commits into from Jun 19, 2023
Merged

Core encoder/sensor refactor #1039

merged 10 commits into from Jun 19, 2023

Conversation

petejohanson
Copy link
Contributor

@petejohanson petejohanson commented Nov 30, 2021 •
edited

Some major refactors to sensors:

  • Refactor ec11 sensor driver to align w/ Zephyr degree channel convention, that sensor data for that channel is in degrees of change since the last event.
  • Refactor to allow specitying what angle delta triggers a given behavior.
  • Move channel data into the sensor event, instead of requiring listeners to probe the sensor themselves upon events.

@petejohanson petejohanson added enhancement New feature or request core Core functionality/behavior of ZMK sensors refactor labels Nov 30, 2021
@petejohanson petejohanson self-assigned this Nov 30, 2021
@ReFil
Copy link
Contributor

ReFil commented Dec 2, 2021

If it's alright I'll send you a revised rotr folder with a better keymap and a few definition tweaks to match the bt60s

@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch from 2eb4df3 to 0838fa9 Compare December 2, 2021 21:32
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch 4 times, most recently from 867ef6a to ef57a53 Compare February 10, 2022 03:15
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch 2 times, most recently from 326a540 to b45a26b Compare June 16, 2022 05:12
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch 3 times, most recently from 98a3d97 to c87e149 Compare June 29, 2022 03:45
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch from c87e149 to fe08e17 Compare April 6, 2023 06:45
@petejohanson petejohanson changed the title Core/encoder sensor refactor and ma730 driver Core encoder/sensor refactor Apr 6, 2023
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch 9 times, most recently from adae294 to d557438 Compare April 10, 2023 06:27
joelspadin
joelspadin reviewed Apr 16, 2023
View reviewed changes
app/Kconfig Outdated
@@ -482,8 +494,12 @@ config ZMK_WPM
bool "Calculate WPM"
default n

config SENSOR
DT_COMPAT_ZMK_KEYMAP_SENSORS := zmk,keymap-sensors
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does battery.c require SENSOR be enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure does. Added a new commit here that creates a new ZMK_BATTERY_REPORTING Kconfig symbol:

config ZMK_BATTERY_REPORTING
	bool "Battery level detection/reporting"
	default n
	select SENSOR
	select BT_BAS if ZMK_BLE

That is implyd by ZMK_BLE automatically.

Thoughts?

@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch 2 times, most recently from f51b3fa to dd224e1 Compare April 19, 2023 23:36
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch 6 times, most recently from a7e2e61 to 2ea2a14 Compare May 24, 2023 05:07
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch from f37414d to 5e4abe0 Compare May 30, 2023 02:40
caksoylar
caksoylar reviewed May 30, 2023
View reviewed changes
docs/blog/2023-06-10-encoder-refactors.md Outdated Show resolved Hide resolved
docs/blog/2023-06-10-encoder-refactors.md Outdated Show resolved Hide resolved
docs/blog/2023-06-10-encoder-refactors.md Outdated Show resolved Hide resolved
docs/blog/2023-06-10-encoder-refactors.md Outdated Show resolved Hide resolved
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch from db70caf to f3450ca Compare June 1, 2023 05:49
joelspadin
joelspadin reviewed Jun 17, 2023
View reviewed changes
app/Kconfig

if ZMK_KEYMAP_SENSORS

config ZMK_KEYMAP_SENSORS_DEFAULT_TRIGGERS_PER_ROTATION
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the difference between this and ZMK_ENCODERS_DEFAULT_TRIGGERS_PER_ROTATION? Do we need two different configs (with different defaults) that seem to be for the same thing?

app/include/drivers/behavior.h Outdated Show resolved Hide resolved
app/src/behaviors/behavior_sensor_rotate_common.c Outdated Show resolved Hide resolved
app/src/sensors.c Outdated Show resolved Hide resolved
app/src/sensors.c Outdated Show resolved Hide resolved
app/src/sensors.c Show resolved Hide resolved
docs/blog/2023-06-10-encoder-refactors.md Outdated Show resolved Hide resolved
@petejohanson
refactor(sensors): Sensor event channel data, resolution tweaks.
70e79ad 
* Refactor sensor events to include channel data,
  necessary for prop split encoders, and avoiding duplicate calls,
  to fetch channel data twice, etc.
* More consistent behavior driver API.
* Allow setting triggers per resolution at the behavior level optionally.
@petejohanson
refactor(sensors): ec11 rotation sensor value in degrees.
0d23337 
* Add new `steps` property to the `aips,ec11` binding, to make
  the driver properly report degrees in the rotation delta channel.
* Handle old sensor values in sensor rotate behavior.
@petejohanson
refactor(config): Select SENSOR as needed.
0529949 
* Don't force on SENSOR Kconfig setting unless
  needed based on the detected DT.
@petejohanson
refactor(bluetooth): Bump HoG stack size.
d8ccde6 
* Bump the default stack size for the HoG processing thread
  to avoid issues w/ some pathways.
@petejohanson
refactor(shields): Updated ZMK Uno encoder config.
2c457f5 
* Move to new steps/triggers-per-rotation config.
* Leverage QDEC Nordic driver when used on Nordic DK.
@petejohanson
refactor(bluetooth): Add battery reporting config.
6296925 
* Add dedicated battery reporting Kconfig that is `imply`d by
  enabling ZMK_BLE.
@petejohanson
refactor(sensors): Split data handling from triggers.
0a386e2 
* All sensor behaviors should see sensor data, then selectively
  only have some trigger their behaviors.
@petejohanson
refactor(sensors): Use "sensor index" consistently
2465e3d
@petejohanson
fix(sensors): Clean ups based on code review.
ecabd88
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch from f3450ca to b05c1af Compare June 18, 2023 06:20
@petejohanson petejohanson requested a review from a team as a code owner June 18, 2023 06:20
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch from b05c1af to 471dddb Compare June 18, 2023 06:22
@petejohanson @caksoylar
feat(blog): Add post about sensor refactor.
8cfbd64 
* Document changed configuration, support for other Zephyr
  drivers, next steps.

Co-authored-by: Cem Aksoylar <caksoylar@users.noreply.github.com>
@petejohanson petejohanson force-pushed the core/encoder-sensor-refactor-and-ma730-driver branch from 471dddb to 8cfbd64 Compare June 18, 2023 20:48
@petejohanson petejohanson merged commit 5763558 into zmkfirmware:main Jun 19, 2023
46 checks passed
@petejohanson petejohanson mentioned this pull request Jun 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelspadin joelspadin joelspadin approved these changes

@caksoylar caksoylar caksoylar approved these changes

Assignees

@petejohanson petejohanson

Labels
core Core functionality/behavior of ZMK enhancement New feature or request refactor sensors
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@petejohanson @ReFil @joelspadin @caksoylar