Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Sys ex control #287

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

Conversation

BenZonneveld
Copy link

Several improvement in sysex control.
All instances get a fixed sysex channel as not to cause any conflicts.

@probonopd
Copy link
Owner

probonopd commented Jun 20, 2022

Hi @BenZonneveld, thank you very much.
I am a bit puzzled by the build error.
Can you build it locally for the Raspberry Pi 4?

@BenZonneveld
Copy link
Author

Replace my build.sh with the original. For testing I removed the extracting of the arm compiler and rebuilds of circle

@probonopd
Copy link
Owner

Can you please change that in https://github.com/BenZonneveld/MiniDexed/tree/SysExControl? Then the build in this Pull Request should "magically" start working.
Thanks!

BenZonneveld and others added 6 commits June 20, 2022 22:40
@probonopd
Copy link
Owner

Something weird is going on with the git submodules:

Cloning into '/home/runner/work/MiniDexed/MiniDexed/CMSIS_5'...
Cloning into '/home/runner/work/MiniDexed/MiniDexed/Synth_Dexed'...
Cloning into '/home/runner/work/MiniDexed/MiniDexed/circle-stdlib'...
Submodule path 'CMSIS_5': checked out '8a64562247485159a090ec4a01588f44f8d10311'
fatal: remote error: upload-pack: not our ref a3a696b003ef76f79dbd91ff8188a[12](https://github.com/probonopd/MiniDexed/runs/6975070899?check_suite_focus=true#step:3:13)3b74e5e17
fatal: Fetched in submodule path 'Synth_Dexed', but it did not contain a3a696b003ef76f79dbd91ff8188a123b74e5e17. Direct fetching of that commit failed.
Error: Process completed with exit code 128.

@probonopd
Copy link
Owner

probonopd commented Jun 20, 2022

@BenZonneveld would this fix
asb2m10/dexed#352?

@BenZonneveld
Copy link
Author

No...actually I removed the sending of sysex voices and only send the sysex on a patch request sysex, as it should be.
A program change shouldn't send a sysex with a voice dump.

@dcoredump
Copy link
Contributor

No...actually I removed the sending of sysex voices and only send the sysex on a patch request sysex, as it should be.
A program change shouldn't send a sysex with a voice dump.

I had added this because then the editors immediately display the correct data. Maybe this should be made configurable via minidexed.ini, because I found it a great help so far.

@probonopd
Copy link
Owner

Yes, we should definitely not remove this feature!

@BenZonneveld
Copy link
Author

No...actually I removed the sending of sysex voices and only send the sysex on a patch request sysex, as it should be.
A program change shouldn't send a sysex with a voice dump.

I had added this because then the editors immediately display the correct data. Maybe this should be made configurable via minidexed.ini, because I found it a great help so far.

Still, the normal way for an editor would be to do a patch request. It is what my MiniDexed master does: receive pgm change, request voice data.

If you always send the sysex voicedump it would mean I can never use MiniDexed with for example my DX7

@probonopd
Copy link
Owner

It could be made configurable.

@BenZonneveld
Copy link
Author

And there we have a building branch with my patches.

@probonopd
Copy link
Owner

probonopd commented Jul 6, 2022

It builds! 👍

Can you describe what exactly this PR fixes/improves?

I figured out:

  • More MIDI commands received via serial are interpreted
  • More MIDI control change messages are sent via serial
  • Convert NL+CR to NL on serial output

@probonopd
Copy link
Owner

probonopd commented Jul 17, 2022

Would #311 do the same job instead of having a private copy of Circle's serial.h? If possible I would like to avoid a private copy of that file, in order not to have the burden of maintenance. It looks to me like #311 also converts NL+CR to NL on serial output, but without the need for a patched serial.h.

@probonopd
Copy link
Owner

I had added this because then the editors immediately display the correct data. Maybe this should be made configurable via minidexed.ini, because I found it a great help so far.

...or we ask editors (yes I am thinking of you, Dexed) to send a patch request, like @BenZonneveld is saying? This is probably the way it should be done in MIDI.

@psethwick
Copy link

I tracked down the reason that my M-Audio Oxygen49 quits working after a program change to: receiving the sysex data afterwards, so +1 for configuration parameter, at least!

My keyboard probably shouldn't crash, but, well, it does. Probably something to do with its own memory dump feature.

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

Successfully merging this pull request may close these issues.

4 participants