Sys ex control by BenZonneveld · Pull Request #287 · probonopd/MiniDexed

BenZonneveld · 2022-06-19T19:37:32Z

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

Update build.yml

Minor update to build yaml

probonopd · 2022-06-20T17:37:06Z

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 · 2022-06-20T18:03:45Z

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

probonopd · 2022-06-20T18:05:39Z

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!

Update

Revert changes unrelated to the PR

probonopd · 2022-06-20T22:43:11Z

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 · 2022-06-20T23:22:40Z

@BenZonneveld would this fix
asb2m10/dexed#352?

BenZonneveld · 2022-06-21T00:13:21Z

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 · 2022-06-21T05:16:29Z

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 · 2022-06-28T17:17:33Z

Yes, we should definitely not remove this feature!

BenZonneveld · 2022-07-05T13:31:36Z

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 · 2022-07-05T16:45:19Z

It could be made configurable.

into SysExControl

BenZonneveld · 2022-07-05T21:38:48Z

And there we have a building branch with my patches.

probonopd · 2022-07-06T22:32:50Z

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 · 2022-07-17T18:37:18Z

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 · 2022-07-17T18:39:08Z

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 · 2022-09-22T20:03:34Z

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.

Copilot

Copilot reviewed 8 out of 12 changed files in this pull request and generated 2 comments.

Files not reviewed (4)

Synth_Dexed: Language not supported
build.sh: Language not supported
circle-stdlib: Language not supported
rebuild.sh: Language not supported

Comments suppressed due to low confidence (1)

src/minidexed.cpp:449

Correct the typo in the log message by changing 'Resonanece' to 'Resonance'.

LOGDBG("Set Resonanece for TG %i", instanceID);

Copilot · 2025-04-14T16:59:09Z

src/mididevice.cpp

+void CMIDIDevice::SendCtrlChange14Bit(uint8_t ctrl, int16_t val, uint8_t nTG)
+{
+    uint8_t lsb = (val & 0x7f);
+    uint8_t msb = (val >> 9)&0x7f;


The shift amount in SendCtrlChange14Bit appears incorrect; for a 14-bit value the MSB should be extracted using (val >> 7) instead of (val >> 9).

Suggested change

uint8_t msb = (val >> 9)&0x7f;

uint8_t msb = (val >> 7)&0x7f;

Copilot · 2025-04-14T16:59:09Z

src/mididevice.cpp

+  // send voice dump to all MIDI interfaces
+  for(Iterator = s_DeviceMap.begin(); Iterator != s_DeviceMap.end(); ++Iterator)
+  {
+    Iterator->second->Send (banksysex, sizeof(banksysex)*sizeof(uint8_t));


The size argument in SendBankName multiplies sizeof(banksysex) by sizeof(uint8_t), which is redundant since sizeof(uint8_t) is always 1; it should be simplified to sizeof(banksysex).

Suggested change

Iterator->second->Send (banksysex, sizeof(banksysex)*sizeof(uint8_t));

Iterator->second->Send (banksysex, sizeof(banksysex));

probonopd · 2025-04-22T18:48:01Z

The description I wish this PR would have had to begin with:

Add support for

Configuration dumps
Bank name requests
Improved alignment between SysEx messages and tone generators
Enhanced debugging and logging
Streamlined SysEx voice dumping and parameter updates

1. Changes to `src/mididevice.cpp`

a. Updated SysEx Handling

The HandleSystemExclusive method in CMIDIDevice now includes:

Instance ID Validation: Ensures that the SysEx messages align with the correct tone generator (TG) by comparing the SysEx instance ID (pMessage[2] & 0x0F) with the TG (nTG). A warning is logged if they do not match:
```
uint8_t instanceID = pMessage[2] & 0xF;
if (instanceID != nTG) {
    printf("WARNING instanceID and nTG do not match!!!!!\n");
}
```
Support for New SysEx Requests:
- Config Request: When SysEx ID 0x30 is received, it triggers a configuration dump.
- Bank Name Request: When SysEx ID 0x40 is received, it sends the name of the current bank.

b. New SysEx Responses

Configuration Request (SendSystemExclusiveConfig):
- Assembles a detailed configuration dump containing various tone generator parameters, including:
  - Compressor enable/disable.
  - Reverb settings (size, damp, low-pass, diffusion, level).
  - Volume, pan, master tune, cutoff, resonance, reverb send, pitch bend, portamento mode, time, etc.
- Sends this configuration as a SysEx message to all MIDI devices.
Bank Name Request (SendBankName):
- Constructs a SysEx message containing the bank name for a specific tone generator.
- Extracts the bank name using:
```
m_pSynthesizer->GetSysExFileLoader()->GetBankName(...)
```
- Sends the message to all MIDI interfaces.
Voice Dump (SendSystemExclusiveVoice):
- Refactored to use the instance ID instead of the TG directly, ensuring better alignment with SysEx messages.

2. Changes to `src/mididevice.h`

a. New SysEx-Related Methods

The following methods were added to the CMIDIDevice class:

SendSystemExclusiveConfig(): Sends a SysEx message containing the configuration of the synthesizer and tone generators.
SendBankName(uint8_t nTG): Sends the name of the current bank via SysEx.
Updated SendSystemExclusiveVoice to take only nVoice and nTG as parameters (removes nCable).

3. Changes to `src/minidexed.cpp`

a. SysEx Enhancements

Updated SysEx-related methods to ensure consistency with the new instance ID approach:
- The getSysExVoiceDump method now sets the instance ID (nTG) in the SysEx message instead of the MIDI channel:
```
dest[2] = nTG; // Sub-status and MIDI channel/Instance ID
```
Improved parameter change handling to trigger corresponding SysEx updates:
- For example, when the volume is updated, a MIDI Control Change (CC) message is sent:
```
m_SerialMIDI.SendCtrlChange(MIDI_CC_VOLUME, m_nVolume[nTG], nTG);
```

4. New MIDI Control Codes

Defined in src/mididevice.h:

Added new constants for MIDI Control Change (CC) messages:

#define MIDI_CC_BANK_SELECT_MSB    0
#define MIDI_CC_MODULATION         1
#define MIDI_CC_VOLUME             7
#define MIDI_CC_PAN_POSITION       10
#define MIDI_CC_BANK_SELECT_LSB    32
#define MIDI_CC_REVERB_LEVEL       91
#define MIDI_CC_DETUNE_LEVEL       94

5. Logging and Debugging

Enhanced debugging information for SysEx handling:
- Logs the return value from the SysEx handler:
```
LOGDBG("SYSEX handler return value: %d", sysex_return);
```
- Logs the details of the SysEx messages being processed, including the TG and instance ID.

probonopd · 2025-04-22T18:49:52Z

@diyelectromusic @soyersoyer what do you think about the proposed changes?

diyelectromusic · 2025-04-22T19:18:58Z

I think we need to isolate the key changes in this PR and show them applied to the latest code base - I'm struggling to see what depends on what... I mean what is with that serial.h for example? And all those build changes.

Also, I don't think we should be changing how the SysEx device number is interpreted without some discussion. We don't really have the concept in MiniDexed atm. I think we map it over to TG MIDI channel (rightly or wrongly).

Kevin

soyersoyer · 2025-04-22T20:53:54Z

Added new constants for MIDI Control Change (CC) messages:

These have since been implemented.

Is there any documentation on SysEX somewhere?

probonopd · 2025-04-22T23:16:09Z

Is there any documentation on SysEX somewhere?

You mean, like, in the DX7 manual?

BenZonneveld added 10 commits June 8, 2022 00:04

Update build.yml

85a663a

Update build.yml

Update build.yml

44d9c7e

Minor update to build yaml

Sysex patches

a1c9ec0

Patches to support TG settings using sysex.

b13c1f2

Sysex implemented

bbd3db3

More sysex implementations

d62cbb7

More midi

31c52b3

More sysex

6d189f8

Small sysex changes

d8316fd

Forgot to add these files

65ebb88

BenZonneveld and others added 6 commits June 20, 2022 22:40

Added extra files

959b123

This should fix the builds

d944ec4

Update build.yml

5ed8b18

Update build.yml

d10c784

Update

Restore build.yml

fd345d8

Revert changes unrelated to the PR

Restore build.sh

6351fdc

Revert changes unrelated to the PR

BenZonneveld added 4 commits July 5, 2022 21:56

Moving Sysex check to mididevice.cpp

cb0f524

This should fix the build issues

92d5f4c

Merge branch 'SysExControl' of https://github.com/BenZonneveld/MiniDexed

b87fd74

into SysExControl

Update rebuild.sh

637bcec

BenZonneveld added 6 commits July 5, 2022 22:04

Update rebuild.sh

1902c47

Upstream Dexed

cf03de9

Fixing build

613d721

Refactored the sysex check

5ee4469

Refactored the sysex check

3e9ae9b

Build Fix and Working Sysex

01ef041

The check was not correct, fixed

05f1e43

probonopd mentioned this pull request Jul 17, 2022

Support more MIDI messages #313

Closed

Merge branch 'main' into SysExControl

7c60100

probonopd mentioned this pull request Jul 17, 2022

Add call to set Serial options to prevent CR to CRLF conversion #311

Merged

probonopd requested a review from Copilot April 14, 2025 16:58

Copilot AI reviewed Apr 14, 2025

View reviewed changes

	uint8_t msb = (val >> 9)&0x7f;
	uint8_t msb = (val >> 7)&0x7f;

	Iterator->second->Send (banksysex, sizeof(banksysex)*sizeof(uint8_t));
	Iterator->second->Send (banksysex, sizeof(banksysex));

Conversation

BenZonneveld commented Jun 19, 2022

Uh oh!

probonopd commented Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BenZonneveld commented Jun 20, 2022

Uh oh!

probonopd commented Jun 20, 2022

Uh oh!

probonopd commented Jun 20, 2022

Uh oh!

probonopd commented Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BenZonneveld commented Jun 21, 2022

Uh oh!

dcoredump commented Jun 21, 2022

Uh oh!

probonopd commented Jun 28, 2022

Uh oh!

BenZonneveld commented Jul 5, 2022

Uh oh!

probonopd commented Jul 5, 2022

Uh oh!

BenZonneveld commented Jul 5, 2022

Uh oh!

probonopd commented Jul 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

probonopd commented Jul 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

probonopd commented Jul 17, 2022

Uh oh!

psethwick commented Sep 22, 2022

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

probonopd commented Apr 22, 2025

1. Changes to src/mididevice.cpp

a. Updated SysEx Handling

b. New SysEx Responses

2. Changes to src/mididevice.h

a. New SysEx-Related Methods

3. Changes to src/minidexed.cpp

a. SysEx Enhancements

4. New MIDI Control Codes

5. Logging and Debugging

Uh oh!

probonopd commented Apr 22, 2025

Uh oh!

diyelectromusic commented Apr 22, 2025

Uh oh!

soyersoyer commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

probonopd commented Apr 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

probonopd commented Jun 20, 2022 •

edited

Loading

probonopd commented Jun 20, 2022 •

edited

Loading

probonopd commented Jul 6, 2022 •

edited

Loading

probonopd commented Jul 17, 2022 •

edited

Loading

1. Changes to `src/mididevice.cpp`

2. Changes to `src/mididevice.h`

3. Changes to `src/minidexed.cpp`

soyersoyer commented Apr 22, 2025 •

edited

Loading