Skip to content

Data is “swallowed”...? #92

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

Open
jenkuki opened this issue Jul 20, 2024 · 15 comments
Open

Data is “swallowed”...? #92

jenkuki opened this issue Jul 20, 2024 · 15 comments

Comments

@jenkuki
Copy link

jenkuki commented Jul 20, 2024

Hi,

I have a problem with receiving MIDI data via BLE.
The task: the Arduino Nano 33 BLE should receive data from a MIDI interface (here: BLE), which is generated by the Android APP “MIDI Commander”. The Nano has to filter out and process the data that is important to it (these are NRPN Controller CC 98, 99 and 06) and forward all MIDI commands to other devices, which then do their part.
A Program Change and a total of 7 different Control Changes are sent (actually 3 of these are unnessesary, but the APP unfortunately sends them, although incorrectly - CC100, 101 and 38).
This works on a Uno and a ordinary Nano. I have written a test example for this (see ZIP): MIDI_NRPN-test.ino (Program Change is not processed)
The output is as it should be (format: Controller Number / value):
CC: 0 / 3
CC: 99 / 4
CC: 98 / 2
CC: 6 / 48
Maske: 10001000
Poti Soll: 48
CC: 38 / 0
CC: 101 / 127
CC: 100 / 127

Now the test example on the Nano 33 BLE: MIDI_BLE_test2.ino
The output is incorrect:
CC: 0 / 3
CC: 98 / 2
CC: 6 / 48
CC: 101 / 127
CC: 100 / 127

The CC99 is missing, so there is no further processing, as is the CC38 - but that's okay because it's wrong anyway.
This is just one example. Other send processes result in different outputs. Other controllers are then missing or the values for the controllers are incorrect.
So somehow something is “swallowed up”. What is the cause?

  • Are BLE and the serial interface interfering (timer?, interrupt?)?
  • Is the timing of the incoming signal wrong? Is it possible to control this? If so, where and how?
  • Is this sent at all or does the APP already “swallow” it? I don't know how to find out...

Best Regards

Uwe (jenkuki)

Device: Nano 33 BLE
IDE: Arduino IDE 2.3.2 on Ubuntu 22.04.4 LTS
BLE_Samples.zip

@anthem400
Copy link

anthem400 commented Feb 3, 2025

Hi,
I have a similar issue with midi CCs being swallowed on any channel other than 1. As far as I have got debugging, it appears the BLE stack is adding some dodgy bytes causing some data to be invalid. I'm still working on it, but any help would be appreciated. I'm using XIAO ESP32S3.
Regards,
Ant

@lathoub
Copy link
Owner

lathoub commented Feb 4, 2025

@jenkuki Please provide a condensed version of the source code that demonstrates the issue (for security reasons, I will not download .zip files)

From what I read, the cause is not related to the hardware (ESP32, Nano use different code in the /hardware folder), but related to how the incoming bytes are ordered for BLE.

You might want to put debug statements in the receive section in BLEMIDI_Transport.h

Also test the RunningStatus, that is turned on by default:

#define RUNNING_ENABLE

See what happens if you turn it off (just comment out the line or remove line 252)

@jenkuki
Copy link
Author

jenkuki commented Feb 4, 2025

Hi lathoub,
thanks for your feedback, here are the sketches:
The version for Nano

#include <MIDI.h>


MIDI_CREATE_DEFAULT_INSTANCE();

bool CCrcved = false;
byte CC_kind = 0;
byte CC_val = 0;
byte NRPN1 = 1;
byte NRPN2 = 1;
byte NRPN_val = 63;
byte NRPNcount = 0;
byte maske = 0;


void myHandleCC(byte in_channel, byte in_number, byte in_value)
{
    CCrcved = true;
    CC_kind = in_number;
    CC_val = in_value;

}


void Parse_CC() {
  switch (CC_kind) {
        case 99 :{
          NRPN1 = CC_val;
          NRPNcount ++;
          break;
        }
          case 98 :{
          NRPN2 = CC_val;
          NRPNcount ++;
          break;
        }  case 6 :{
          NRPN_val = CC_val;
          NRPNcount ++;
          break;
        }
  }

}

void Switchmask() {
  byte temp_mask = 0;

  if (1 <= NRPN1 <= 6) {
    bitSet(temp_mask, NRPN1-1);
    if (NRPN2 == 2) {
      bitSet (temp_mask, 7);
    }
    if (temp_mask != maske) {
      maske = temp_mask;
      Serial.print ("Maske: ");
      Serial.println(maske, BIN);
      Serial.print ("Poti Soll: ");
      Serial.println (NRPN_val);
      }
  }
  // save_vals();
  NRPNcount = 0;
}


// -----------------------------------------------------------------------------

void setup(){
  MIDI.setHandleControlChange(myHandleCC);
       
  MIDI.begin(MIDI_CHANNEL_OMNI);
  MIDI.turnThruOff();

  // very_first();

  Switchmask();
}

void loop()
{
    MIDI.read();

    if (CCrcved) {
      CCrcved = false;
      Serial.print("CC: ");
      Serial.print(CC_kind);
      Serial.print(" / ");
      Serial.println(CC_val);
      
      Parse_CC();
      }

    if (NRPNcount == 3){
      Switchmask();
    }

}

The version for Nano 33 BLE

#include <BLEMIDI_Transport.h>

// #include <hardware/BLEMIDI_ESP32_NimBLE.h>
// #include <hardware/BLEMIDI_ESP32.h>
// #include <hardware/BLEMIDI_nRF52.h>
#include <hardware/BLEMIDI_ArduinoBLE.h>

BLEMIDI_CREATE_DEFAULT_INSTANCE()

unsigned long t0 = millis();
bool isConnected = false;

bool CCrcved = false;
byte CC_kind = 0;
byte CC_val = 0;
byte NRPN1 = 1;
byte NRPN2 = 1;
byte NRPN_val = 63;
byte NRPNcount = 0;
byte maske = 0;


void myHandleCC(byte in_channel, byte in_number, byte in_value)
{
  CCrcved = true;
  CC_kind = in_number;
  CC_val = in_value;

}


void Parse_CC() {
  switch (CC_kind) {
        case 99 :{
          NRPN1 = CC_val;
          NRPNcount ++;
          break;
        }
          case 98 :{
          NRPN2 = CC_val;
          NRPNcount ++;
          break;
        }  case 6 :{
          NRPN_val = CC_val;
          NRPNcount ++;
          break;
        }
  }

}

void Switchmask() {
  byte temp_mask = 0;

  if (1 <= NRPN1 <= 6) {
    bitSet(temp_mask, NRPN1-1);
    if (NRPN2 == 2) {
      bitSet (temp_mask, 7);
    }
    if (temp_mask != maske) {
      maske = temp_mask;
      Serial.print ("Maske: ");
      Serial.print(maske, BIN);
      Serial.print ("  Poti Soll: ");
      Serial.println (NRPN_val);

      Parse_CC();
      }
  }

  NRPNcount = 0;
}

// -----------------------------------------------------------------------------
// When BLE connected, LED will turn on (indication that connection was successful) 
// -----------------------------------------------------------------------------

void setup()
{
  pinMode(LED_BUILTIN, OUTPUT);
  digitalWrite(LED_BUILTIN, LOW);

  BLEMIDI.setHandleConnected([]() {
    isConnected = true;
    digitalWrite(LED_BUILTIN, HIGH);
  });

  BLEMIDI.setHandleDisconnected([]() {
    isConnected = false;
    digitalWrite(LED_BUILTIN, LOW);
  });

  MIDI.setHandleControlChange(myHandleCC);

  MIDI.begin(MIDI_CHANNEL_OMNI);
  // MIDI.turnThruOn();
  
}

// -----------------------------------------------------------------------------
//
// -----------------------------------------------------------------------------
void loop()
{
  MIDI.read();

  if (isConnected && (millis() - t0) > 5000) {
    t0 = millis();
    Serial.println("verbunden...");
    }

  if (CCrcved) {
    CCrcved = false;
    Serial.print("CC: ");
    Serial.print(CC_kind);
    Serial.print(" / ");
    Serial.println(CC_val);

//  Parse_CC();
    }

  if (NRPNcount == 3){
    Switchmask();
    }
}

I'll test the #Running/Status in the next few days and get back to you. It may take a few days.

Regards

@anthem400
Copy link

Hi,
Thanks for all the responses - it was my bad, and cost me 3 days! Schoolboy error:
The key line was this:
BLE_MIDI.read(MIDI_CHANNEL_OMNI)

With no arguments, input defaults to channel 1. Doh!

On another note, I just had issues compiling with the latest BLEMIDI_Transport.h
-- lines 186 & 188: appear duplicated
-- lines 203 and 206 appear duplicated

I may have jumped the gun as I'm still testing, but it did at least compile with the duplicates removed.

Best regards,
Ant

@lathoub
Copy link
Owner

lathoub commented Feb 4, 2025

Thanks Ant.
Indeed, no arguments default to MIDI Channel 1. We wanted to make the argument mandatory, but that would cause compilation errors in early code

@lathoub
Copy link
Owner

lathoub commented Feb 4, 2025

duplicated error fixed - thx @anthem400

@jenkuki
Copy link
Author

jenkuki commented Feb 5, 2025

Hi lathoub,

I tested to comment out the #define RUNNING_ENABLE . It's in my library at line 245? (not actual version? I have 2.2.)

Unfortunately, this had no effect at all... What do do next?

regards
Jenkuki

@matpowel
Copy link

matpowel commented Apr 4, 2025

@jenkuki I have the same problem, I spent an hour delving into the buffering going on in both ArduinoBLE and BLE-MIDI and honestly I think it's all a little fragile. I would love to get this resolved, and am happy to help but a little time limited right now. My use case is to have the Nano set up as a Control Surface for Logic Pro (or any music DAW) so it receives a load of data, SysEx messages as well as notes/CCs, to keep the DAW and the fake "control surface" in sync. I can see a lot of data in the BLE Debug that doesn't make it to my handler methods, or sometimes shows up later (maybe after buffer fills?)

Did you ever resolve this?

@jenkuki
Copy link
Author

jenkuki commented Apr 4, 2025

@matpowel
Thank you for your answer to my problem. For my private purposes I have built a kind of router for different guitars on different outputs, which does this with CC-NRPM commands. PC and CC commands are also forwarded to an effects unit. This now works very well with an ordinary Nano. But I want to get rid of the annoying cable and control the whole thing via Bluetooth. The hardware with the option of adapting to 3.3V is already ready. All I really need to do is plug in the Nano-BLE and set a few jumpers...

No, I haven't made any progress with the software yet. A few tips on how to debug the data flow would help me. If you could help me with that, it would help a lot, so far I don't know how to do it most effectively.

Thanks in advance

Jenkuki

Translated with DeepL.com (free version)

@matpowel
Copy link

matpowel commented Apr 7, 2025

@jenkuki ok I went down the rabbit hole on this one and the short version is I ended up just writing my own thin wrapper around ArduinoBLE using the BLE MIDI spec and I now am receiving every message perfectly from Apple's built in BLE MIDI implementation (from a Macbook Pro or iOS device currently). Because it's a total custom re-write I don't know for sure if these changes will get this BLE-MIDI library working properly but you could try:

  1. I initialize the Characteristic differently, ended up using the BLECharacteristic instead of BLEStringCharacteristic. No big reason except that the best examples I found of how to implement BLE used that directly and I also have no idea why we would be converting a MIDI byte stream to strings before processing instead of just leaving them as what they are, byte streams.
  2. Set the value size to 239 instead of 20 when initializing (the 3rd argument to the BLECharacteristic initializer, the comment is misleading in this BLE-MIDI code. That would be set in hardware/BLEMIDI_ArduinoBLE.h for you. 239 is because I noticed MTU is always negotiated to 242 in the BT layer and there are 3 header bytes. I actually extended ArduinoBLE to update the valueSize when it changes which is kind of complicated but for now 239 seems to work perfectly at least for Apple devices.
  3. Setting UseRunningStatus = true in MIDI_Library/src/midi_Settings.h (that's the separate MIDI_Library you would have installed, not inside the BLE-MIDI
  4. Re-wrote the receiving/buffering paradigm to be faster and remove some extra polling in the available method etc.
  5. Tweaked the away ArduinoBLE does its polling, it felt like HCI::poll could get into a bit of a tight loop when receiving loads of data, but honestly I'm not sure if this is needed or not. Probably not.

I'm honestly not sure which change(s) were the critical ones, you may have success just implementing numbers 2 and 3 using this BLE-MIDI library. Worth a go. I may test it at some point myself just out of curiosity.

@jenkuki
Copy link
Author

jenkuki commented Apr 9, 2025

@matpowel
Thank you very much for your feedback. This gives me a few ways that I can try to solve my problem of “swallowed” messages.
Above all, the changes to the initialization seem promising to me. I will definitely try to implement this in the next few days - if I can find the time. I'll get back to you, even if I have difficulties with it and need your help again. thanks again...
Regards
Jenkuki

@lathoub
Copy link
Owner

lathoub commented Apr 9, 2025

Interesting changes indeed - please keep us posted, hopefully characteristic and initializer fixes the issue.

I need to check on the UseRunningStatus = true, this might also be a culprit (1byte parsing is true for all streaming protocols by default).

@matpowel
Copy link

matpowel commented Apr 9, 2025

Here is a PR I created into the ArduinoBLE repo with the changes for automatically adjusting valueSize, in case you want to use some of it:

arduino-libraries/ArduinoBLE#391

Usage in your app:

BLE.setEventHandler(BLEMtuChanged, onMtuChanged);
void onMtuChanged(BLEDevice central) {
  const auto mtu = central.getMTU();
  Serial.print("MTU changed to ");
  Serial.println(mtu);
  if (midiChar.setValueSize(mtu - 3)) {
    Serial.print("Successfully adjusted the BLE buffer to ");
    Serial.println(mtu - 3);
  } else {
    Serial.print("Failed to adjust the BLE buffer! Likely too much data was still in the buffer.");
  };
  //MIDI.setBufferSize(mtu - 3); // I haven't needed this yet, not sure if it's an optimization
}

@jenkuki
Copy link
Author

jenkuki commented Apr 11, 2025

@matpowel
So, the first test is done.
but I only tested points 2 and 3. Test 1 is too hot for me... I can't rewrite the whole library that belongs to lathoub.
First to test 3: the change in MIDI_Library/src/midi_Settings.h to static const bool UseRunningStatus = true; had no effect.
to test 2: first I set the parameter valueSize in BLEStringCharacteristic to 239 as recommended. No effect. Then I googled again and found that Android MTU is generally set to 517. So I set valueSize to 514 (517 - 3). This also had no effect.

The MIDI string is not that big. 1x PC and 8xCC makes only 26 bytes. Nevertheless, I sent only a single CC (Bank select CC0) once. It arrived perfectly every time. Then I added another CC (CC32). Both commands did not arrive at every attempt, again sometimes one of them was “swallowed”.

Now I have tried to comment out all Serial.print/ln() commands. I wanted to rule out the possibility of timing difficulties. To do this, I activated the Parse_CC() function in the loop. (see sketches above, I have now formatted them properly again...sorry). All three CC evnet 98, 99 and 06 must have arrived. Then they are evaluated and displayed with switchmask(). This sometimes works, but again not for every send. And sometimes the evaluation was not correct either.

Now I will try again to implement your changes with “onMTuChanged” to see how big MTU actually is.

Translated with DeepL.com (free version)

regards
Jenkuki

@jenkuki
Copy link
Author

jenkuki commented Apr 15, 2025

@matpowel
Dear Matt, I need some help, I am not a professional C++ programmer...
I have downloaded and installed the modified library from your repo. Then I checked all the files that have been changed to make sure that the correct version is installed - everything is OK.
Then I integrated your test program into my sketch and tried to compile it.
First I got the error message

Compilation error: 'midiChar' was not declared in this scope

OK, no problem, I'll just add the corresponding namespace
if (bleMidi::midiChar.setValueSize(mtu - 3)) {

the error from above is now gone, but: ...but now I get strange messages from the compiler, which aborts with the error:

home/uwe/.cache/arduino/sketches/A26A605AA6AB10A6AB9D143E3E580EBB/sketch/MIDI_BLE_test2.ino.cpp.o: In function onMtuChanged(BLEDevice)': /home/uwe/sketchbook/MIDI_BLE_test2/MIDI_BLE_test2.ino:77: undefined reference to BLEDevice::getMTU() const'
/home/uwe/sketchbook/MIDI_BLE_test2/MIDI_BLE_test2.ino:80: undefined reference to `BLECharacteristic::setValueSize(unsigned short)'
collect2: error: ld returned 1 exit status

That's very funny. It doesn't find the references? If I go to the identifier with the right mouse button and select “Go to definition”, it finds the corresponding codes in the header files. Why doesn't the compiler find them?

I am desperate...

Jenkuki

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

No branches or pull requests

4 participants