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 CLI flag to disconnect all MIDI controllers when loading a project #4883

Closed
wants to merge 4 commits into from
Closed

Add CLI flag to disconnect all MIDI controllers when loading a project #4883

wants to merge 4 commits into from

Conversation

pboivin
Copy link

@pboivin pboivin commented Mar 9, 2019

This is a redo of #4874, using master as base branch.

==

Hi!

Consider this PR as a request for comment, as this is my first attempt at hacking on LMMS.

This introduces a CLI flag (--disconnect-midi-controllers) used to disconnect all MIDI inputs from instrument tracks, as well as all MIDI controllers from instrument, FX and other global/song parameters. Other user controllers should be left untouched.

In and of itself, I find it useful to have a quick way to remove all midi connections from an existing project. For example, consider moving a project that has an extensive MIDI configuration for a specific hardware controller, over to another type of hardware controller. As I understand it, the current workflow would require disconnecting each individual controller mapping, one by one.

Also, it could be thought of as a quick fix for the following issues:
#4385
#2625
#193

Is this idea of any interest to anyone else?

If so, I understand that this is probably a naive approach and would welcome your feedback!

Many thanks!

@JohannesLorenz JohannesLorenz added needs code review A functional code review is currently required for this PR needs style review A style review is currently required for this PR labels Apr 22, 2019
@Reflexe
Copy link
Member

Reflexe commented May 27, 2019

The code looks fine. However, maybe we should add an export option that will not save MIDI connections to the file in first place? If that was your intention, I could try to implement that in the next following days.

@Reflexe Reflexe self-requested a review May 27, 2019 18:37
Copy link
Member

@Reflexe Reflexe left a comment

Choose a reason for hiding this comment

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

Looks fine for me, however, look on my comment please.

@Reflexe Reflexe added response required A response from OP is required or the issue is closed automatically within 14 days. and removed needs code review A functional code review is currently required for this PR needs style review A style review is currently required for this PR labels May 27, 2019
@pboivin
Copy link
Author

pboivin commented May 28, 2019

Hi, thanks for the feedback!

I also think that an export option to omit the MIDI connections would be a cleaner solution. I hesitated before sending this PR because my solution felt a bit naive and hacky.

If you have some time to look into it, I welcome your help in implementing this export option.

@Reflexe Reflexe removed the response required A response from OP is required or the issue is closed automatically within 14 days. label May 30, 2019
@Reflexe
Copy link
Member

Reflexe commented Jun 2, 2019

Update: I have a working POC for a save dialog with a "discard MIDI connections" option. I'll polish and finish it in the next following days (hopefully tomorrow)

@Reflexe
Copy link
Member

Reflexe commented Jun 9, 2019

Update: I've opened a PR at #5021; feel free to test it and report back.

@pboivin
Copy link
Author

pboivin commented Jun 10, 2019

Hi!

Just tested the master branch with all of your merged commits. It works very well.

Closing this PR because the new save option makes it irrelevant.

Many thanks,

P.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants