Make number of count-in bars configurable#1305
Merged
Merged
Conversation
pmatila
force-pushed
the
countin
branch
2 times, most recently
from
29431e3 to
c799757
Compare
Collaborator
|
Hi! Sorry it took so long to review this. Looks like the format check is cranky, you can run |
Repurpose the count-in on/off toggle into number of count-in bars, allow up to four bars. This should be entirely backwards compatible as older firmware only checks for a non-zero value for enabled/disabled so if going backwards in firmware it counts as enabled and on update a previous enabled state gives one bar count-in as before.
|
Oh, I didn't realize 'dbt format' was a thing, and a thing you're supposed to run. Maybe something to put into CONTRIBUTING.md. Anyway, formatted (one of my includes wasn't in alphabetical order) and rebased. I actually consider this a rather speedy review, no worries on that front. Thanks for the review! |
github-actions Bot
pushed a commit
that referenced
this pull request
Feb 26, 2024
Repurpose the count-in on/off toggle into number of count-in bars, allow up to four bars. This should be entirely backwards compatible as older firmware only checks for a non-zero value for enabled/disabled so if going backwards in firmware it counts as enabled and on update a previous enabled state gives one bar count-in as before.
sapphire-arches
pushed a commit
that referenced
this pull request
Feb 26, 2024
Repurpose the count-in on/off toggle into number of count-in bars, allow up to four bars. This should be entirely backwards compatible as older firmware only checks for a non-zero value for enabled/disabled so if going backwards in firmware it counts as enabled and on update a previous enabled state gives one bar count-in as before.
pmatila
added a commit
to pmatila/DelugeFirmware
that referenced
this pull request
May 5, 2024
The configurable count-in bars change (PR SynthstromAudible#1305 commit 2a68d0e) had a changelog entry but lacked the community features documentation part, but PR SynthstromAudible#1419 (commit 21c4a75) added a second changelog entry along with the feature doc. Dropping my original changelog as the latter one is more detailed.
pmatila
added a commit
to pmatila/DelugeFirmware
that referenced
this pull request
May 5, 2024
The configurable count-in bars change (PR SynthstromAudible#1305 commit 2a68d0e) had a changelog entry but lacked the community features documentation part. PR SynthstromAudible#1419 (commit 21c4a75) added a second changelog entry along with the feature doc. Dropping my original changelog as the latter one is more detailed.
tastycode
pushed a commit
to tastycode/DelugeFirmware
that referenced
this pull request
Jul 3, 2024
Repurpose the count-in on/off toggle into number of count-in bars, allow up to four bars. This should be entirely backwards compatible as older firmware only checks for a non-zero value for enabled/disabled so if going backwards in firmware it counts as enabled and on update a previous enabled state gives one bar count-in as before.
tastycode
pushed a commit
to tastycode/DelugeFirmware
that referenced
this pull request
Jul 3, 2024
…1835) The configurable count-in bars change (PR SynthstromAudible#1305 commit 2a68d0e) had a changelog entry but lacked the community features documentation part. PR SynthstromAudible#1419 (commit 21c4a75) added a second changelog entry along with the feature doc. Dropping my original changelog as the latter one is more detailed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First-time contributor to Deluge here (Turre on Discord), so apologies if I've missed something in the community protocol.
As for the feature: I don't know how people get by with just one bar of count-in 😳 Except for very slow songs, I need that two bars to get my hands in position and stuff. Maybe everybody else just uses a foot-switch to control 😄 Anyway, this changes the on/off toggle into an integer-with-off, allow up to four bars of count-in.
I repurposed the count-in enabled toggle for the purpose because this should be entirely backwards compatible and so adding a separate value for the number of bars seemed silly waste of a perfectly good byte to me 😄
I was a bit surprised IntergerWithOff renders the off-state with a zero, "Off" would seem more appropriate in at least this case. I can add that to the PR if that would be preferred.
For simplicity's sake I just made the countdown ticks a multiply of the bars, a fancier way could be counting down bars and ticks separately.