Change tempo knob behavior and stop text scrolling on OLED

[suivaht]

Hello everyone... two things that have bugged me with my Deluge is the tempo knob changing in 4 bpm increments by default, so I switched the tempo knob behavior so it changes in 1 bpm increments by default and changes by 4 when you push in and turn.

I also disabled OLED text scrolling because i find it incredibly distracting. I simply commented out a line in View.cpp.

I would like to have these both available as options in the community features menu of the community branch but I am not sure how to go about implementing that... Ideally users could toggle either one of these options on or off.

[bfredl]

I think it would be a more useful option for the menu if it still were to scroll the full name once and then stop. Although this doesn't seem to be an option in the function signature yet..

[suivaht]

Agreed. I am a relative novice and implemented this change after seeing what minor things I could understand enough to modify without breaking anything. Any suggestions or modifications are welcome, and scrolling once would be better than no scrolling (though I prefer this to infinite scrolling personally)

[x3003]

On the oled version, it normally doesn't increment (just) by 4. I think it did on the non-oled.
On the oled It depends on the tempo, the faster the tempo, the more bpm is added.
So plz check if it really adds 4.

[suivaht]

@x3003 >

All I did was swap the coarse/fine tempo behavior code, so whatever it did by default now happens when pushing and turning the tempo knob instead. It isn't always by 4 (for example if my tempo is 91 and I hold and turn the encoder it will jump to 96, not 95). But I didn't modify the original behavior other than swapping what happens when turning versus pushing and turning the tempo knob.

[m-m-adams]

Couple things

1. Please separate the tempo change and the scroll change into two separate PRs for better tracking

2. For both of them please add a menu setting to change the behaviour rather than just changing it. While I agree this is probably an improvement for the tempo knob a change shouldn't be forced unless it's basically unanimous. Tempo coarse/fine swap should fit under the tempo heading in the main menu. Stopping scroll probably belongs in the community menu

Also welcome to the community!

[suivaht]

@m-m-adams Thank you for the kind welcome and for the helpful feedback. This is my first time going through this process so I appreciate the patience. Should I close this pull request and create two separate ones using this same branch (that includes both changes), or should I make a separate branch for each change and make separate pull requests for each branch?

[weavermedia]

@suivaht Take a look at some other feature addition PRs to see how the menu settings are implemented. Love the tempo change! It's been a pain for me for years. It was on my list of things to do but you beat me to it! 👏

[weavermedia]

Should I close this pull request and create two separate ones using this same branch (that includes both changes), or should I make a separate branch for each change and make separate pull requests for each branch?

@suivaht Close this one and submit 2 separate ones, each with only one change.

[m-m-adams]

Two separate feature branches works best, if you open two PRs from the same branch name GitHub will helpfully recombine them. You could close this PR or keep it for the side scrolling (by force pushing back to the view change). Then make a clean branch from community for the tempo change and put both PRs into draft. Once they're integrated into the menu take them out of draft to show that they're ready for testing and merge.

Thanks!

[suivaht]

Closing this for now and will create two separate branches after I get menu items in the community features menu... Thanks for the helpful comments!