Added "catch" to knob behavior

[gregornoriskin]

I added a "catch" behavior to the knobs. Suppressed automatic page changing for some buttons. Fixed a couple of minor bugs.

[beserge]

I dig the catch behavior. It can be slightly confusing though when you turn a knob and the page doesn't change. Maybe the page should change when the knob moves, but the controls should still use the catch behavior. Just personal preference.

I noticed none of the controls other than reverb work when you have shift enabled. I don't really like that behavior. IMO all that should change is whether control 8 maps to Reverb or Feedback.

@stephenhensley any thoughts?

[gregornoriskin]

Ben, I agree. I will fix both those things in this PR.

Thanks, Gregor

Sent from Mailhttps://go.microsoft.com/fwlink/?LinkId=550986 for Windows 10

From: Ben @.> Sent: July 26, 2021 12:55 PM To: @.> Cc: Gregor @.>; @.> Subject: Re: [electro-smith/DaisyExamples] Added "catch" to knob behavior (#210)

I dig the catch behavior. It can be slightly confusing though when you turn a knob and the page doesn't change. Maybe the page should change when the knob moves, but the controls should still use the catch behavior. Just personal preference.

I noticed none of the controls other than reverb work when you have shift enabled. I don't really like that behavior. IMO all that should change is whether control 8 maps to Reverb or Feedback.

@stephenhensleyhttps://github.com/stephenhensley any thoughts?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/electro-smith/DaisyExamples/pull/210#issuecomment-886983024, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAOPXUR525Y3RB75FY5LQPDTZW4SXANCNFSM5A5UMYBA.

[stephenhensley]

I haven't had a chance to try it out quite yet, but it changing to the correct page with the knob turn seems like a good UI decision.

I could go either way on the shift behavior, but if it's the only thing the shift button does right now then having it toggle only the reverb/feedback control makes sense as well.

[stephenhensley]

Hi @gregornoriskin. Hope you're well. Looks like it was just a small change request remaining on this.

Just wanted to see if you were interested in wrapping that up since this has been sitting for a bit.

[gregornoriskin]

Stephen, Apologies, been caught up in a role change at work. I will get this done before the end of the month. It been bugging me too.

Thanks, Gregor

On Dec 1, 2021, at 1:18 PM, Stephen Hensley @.***> wrote:



Hi @gregornoriskinhttps://github.com/gregornoriskin. Hope you're well. Looks like it was just a small change request remaining on this.

Just wanted to see if you were interested in wrapping that up since this has been sitting for a bit.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/electro-smith/DaisyExamples/pull/210#issuecomment-984063089, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAOPXUWL7JTPGJVLSGXUZHDUO2GJXANCNFSM5A5UMYBA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[stephenhensley]

@gregornoriskin thanks for the hasty response.

No need to apologize, just wanted to check in.

Thanks! Looking forward to merging this :)

[gregornoriskin]

Dusting off my code, doing an upstream fetch and merge… perhaps I should abandon my previous PR and create a new one. Don’t see an option to do that though. What is your policy regarding stale PRs?

Thanks, Gregor

From: Stephen Hensley @.> Sent: December 1, 2021 2:16 PM To: electro-smith/DaisyExamples @.> Cc: Gregor Noriskin @.>; Mention @.> Subject: Re: [electro-smith/DaisyExamples] Added "catch" to knob behavior (#210)

@gregornoriskinhttps://github.com/gregornoriskin thanks for the hasty response.

No need to apologize, just wanted to check in.

Thanks! Looking forward to merging this :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/electro-smith/DaisyExamples/pull/210#issuecomment-984102987, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAOPXUVSOOPSOKY4O4IBDQ3UO2ND3ANCNFSM5A5UMYBA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[stephenhensley]

@gregornoriskin we don't have a specific policy in place yet.

If there were significant changes to the relevant files, I'd say resubmitting would be the best path forward, but the PR is still looking fairly clean (only 2 files changed), and I typically squash and merge on this repo so the commits will get grouped down to one.

So I think it's fine to leave to continue with this PR.

Let me know if you have any issues or questions.

[gregornoriskin]

Mkay, will just add to the existing PR. Hopefully not too many additional changes to address changes in libdaisy and DaisyDSP.

Thanks, Gregor

On Dec 7, 2021, at 12:34 PM, Stephen Hensley @.***> wrote:



@gregornoriskinhttps://github.com/gregornoriskin we don't have a specific policy in place yet.

If there were significant changes to the relevant files, I'd say resubmitting would be the best path forward, but the PR is still looking fairly clean (only 2 files changed), and I typically squash and merge on this repo so the commits will get grouped down to one.

So I think it's fine to leave to continue with this PR.

Let me know if you have any issues or questions.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/electro-smith/DaisyExamples/pull/210#issuecomment-988245071, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAOPXUS4GQORSZZE4W6YY3LUPZVTNANCNFSM5A5UMYBA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[gregornoriskin]

Done and done. Please see the PR. I also built this against the latest submodules so confirmed that it still works 😊

From: Gregor Noriskin @.> Sent: December 7, 2021 1:26 PM To: electro-smith/DaisyExamples @.> Cc: electro-smith/DaisyExamples @.>; Gregor Noriskin @.>; Mention @.***> Subject: Re: [electro-smith/DaisyExamples] Added "catch" to knob behavior (#210)

Mkay, will just add to the existing PR. Hopefully not too many additional changes to address changes in libdaisy and DaisyDSP. Thanks, Gregor

On Dec 7, 2021, at 12:34 PM, Stephen Hensley @.**@.>> wrote: 

@gregornoriskinhttps://github.com/gregornoriskin we don't have a specific policy in place yet.

If there were significant changes to the relevant files, I'd say resubmitting would be the best path forward, but the PR is still looking fairly clean (only 2 files changed), and I typically squash and merge on this repo so the commits will get grouped down to one.

So I think it's fine to leave to continue with this PR.

Let me know if you have any issues or questions.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/electro-smith/DaisyExamples/pull/210#issuecomment-988245071, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAOPXUS4GQORSZZE4W6YY3LUPZVTNANCNFSM5A5UMYBA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[beserge]

Works well, and the changes look good! I'd say it's ready to merge. 💯

[stephenhensley]

Thanks again for the contribution @gregornoriskin !!