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

Add step sequencer feature #9703

Merged
merged 19 commits into from
Nov 8, 2020
Merged

Add step sequencer feature #9703

merged 19 commits into from
Nov 8, 2020

Conversation

rbelouin
Copy link

@rbelouin rbelouin commented Jul 11, 2020

Description

I have MIDI pads that have stopped working recently, and have considered my Planck EZ as a potential replacement. After all, QMK already has support for the MIDI protocol and it should not be that hard to build a step sequencer. So I gave it a try!

Dear reviewers, please keep in mind this is my first contribution to a C project. I’m more than happy to learn from your experience! Also, reading the pull request commit by commit might be preferable.

The feature consists of quantum/sequencer, quantum/process_keycode/process_sequencer and some documentation. I’ve been willing to add unit tests (e.g. calculation of the step duration, (de)activation of tracks that makes use of bitwise operations) but could not get them to run locally. Any guidance is welcome!

To summarize it in one sentence, the sequencer is just a layer on top of the midi feature that manages some time-dependent state. I did not couple it with the MIDI feature though, as people could potentially be interested in reusing the functionality to build non-MIDI sequences.

Event throttling

While iterating on the implementation, I have noticed that my DAW was dropping events when too many were sent in a short period of time. So I have designed the feature to follow three sequential phases for each step to throttle the events: the attack phase, the release phase, and the pause phase.

Attack Phase:

This the phase where the feature sends MIDI on events. It initializes a timer and sends an event for each track that is active for the current step, with a delay of 3 milliseconds between each note.

Release Phase:

When all tracks have been processed by the attack phase and a delay of 20 milliseconds has passed since the timer has been initialized, we send MIDI off events for each active track. Still with a 3ms delay between each note.

Pause Phase:

Based on the current tempo and step resolution, we wait the necessary amount of time before processing the next step.

Keymap

It is left to the user to build a sequencer layout that suits their needs, you can always check mine for inspiration.

Demo

Watch me play with the feature on this YouTube video!

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Status: I’m still testing the feature with different parameters on my Planck EZ, I would appreciate if anybody can help test it on another device.

@rbelouin rbelouin marked this pull request as ready for review July 11, 2020 21:10
@drashna drashna requested a review from a team July 13, 2020 20:10
@drashna
Copy link
Member

drashna commented Jul 13, 2020

VERY COOL!!

However, could you target the develop branch with this, instead?
It should be as simple is clicking the edit for the title, and changing the branch.

@rbelouin rbelouin changed the base branch from master to develop July 13, 2020 20:28
@rbelouin
Copy link
Author

Absolutely, I might have missed that part in the how to contribute docs. I’ll rebase interactively to get rid of the other commits that have just appeared.

Copy link
Member

@fauxpark fauxpark left a comment

Choose a reason for hiding this comment

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

Also, it looks like it but just in case: could you run clang-format over these files?

quantum/process_keycode/process_sequencer.c Outdated Show resolved Hide resolved
quantum/process_keycode/process_sequencer.c Outdated Show resolved Hide resolved
docs/feature_sequencer.md Outdated Show resolved Hide resolved
quantum/process_keycode/process_sequencer.h Outdated Show resolved Hide resolved
quantum/process_keycode/process_sequencer.h Outdated Show resolved Hide resolved
quantum/quantum_keycodes.h Outdated Show resolved Hide resolved
quantum/sequencer/sequencer.c Outdated Show resolved Hide resolved
quantum/sequencer/sequencer.c Show resolved Hide resolved
quantum/sequencer/sequencer.c Show resolved Hide resolved
quantum/sequencer/sequencer.c Show resolved Hide resolved
quantum/sequencer/sequencer.c Show resolved Hide resolved
quantum/sequencer/sequencer.c Outdated Show resolved Hide resolved
@rbelouin
Copy link
Author

rbelouin commented Jul 24, 2020

@fauxpark @tzarc thanks a lot for taking the time to review the pull request, I’ve finally taken the time to address your comments. Let me know if you have any other suggestions.

I still need assistance with unit testing. make test:all fails locally, even on a fresh master; it makes it somewhat challenging to add unit tests for my feature.

@drashna
Copy link
Member

drashna commented Jul 30, 2020

I still need assistance with unit testing. make test:all fails locally, even on a fresh master; it makes it somewhat challenging to add unit tests for my feature.

❤️

The simplest way may be to set up docker, and then run the tests in that. That's what Travis CI uses, so should be more accurate.

Though, tests are one of the under developed areas (eg, not many people write or know how to write tests). So doing so is very appreciated.

quantum/quantum_keycodes.h Outdated Show resolved Hide resolved
@noroadsleft
Copy link
Member

This needs to be rebased, and the merge conflicts resolved.

noroadsleft and others added 2 commits September 12, 2020 04:45
The matrix_col_t type was added in commit 0284431 (part of qmk#3449),
but then the code which used that type was removed in qmk#6140, and no
other users were added since that time.  The presence of that type,
however, limits MATRIX_ROWS to 32, which probably does not matter for a
real keyboard, but prevents doing things like making a firmware to test
all existing pins on a board like Teensy++ 2.0 (which has 46 GPIOs).
@rbelouin
Copy link
Author

Hey @noroadsleft, @tzarc, @fauxpark 👋

Thanks a lot for your feedback, I think it’s all been addressed. I’m quite happy with my changes, with their test coverage, and I’ve double checked again that the feature was working on my Planck EZ.

I have a personal branch with a layout that uses this feature, feel free to check it out and adapt it on other QMK-running keyboards.

I’m happy to take any additional comments if you have some, otherwise let me know what the next steps are!

Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

__attribute__ ((weak)) ✔️

@rbelouin
Copy link
Author

Anything I can do to move this forward?

@noroadsleft
Copy link
Member

I didn't want to be the one to merge this because I don't have any experience with anything related to this feature, but #YOLO...

@noroadsleft noroadsleft merged commit 38527f9 into qmk:develop Nov 8, 2020
@noroadsleft
Copy link
Member

Thanks!

@rbelouin rbelouin deleted the sequencer branch November 10, 2020 19:33
noroadsleft added a commit that referenced this pull request Nov 28, 2020
* Branch point for 2020 November 28 Breaking Change                                                

* Remove matrix_col_t to allow MATRIX_ROWS > 32 (#10183)                                           

* Add support for soft serial to ATmega32U2 (#10204)                                               

* Change MIDI velocity implementation to allow direct control of velocity value (#9940)            

* Add ability to build a subset of all keyboards based on platform.                                

* Actually use eeprom_driver_init().                                                               

* Make bootloader_jump weak for ChibiOS. (#10417)                                                  

* Joystick 16-bit support (#10439)                                                                 

* Per-encoder resolutions (#10259)                                                                 

* Share button state from mousekey to pointing_device (#10179)                                     

* Add hotfix for chibios keyboards not wake (#10088)                                               

* Add advanced/efficient RGB Matrix Indicators (#8564)                                             

* Naming change.                                                                                   

* Support for STM32 GPIOF,G,H,I,J,K (#10206)                                                       

* Add milc as a dependency and remove the installed milc (#10563)                                  

* ChibiOS upgrade: early init conversions (#10214)                                                 

* ChibiOS upgrade: configuration file migrator (#9952)                                             

* Haptic and solenoid cleanup (#9700)                                                              

* XD75 cleanup (#10524)                                                                            

* OLED display update interval support (#10388)                                                    

* Add definition based on currently-selected serial driver. (#10716)                               

* New feature: Retro Tapping per key (#10622)                                                      

* Allow for modification of output RGB values when using rgblight/rgb_matrix. (#10638)             

* Add housekeeping task callbacks so that keyboards/keymaps are capable of executing code for each main loop iteration. (#10530)

* Rescale both ChibiOS and AVR backlighting.                                                       

* Reduce Helix keyboard build variation (#8669)                                                    

* Minor change to behavior allowing display updates to continue between task ticks (#10750)        

* Some GPIO manipulations in matrix.c change to atomic. (#10491)                                   

* qmk cformat (#10767)                                                                             

* [Keyboard] Update the Speedo firmware for v3.0 (#10657)                                          

* Maartenwut/Maarten namechange to evyd13/Evy (#10274)                                             

* [quantum] combine repeated lines of code (#10837)                                                

* Add step sequencer feature (#9703)                                                               

* aeboards/ext65 refactor (#10820)                                                                 

* Refactor xelus/dawn60 for Rev2 later (#10584)                                                    

* add DEBUG_MATRIX_SCAN_RATE_ENABLE to common_features.mk (#10824)                                 

* [Core] Added `add_oneshot_mods` & `del_oneshot_mods` (#10549)                                    

* update chibios os usb for the otg driver (#8893)                                                 

* Remove HD44780 References, Part 4 (#10735)                                                       

* [Keyboard] Add Valor FRL TKL (+refactor) (#10512)                                                

* Fix cursor position bug in oled_write_raw functions (#10800)                                     

* Fixup version.h writing when using SKIP_VERSION=yes (#10972)                                     

* Allow for certain code in the codebase assuming length of string. (#10974)                       

* Add AT90USB support for serial.c (#10706)                                                        

* Auto shift: support repeats and early registration (#9826)                                       

* Rename ledmatrix.h to match .c file (#7949)                                                      

* Split RGB_MATRIX_ENABLE into _ENABLE and _DRIVER (#10231)                                        

* Split LED_MATRIX_ENABLE into _ENABLE and _DRIVER (#10840)                                        

* Merge point for 2020 Nov 28 Breaking Change
xgnxs pushed a commit to xgnxs/qmk_firmware that referenced this pull request Jan 9, 2021
* Branch point for 2020 November 28 Breaking Change                                                

* Remove matrix_col_t to allow MATRIX_ROWS > 32 (qmk#10183)                                           

* Add support for soft serial to ATmega32U2 (qmk#10204)                                               

* Change MIDI velocity implementation to allow direct control of velocity value (qmk#9940)            

* Add ability to build a subset of all keyboards based on platform.                                

* Actually use eeprom_driver_init().                                                               

* Make bootloader_jump weak for ChibiOS. (qmk#10417)                                                  

* Joystick 16-bit support (qmk#10439)                                                                 

* Per-encoder resolutions (qmk#10259)                                                                 

* Share button state from mousekey to pointing_device (qmk#10179)                                     

* Add hotfix for chibios keyboards not wake (qmk#10088)                                               

* Add advanced/efficient RGB Matrix Indicators (qmk#8564)                                             

* Naming change.                                                                                   

* Support for STM32 GPIOF,G,H,I,J,K (qmk#10206)                                                       

* Add milc as a dependency and remove the installed milc (qmk#10563)                                  

* ChibiOS upgrade: early init conversions (qmk#10214)                                                 

* ChibiOS upgrade: configuration file migrator (qmk#9952)                                             

* Haptic and solenoid cleanup (qmk#9700)                                                              

* XD75 cleanup (qmk#10524)                                                                            

* OLED display update interval support (qmk#10388)                                                    

* Add definition based on currently-selected serial driver. (qmk#10716)                               

* New feature: Retro Tapping per key (qmk#10622)                                                      

* Allow for modification of output RGB values when using rgblight/rgb_matrix. (qmk#10638)             

* Add housekeeping task callbacks so that keyboards/keymaps are capable of executing code for each main loop iteration. (qmk#10530)

* Rescale both ChibiOS and AVR backlighting.                                                       

* Reduce Helix keyboard build variation (qmk#8669)                                                    

* Minor change to behavior allowing display updates to continue between task ticks (qmk#10750)        

* Some GPIO manipulations in matrix.c change to atomic. (qmk#10491)                                   

* qmk cformat (qmk#10767)                                                                             

* [Keyboard] Update the Speedo firmware for v3.0 (qmk#10657)                                          

* Maartenwut/Maarten namechange to evyd13/Evy (qmk#10274)                                             

* [quantum] combine repeated lines of code (qmk#10837)                                                

* Add step sequencer feature (qmk#9703)                                                               

* aeboards/ext65 refactor (qmk#10820)                                                                 

* Refactor xelus/dawn60 for Rev2 later (qmk#10584)                                                    

* add DEBUG_MATRIX_SCAN_RATE_ENABLE to common_features.mk (qmk#10824)                                 

* [Core] Added `add_oneshot_mods` & `del_oneshot_mods` (qmk#10549)                                    

* update chibios os usb for the otg driver (qmk#8893)                                                 

* Remove HD44780 References, Part 4 (qmk#10735)                                                       

* [Keyboard] Add Valor FRL TKL (+refactor) (qmk#10512)                                                

* Fix cursor position bug in oled_write_raw functions (qmk#10800)                                     

* Fixup version.h writing when using SKIP_VERSION=yes (qmk#10972)                                     

* Allow for certain code in the codebase assuming length of string. (qmk#10974)                       

* Add AT90USB support for serial.c (qmk#10706)                                                        

* Auto shift: support repeats and early registration (qmk#9826)                                       

* Rename ledmatrix.h to match .c file (qmk#7949)                                                      

* Split RGB_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10231)                                        

* Split LED_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10840)                                        

* Merge point for 2020 Nov 28 Breaking Change
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Jan 13, 2021
* Branch point for 2020 November 28 Breaking Change

* Remove matrix_col_t to allow MATRIX_ROWS > 32 (qmk#10183)

* Add support for soft serial to ATmega32U2 (qmk#10204)

* Change MIDI velocity implementation to allow direct control of velocity value (qmk#9940)

* Add ability to build a subset of all keyboards based on platform.

* Actually use eeprom_driver_init().

* Make bootloader_jump weak for ChibiOS. (qmk#10417)

* Joystick 16-bit support (qmk#10439)

* Per-encoder resolutions (qmk#10259)

* Share button state from mousekey to pointing_device (qmk#10179)

* Add hotfix for chibios keyboards not wake (qmk#10088)

* Add advanced/efficient RGB Matrix Indicators (qmk#8564)

* Naming change.

* Support for STM32 GPIOF,G,H,I,J,K (qmk#10206)

* Add milc as a dependency and remove the installed milc (qmk#10563)

* ChibiOS upgrade: early init conversions (qmk#10214)

* ChibiOS upgrade: configuration file migrator (qmk#9952)

* Haptic and solenoid cleanup (qmk#9700)

* XD75 cleanup (qmk#10524)

* OLED display update interval support (qmk#10388)

* Add definition based on currently-selected serial driver. (qmk#10716)

* New feature: Retro Tapping per key (qmk#10622)

* Allow for modification of output RGB values when using rgblight/rgb_matrix. (qmk#10638)

* Add housekeeping task callbacks so that keyboards/keymaps are capable of executing code for each main loop iteration. (qmk#10530)

* Rescale both ChibiOS and AVR backlighting.

* Reduce Helix keyboard build variation (qmk#8669)

* Minor change to behavior allowing display updates to continue between task ticks (qmk#10750)

* Some GPIO manipulations in matrix.c change to atomic. (qmk#10491)

* qmk cformat (qmk#10767)

* [Keyboard] Update the Speedo firmware for v3.0 (qmk#10657)

* Maartenwut/Maarten namechange to evyd13/Evy (qmk#10274)

* [quantum] combine repeated lines of code (qmk#10837)

* Add step sequencer feature (qmk#9703)

* aeboards/ext65 refactor (qmk#10820)

* Refactor xelus/dawn60 for Rev2 later (qmk#10584)

* add DEBUG_MATRIX_SCAN_RATE_ENABLE to common_features.mk (qmk#10824)

* [Core] Added `add_oneshot_mods` & `del_oneshot_mods` (qmk#10549)

* update chibios os usb for the otg driver (qmk#8893)

* Remove HD44780 References, Part 4 (qmk#10735)

* [Keyboard] Add Valor FRL TKL (+refactor) (qmk#10512)

* Fix cursor position bug in oled_write_raw functions (qmk#10800)

* Fixup version.h writing when using SKIP_VERSION=yes (qmk#10972)

* Allow for certain code in the codebase assuming length of string. (qmk#10974)

* Add AT90USB support for serial.c (qmk#10706)

* Auto shift: support repeats and early registration (qmk#9826)

* Rename ledmatrix.h to match .c file (qmk#7949)

* Split RGB_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10231)

* Split LED_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10840)

* Merge point for 2020 Nov 28 Breaking Change
hazelweakly pushed a commit to hazelweakly/qmk_firmware that referenced this pull request Sep 23, 2021
* sequencer: create togglable feature

* sequencer: add support for steps

* sequencer: add support for tempo and resolutions

* sequencer: schedule a message print at the right frequency

* sequencer: send a hardcoded note

* sequencer: add support for 8 tracks

* sequencer: play several notes simultaneously

* sequencer: only play the active tracks for a given step

* sequencer: change the default behavior of the track toggler

* sequencer: make number of tracks and track notes customizable

* sequencer: move the keycodes down

Not adding them at the end of the list apparently risks breaking
compatibility with VIA.

Source: qmk#9703 (comment)

* sequencer: add unit tests

* sequencer: add unit test for matrix_scan_sequencer

* sequencer: expose internal state for better unit-testability

* sequencer: add unit tests for matrix_scan_sequencer
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Branch point for 2020 November 28 Breaking Change                                                

* Remove matrix_col_t to allow MATRIX_ROWS > 32 (qmk#10183)                                           

* Add support for soft serial to ATmega32U2 (qmk#10204)                                               

* Change MIDI velocity implementation to allow direct control of velocity value (qmk#9940)            

* Add ability to build a subset of all keyboards based on platform.                                

* Actually use eeprom_driver_init().                                                               

* Make bootloader_jump weak for ChibiOS. (qmk#10417)                                                  

* Joystick 16-bit support (qmk#10439)                                                                 

* Per-encoder resolutions (qmk#10259)                                                                 

* Share button state from mousekey to pointing_device (qmk#10179)                                     

* Add hotfix for chibios keyboards not wake (qmk#10088)                                               

* Add advanced/efficient RGB Matrix Indicators (qmk#8564)                                             

* Naming change.                                                                                   

* Support for STM32 GPIOF,G,H,I,J,K (qmk#10206)                                                       

* Add milc as a dependency and remove the installed milc (qmk#10563)                                  

* ChibiOS upgrade: early init conversions (qmk#10214)                                                 

* ChibiOS upgrade: configuration file migrator (qmk#9952)                                             

* Haptic and solenoid cleanup (qmk#9700)                                                              

* XD75 cleanup (qmk#10524)                                                                            

* OLED display update interval support (qmk#10388)                                                    

* Add definition based on currently-selected serial driver. (qmk#10716)                               

* New feature: Retro Tapping per key (qmk#10622)                                                      

* Allow for modification of output RGB values when using rgblight/rgb_matrix. (qmk#10638)             

* Add housekeeping task callbacks so that keyboards/keymaps are capable of executing code for each main loop iteration. (qmk#10530)

* Rescale both ChibiOS and AVR backlighting.                                                       

* Reduce Helix keyboard build variation (qmk#8669)                                                    

* Minor change to behavior allowing display updates to continue between task ticks (qmk#10750)        

* Some GPIO manipulations in matrix.c change to atomic. (qmk#10491)                                   

* qmk cformat (qmk#10767)                                                                             

* [Keyboard] Update the Speedo firmware for v3.0 (qmk#10657)                                          

* Maartenwut/Maarten namechange to evyd13/Evy (qmk#10274)                                             

* [quantum] combine repeated lines of code (qmk#10837)                                                

* Add step sequencer feature (qmk#9703)                                                               

* aeboards/ext65 refactor (qmk#10820)                                                                 

* Refactor xelus/dawn60 for Rev2 later (qmk#10584)                                                    

* add DEBUG_MATRIX_SCAN_RATE_ENABLE to common_features.mk (qmk#10824)                                 

* [Core] Added `add_oneshot_mods` & `del_oneshot_mods` (qmk#10549)                                    

* update chibios os usb for the otg driver (qmk#8893)                                                 

* Remove HD44780 References, Part 4 (qmk#10735)                                                       

* [Keyboard] Add Valor FRL TKL (+refactor) (qmk#10512)                                                

* Fix cursor position bug in oled_write_raw functions (qmk#10800)                                     

* Fixup version.h writing when using SKIP_VERSION=yes (qmk#10972)                                     

* Allow for certain code in the codebase assuming length of string. (qmk#10974)                       

* Add AT90USB support for serial.c (qmk#10706)                                                        

* Auto shift: support repeats and early registration (qmk#9826)                                       

* Rename ledmatrix.h to match .c file (qmk#7949)                                                      

* Split RGB_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10231)                                        

* Split LED_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10840)                                        

* Merge point for 2020 Nov 28 Breaking Change
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* sequencer: create togglable feature

* sequencer: add support for steps

* sequencer: add support for tempo and resolutions

* sequencer: schedule a message print at the right frequency

* sequencer: send a hardcoded note

* sequencer: add support for 8 tracks

* sequencer: play several notes simultaneously

* sequencer: only play the active tracks for a given step

* sequencer: change the default behavior of the track toggler

* sequencer: make number of tracks and track notes customizable

* sequencer: move the keycodes down

Not adding them at the end of the list apparently risks breaking
compatibility with VIA.

Source: qmk#9703 (comment)

* sequencer: add unit tests

* sequencer: add unit test for matrix_scan_sequencer

* sequencer: expose internal state for better unit-testability

* sequencer: add unit tests for matrix_scan_sequencer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants