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

Incoming message length in callback is 0? #333

Open
kisse66 opened this issue Sep 2, 2023 · 5 comments
Open

Incoming message length in callback is 0? #333

kisse66 opened this issue Sep 2, 2023 · 5 comments
Labels

Comments

@kisse66
Copy link

kisse66 commented Sep 2, 2023

Perhaps I have missed something, but it looks like there is an issue with using message callback (midi::Message)?

I'm trying to make a midi router (patchbay) on a Teensy 4 (7 serial ports) and use setHandleMessage() to get incoming messages and decide what to do. Assumption is the incoming message could be forwarded as is. Wrong?

It almost works as expected, but if I just pass the incoming midi::Message& to .send() nothing happens (no valid message sent).
This seems to be because incoming msg.length is 0:
"MIDI MSG: ch 1 type 192 data1 54 data2 0 valid 1 len 0" (debug print).

if I pass that to send() it does not work. If instead of send(msg) using something like

    //g_ports[port]->send(msg);   NOT working length 0 ??
    g_ports[port]->send(msg.type, msg.data1, msg.data2, msg.channel);

does work (except for sysex). Looks like send() is using the .length and thus fails. But why the incoming length is 0 on a valid message?
Other fields look valid.

Teensy 4 (seems to be same for STM32 at least), latest platformio, MIDI 5.0.2 (latest as fetched by platformio).

g_ports is an array of pointers to midi instances like
midi::MidiInterface<midi::SerialMIDI<HardwareSerial>, MyMidiSettings> *g_ports[NUM_PORTS] = { &midi1, &midi2, &midi3, &midi4, &midi5, &midi6, &midi7 };

Instances created via e.g.

midi::SerialMIDI<HardwareSerial> serialmidi1(Serial1);
midi::MidiInterface<midi::SerialMIDI<HardwareSerial>, MyMidiSettings> midi1(serialmidi1);

custom settings used to increase sysex size mainly (SYSEX_BUF_SIZE 768).

struct MyMidiSettings : public midi::DefaultSettings
{
    static const unsigned SysExMaxSize = MIDIROUTER::SYSEX_BUF_SIZE; // the default is too small
    static const bool UseRunningStatus = false;
    static const bool Use1ByteParsing = false;
};
@kisse66 kisse66 added the bug label Sep 2, 2023
@franky47
Copy link
Member

franky47 commented Sep 5, 2023

While working on refactoring the SysEx code, I found that this field seems to only be used for SysEx, which doubles up with the length being encoded in data1 and data2. So there is indeed a bug here.

For your use-case, the upcoming filter and map Thru feature would be a perfect fit, I'm hoping to get it shipped by the end of the month.

@kisse66
Copy link
Author

kisse66 commented Sep 5, 2023

Thanks for looking at this and pointing out the new feature. Was looking through all issues, but that title didn't trigger my eye. Great, will test as available. Could help my case also yes.

@rbarreiros
Copy link

rbarreiros commented Oct 14, 2023

I'm having the same issue, working on a AppleMIDI/SerialMIDI bridge, and it could be quite easy with setHandleMessage, but after a couple of days debugging I found that length is wrong, and send() checks for length to send data1/data2. I haven't tested SysEX yet.

Anyone managed to find the culprit?

@rbarreiros
Copy link

Oh, now I understand franky47 comment, length isn't updated at all during parse(). I managed to do that, might make a PR later or I can just paste a diff, might be quicker as it's just a couple of lines, unless @franky47 has already fixed the issue somewhere!

@franky47
Copy link
Member

Go ahead for a PR, but please base it off feat/v5.1.0. Thanks!

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

No branches or pull requests

3 participants