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

Denon MC7000 improvements #4021

Merged
merged 16 commits into from
Sep 25, 2021
Merged

Denon MC7000 improvements #4021

merged 16 commits into from
Sep 25, 2021

Conversation

toszlanyi
Copy link
Contributor

@toszlanyi toszlanyi commented Jun 21, 2021

Would like to store the ideas for several small improvement that I collected from forum & zulip chats over time. No need to rush that through.

  • several SLIP mode improvements. With activated SLIP mode it will be switched off automatically and therefore jump to the original time position inside the track after:
    scratching
    after Hot Cue jump
    after backspin
  • changed (simplified) jog calculations
    ( - changed scratch parameters EDIT: Won't change)
  • stop to play current sampler when a new one is triggered
  • Rate Range change now considers start point from Mixxx preferences for changing range up and down
  • Jog Wheel search through maximized library

@uklotzde uklotzde added this to the 2.3.1 milestone Jun 28, 2021
res/controllers/Denon-MC7000-scripts.js Outdated Show resolved Hide resolved
res/controllers/Denon-MC7000-scripts.js Outdated Show resolved Hide resolved
@toszlanyi toszlanyi marked this pull request as ready for review July 20, 2021 11:00
res/controllers/Denon-MC7000-scripts.js Outdated Show resolved Hide resolved
res/controllers/Denon-MC7000-scripts.js Outdated Show resolved Hide resolved
@uklotzde
Copy link
Contributor

uklotzde commented Aug 3, 2021

Merge for inclusion in 2.3.1?

@uklotzde uklotzde added the changelog This PR should be included in the changelog label Aug 3, 2021
@Holzhaus
Copy link
Member

Holzhaus commented Aug 3, 2021

Didn't check, is there a manual PR already? Otherwise we may postpone to 2.3.2 (depending how long we need to release 2.3.1).

@uklotzde
Copy link
Contributor

uklotzde commented Aug 3, 2021

@toszlanyi Please prepare a PR for the manual that accounts for the changes and then let's merge.

@toszlanyi
Copy link
Contributor Author

@toszlanyi Please prepare a PR for the manual that accounts for the changes and then let's merge.

I referenced the manual PR 2 weeks ago but here again ;)
mixxxdj/manual#413

Thanks a lot guys!

@uklotzde
Copy link
Contributor

uklotzde commented Aug 4, 2021

@Swiftb0y Merge?

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

sorry for neglecting this PR for so long.

res/controllers/Denon-MC7000-scripts.js Outdated Show resolved Hide resolved
@toszlanyi
Copy link
Contributor Author

Manual was updated with PR mixxxdj/manual#413
Ready for review as well

@toszlanyi toszlanyi requested a review from Swiftb0y August 7, 2021 09:52
res/controllers/Denon-MC7000-scripts.js Outdated Show resolved Hide resolved
res/controllers/Denon-MC7000-scripts.js Outdated Show resolved Hide resolved
@toszlanyi
Copy link
Contributor Author

I had to update just one more thing about the timers I introduced to enable SLIP again. When an action is started while the timer is not yet activated, so when you double push a button very quickly then SLIP mode uses the 2nd action when it was not yet activated again. Example. If one pushes the reverse button twice and while the reverse is pushed down within the timer time but released after SLIP enables again then SLIP continued with reverse play ... I reduced to 20ms and could not press a button again that quickly to get this experience again. According to the Denon documentation the Midi signals rate through USB can be set on the controller to be sent in intervals between 1ms and 14ms. So even if one has set its controller to the max Midi time it should still be fine. Cheers!

@toszlanyi
Copy link
Contributor Author

Hey - I see there is a pre-commit error on my last commit. When i pushed I got following message.

grafik

How can I see what is wrong? Thank you!

@Holzhaus
Copy link
Member

If you only want to check your last commit:

$ pre-commit run --from-ref HEAD~1 --to-ref HEAD

To check this whole PR against the latest upstream 2.3 branch:

$ git fetch upstream
$ pre-commit run --from-ref upstream/2.3 --to-ref HEAD

@toszlanyi
Copy link
Contributor Author

toszlanyi commented Sep 5, 2021

Recently I saw in Traktor that if the library was maximized then one could quickly search through a long Crate or Playlist with the jog wheel. I was very pleased how quickly you could navigate compared to use the encoder knob. So I added this little improvement as well.
There is just no working SLIP mode anymore. I mean - after releasing the platter the background timeline plays significantly slower tempo which results in increased discrepancy between background and playing timeline. So maybe you got a good suggestion for improving the code snippet from lines 653 to 703 inside the .js file. Thanks a lot

@toszlanyi
Copy link
Contributor Author

There is just no working SLIP mode anymore. I mean - after releasing the platter the background timeline plays significantly slower tempo which results in increased discrepancy between background and playing timeline. So maybe you got a good suggestion for improving the code snippet from lines 653 to 703 inside the .js file. Thanks a lot

I fixed this myself - now working as expected. If you see any other issue with that code snippet then please let me know. Thanks

@Be-ing
Copy link
Contributor

Be-ing commented Sep 7, 2021

@Swiftb0y @Holzhaus is this ready to merge? If there's much left to do, let's not delay 2.3.1 for it but if it's ready let's merge it for 2.3.1.

@toszlanyi
Copy link
Contributor Author

Documentation is updated as well... mixxxdj/manual#413

res/controllers/Denon-MC7000-scripts.js Outdated Show resolved Hide resolved
@uklotzde
Copy link
Contributor

@Swiftb0y @Holzhaus Merge?

@toszlanyi
Copy link
Contributor Author

Is there anything left to do for me yet? Would love to see the updates in 2.3.1
Thanks!

@ronso0 ronso0 merged commit f4bfce2 into mixxxdj:2.3 Sep 25, 2021
@ronso0
Copy link
Member

ronso0 commented Sep 25, 2021

(thinking about this just after merging...)
What's the deal now with changelog entries? Didn't follow the discussion about automation / working with PR labels, sry.

@Holzhaus
Copy link
Member

Yes, please add a changelog entry.

@ronso0
Copy link
Member

ronso0 commented Sep 25, 2021

@Holzhaus So for changelog changes the metainfo hook needs to be skipped??

@Holzhaus
Copy link
Member

Holzhaus commented Sep 25, 2021

@Holzhaus So for changelog changes the metainfo hook needs to be skipped??

No, why? Whenever you update the changelog, the metainfo hook will update the appstream metadata automatically.

@ronso0
Copy link
Member

ronso0 commented Sep 25, 2021

I was confused by

metainfo...................................Failed
- hook id: metainfo
- files were modified by this hook

and didn't realize that it's not an error but I need to git add the changes myself.
problem solved. I was just too tired...

@ronso0 ronso0 mentioned this pull request Sep 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This PR should be included in the changelog controller mappings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants