Intermittent freeze on Teensy 4.1 when trying to send CC from inside loop()?

[doctea]

Hi! I have uClock working well in my Teensy USB MIDI project, except if I try to sendControlChange from inside loop(). If I do that then it doesn't last long before the program freezes.

I think its hitting some kind of race condition with USB, as Teensy CrashReport doesn't see to get triggered, and in certain cases its possible to avoid the freeze - or to recover from the frozen state - by connecting the USB Serial monitor. (I've only been able to replicate this in my 'full' project though.)

I'm using utils/atomic.h ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {}, which as far as I understand should handle enabling and disabling interrupts, and prevent concurrency related crashes -- wrapping sections of my loop() code in this macro has solved every crash so far except this one. I've also tried using __disable_irqs and __enable_irqs instead, but I get the same freezes regardless.

My use case here is that there are CCs that I need to send separately from the main clock ticks.

I've made a simple test sketch replicating the problem, attached here. Hopefully this will make it easy to replicate the problem. Defining SEND_CCS_IN_LOOP to 'true' will cause the Teensy to freeze within not much time, a minute at most. Defining it to 'false' seems to run indefinitely.

My test setup is with the Teensy 4.1 USB Host port connected to two USB hubs, and with multiple USB MIDI devices attached to the hubs. It doesn't seem to matter much which devices are connected. Curious whether the same behaviour is replicated on other hardware.

Any ideas what could be causing this, and hopefully how to avoid it, please? :)

/* based on USB MIDI Sync Box example
*/

// switch this between true / false to see problem
// when true, Teensy will freeze up intermittently
// when false, Teensy will run successfully forever
#define SEND_CCS_IN_LOOP true // false

// for attempting to disable interrupts while doing loop() stuff
#include <util/atomic.h>

#include <uClock.h>

// for using Teensy USB MIDI device 
#include "USBHost_t36.h"
#include <MIDI.h>

USBHost myusb;
USBHub hub1(myusb);
USBHub hub2(myusb);
USBHub hub3(myusb);
KeyboardController keyboard1(myusb);
KeyboardController keyboard2(myusb);
MIDIDevice midi1(myusb);
MIDIDevice midi2(myusb);
MIDIDevice midi3(myusb);
MIDIDevice midi4(myusb);
MIDIDevice midi5(myusb);
MIDIDevice midi6(myusb);
MIDIDevice midi7(myusb);
MIDIDevice midi8(myusb);

uint32_t loop_counter = 0;
uint32_t tick_counter = 0;

uint8_t bpm_blink_timer = 1;
void handle_bpm_led(uint32_t tick)
{
  // BPM led indicator
  if ( !(tick % (96)) || (tick == 1) ) {  // first compass step will flash longer
    bpm_blink_timer = 8;
    digitalWrite(LED_BUILTIN, HIGH);
  } else if ( !(tick % (24)) ) {   // each quarter led on
    digitalWrite(LED_BUILTIN, HIGH);
  } else if ( !(tick % bpm_blink_timer) ) { // get led off
    digitalWrite(LED_BUILTIN, LOW);
    bpm_blink_timer = 1;
  }
}

// Internal clock handlers
void ClockOut96PPQN(uint32_t tick) {
  // Send MIDI_CLOCK to external gears
  //myusb.sendRealTime(midi::Clock);
  handle_bpm_led(tick);

  tick_counter++;

  int8_t note_on = tick_counter % 127;
  int8_t note_off = (tick_counter-6) % 127;

  midi1.sendRealTime(midi::Clock);
  midi2.sendRealTime(midi::Clock);
  midi3.sendRealTime(midi::Clock);
  midi4.sendRealTime(midi::Clock);
  midi5.sendRealTime(midi::Clock);
  midi6.sendRealTime(midi::Clock);
  midi7.sendRealTime(midi::Clock);
  midi8.sendRealTime(midi::Clock);

  midi1.sendNoteOn(note_on, 127, 1);
  midi2.sendNoteOn(note_on, 127, 1);
  midi3.sendNoteOn(note_on, 127, 1);
  midi4.sendNoteOn(note_on, 127, 1);
  midi5.sendNoteOn(note_on, 127, 1);
  midi6.sendNoteOn(note_on, 127, 1);
  midi7.sendNoteOn(note_on, 127, 1);
  midi8.sendNoteOn(note_on, 127, 1);

  midi1.sendNoteOff(note_off, 0, 1);
  midi2.sendNoteOff(note_off, 0, 1);
  midi3.sendNoteOff(note_off, 0, 1);
  midi4.sendNoteOff(note_off, 0, 1);
  midi5.sendNoteOff(note_off, 0, 1);
  midi6.sendNoteOff(note_off, 0, 1);
  midi7.sendNoteOff(note_off, 0, 1);
  midi8.sendNoteOff(note_off, 0, 1);

  /*if (random(100)<1) {
      midi1.sendControlChange(random(127), random(127), 1);
      midi2.sendControlChange(random(127), random(127), 1);
      midi3.sendControlChange(random(127), random(127), 1);
      midi4.sendControlChange(random(127), random(127), 1);
      midi5.sendControlChange(random(127), random(127), 1);
      midi6.sendControlChange(random(127), random(127), 1);
      midi7.sendControlChange(random(127), random(127), 1);
      midi8.sendControlChange(random(127), random(127), 1);
    }*/

}

void onClockStart() {
  //myusb.sendRealTime(myUsb.Start);
  midi1.sendRealTime(midi::Start);
  midi2.sendRealTime(midi::Start);
  midi3.sendRealTime(midi::Start);
  midi4.sendRealTime(midi::Start);
  midi5.sendRealTime(midi::Start);
  midi6.sendRealTime(midi::Start);
  midi7.sendRealTime(midi::Start);
  midi8.sendRealTime(midi::Start);
}

void onClockStop() {
  //myUsb.sendRealTime(myUsb.Stop);
  midi1.sendRealTime(midi::Stop);
  midi2.sendRealTime(midi::Stop);
  midi3.sendRealTime(midi::Stop);
  midi4.sendRealTime(midi::Stop);
  midi5.sendRealTime(midi::Stop);
  midi6.sendRealTime(midi::Stop);
  midi7.sendRealTime(midi::Stop);
  midi8.sendRealTime(midi::Stop);
}

void setup() {
  // A led to count bpms
  pinMode(LED_BUILTIN, OUTPUT);

  Serial.begin(115200);
  Serial.println("BOOTING");

    myusb.begin();

  // Setup our clock system
  // Inits the clock
  uClock.init();
  // Set the callback function for the clock output to send MIDI Sync message.
  uClock.setClock96PPQNOutput(ClockOut96PPQN);
  // Set the callback function for MIDI Start and Stop messages.
  uClock.setOnClockStartOutput(onClockStart);  
  uClock.setOnClockStopOutput(onClockStop);
  // Set the clock BPM to 126 BPM
  uClock.setTempo(90);
  // Starts the clock, tick-tac-tick-tac..
  uClock.start();
}

// Do it whatever to interface with Clock.stop(), Clock.start(), Clock.setTempo() and integrate your environment..
void loop() {
  loop_counter++;
  ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
      myusb.Task();
  }
  ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
    midi1.read();
    midi2.read();
    midi3.read();
    midi4.read();
    midi5.read();
    midi6.read();
    midi7.read();
    midi8.read();
  }

  ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
    if (SEND_CCS_IN_LOOP && random(100)<1) {
      // weirdly, in my 'full proper project' that has other stuff going on, adding this 
      // serial debug output and connecting to serial monitor seems to prevent the 'freeze'...
      // seems to have no effect in this simple test case though
      Serial.printf("sending some random control changes: %u\n", loop_counter);
      Serial.flush();

      midi1.sendControlChange(random(127), random(127), 1);
      midi2.sendControlChange(random(127), random(127), 1);
      midi3.sendControlChange(random(127), random(127), 1);
      midi4.sendControlChange(random(127), random(127), 1);
      midi5.sendControlChange(random(127), random(127), 1);
      midi6.sendControlChange(random(127), random(127), 1);
      midi7.sendControlChange(random(127), random(127), 1);
      midi8.sendControlChange(random(127), random(127), 1);

      /*midi1.sendNoteOn(random(127), random(127), 1);
      midi2.sendNoteOn(random(127), random(127), 1);
      midi3.sendNoteOn(random(127), random(127), 1);
      midi4.sendNoteOn(random(127), random(127), 1);
      midi5.sendNoteOn(random(127), random(127), 1);
      midi6.sendNoteOn(random(127), random(127), 1);
      midi7.sendNoteOn(random(127), random(127), 1);
      midi8.sendNoteOn(random(127), random(127), 1);*/

    }
  }

}

[midilab]

hi @doctea, you're probrably getting a race condition accessing MIDI interface over concurrent functions ClockOut96PPQN and loop.

uClock has a atomic macro for those scenarios, please use it: ATOMIC(X)

dont use ATOMIC over big code blocks(more than one call)! you can mess hardly with your clock signal, instead try a ATOMIC(midix.*) per call, in that way you dont lock processor on the loop() to the point of lossing the next tick call of interruption to clock your setup.

Try to avoid process things inside ATOMIC(X), for example: so instead of do ATOMIC over this call that will runs ramdom twice, process random outside and use a variable:

//midi1.sendControlChange(random(127), random(127), 1);

uint8_t random_a = random(127);
uint8_t random_b = random(127);
ATOMIC(midi1.sendControlChange(random_a, random_b, 1))

ATOMIC always should be use carefully otherwise you can mess hardly with your clock signal.

[doctea]

Thanks @midilab for the fast response! I'll try this out to see if a more granular ATOMIC(X) approach helps.

[doctea]

Hm, so doing like below instead still freezes up in the same way:-

#define ATOMIC(X) noInterrupts(); X; interrupts();
// ...
      uint8_t random_a = random(127);
      uint8_t random_b = random(127);
      ATOMIC(midi1.sendControlChange(random_a, random_b, 1) );
      ATOMIC(midi2.sendControlChange(random_a, random_b, 1) );
      ATOMIC(midi3.sendControlChange(random_a, random_b, 1) );
      ATOMIC(midi4.sendControlChange(random_a, random_b, 1) );
      ATOMIC(midi5.sendControlChange(random_a, random_b, 1) );
      ATOMIC(midi6.sendControlChange(random_a, random_b, 1) );
      ATOMIC(midi7.sendControlChange(random_a, random_b, 1) );
      ATOMIC(midi8.sendControlChange(random_a, random_b, 1) );

FWIW in my full project everything works brilliantly and without much clock jitter, even with quite large blocks of ATOMIC_BLOCKs during loop(), the clock is very accurate. Even sending to serial MIDI DIN devices with the same logic seems to work fine. As far as I can tell it is only when the target is a USB MIDI device that these freezes happen.

I thought it could be related to how USBHost_t36 MIDIDeviceBase::sendControlChange() function calls MIDIDeviceBase::write_packed() (https://github.com/PaulStoffregen/USBHost_t36/blob/b6f94f7605b3f6cb57a10e41cab51b25b47f7736/midi.cpp#L292), which will mean that interrupts get turned off and back on again even within the supposedly ATOMIC() operation. I've tinkered with how the interrupts are disabled/restored in that function, without any luck.

Do you have any further insight as to what could be happening and how a solution could be found? Might this be something that the USBHost_t36 maintainer might be able to help with?

Thanks

[doctea]

I've restructured my full app so that it now sends all CCs during the ClockOut96PPQN callback, instead of during loop(). Its been running fine with no freeze, and no impact that I can notice on timing or performance. This is an OK workaround for me for now, but it would be great long term if we could figure out what is causing this freeze so that it is possible to also send USB MIDI from outside of the callback.

Thanks for all your help so far and for this very handy library!

Unrelated to this issue, but I wondered if you had seen my post on https://github.com/midilab/uClock/issues/10 about whether it could be possible to make the library work on RP2040/Pico?

Thanks again

[doctea]

Hmm - some further exploration and I /might/ have figured out a patch to the USBHost_t36 library that solves this -- I won't have chance to more thoroughly test it until the coming Tuesday though -- will report back with more info when I've had a chance to verify what's going on!

[doctea]

OK, so my patched USBHost_t36 library at https://github.com/doctea/USBHost_t36/tree/merge-interrupt-fix seems to solve this.

The important change here is that interrupts are only re-enabled when sending USB MIDI data if they were already enabled in the first place. This prevents interrupts being unexpectedly turned on when they shouldn't be and (I presume) causing some kind of re-entrant problem?

I believe this solves the freeze in the test sketch I posted above.

The branch also contains some other changes that work around freezes and crashes that I see in my full app.

[midilab]

great! thanks for testing and provide those information, i will try to incorporate to troubleshooting doc.