potential bug in TeensyUsbSlaveMidiClockMonitor example about uclock HOT 30 CLOSED

Did your pull request solve this issue?

from uclock.

Hey @midilab No - this issue came after as the code would not compile previously due to the volatile reference pointer.

The current TeensyUsbSlaveMidiClockMonitor example code compiles and works but with the above issues. I have also tested this example with DIN MIDI instead of usbMIDI and the same issues occur.

from uclock.

i will check asap, thanks for test and report!

from uclock.

No problem and thank you!

from uclock.

Hi @midilab ,

A quick update to this issue with more detail: The same issue occurs with the Xiao as well. I also forgot to mention that even though the tempo LED doesn't work after 42 bpm (testing using Abelton link from iOS).

I also tried both options for setting external sync mode and no difference:
uClock.setMode(1)
uClock.setMode(uClock.EXTERNAL_CLOCK)

Below is the UsbSlaveMidiClockMonitor sketch modified for the Xiao:

#include <Arduino.h>

/* USB MIDI Sync Slave Box Monitor
 *  
 * This example demonstrates how to create a
 * MIDI hid compilant slave clock box with 
 * monitor support using oled displays
 *
 * Making use of a 250 usceconds timer to
 * handle MIDI input to avoid jitter on clock
 * 
 * You need the following libraries to make it work
 * - u8g2
 * - uClock
 *
 * This example code is in the public domain.
 */
 
#include <U8x8lib.h>

#include <Adafruit_TinyUSB.h>
#include <MIDI.h>

// USB MIDI object
Adafruit_USBD_MIDI usb_midi;
 
// Create a new instance of the Arduino MIDI Library and attach usb_midi as the transport.
MIDI_CREATE_INSTANCE(Adafruit_USBD_MIDI, usb_midi, MIDI_USB); // Type, SerialPort, Name

//
// BPM Clock support
//
#include <uClock.h>

U8X8 * u8x8;

// IntervalTimer teensyTimer;

// MIDI clock, start, stop, note on and note off byte definitions - based on MIDI 1.0 Standards.
#define MIDI_CLOCK 0xF8
#define MIDI_START 0xFA
#define MIDI_STOP  0xFC

char bpm_str[4];
float bpm = 126;
uint8_t bpm_blink_timer = 1;
uint8_t clock_state = 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
  MIDI_USB.sendClock(); 
  // MIDI_USB.sendClock(); // MIDI_USB.sendRealTime(midi::Clock);
  handle_bpm_led(tick);
}

void onClockStart() {
  MIDI_USB.sendStart();
  // MIDI_USB.sendStart(); //MIDI_USB.sendRealTime(midi::Start);
}

void onClockStop() {
  MIDI_USB.sendStop();
  // MIDI_USB.sendStop(); // MIDI_USB.sendRealTime(midi::Stop);
  digitalWrite(LED_BUILTIN, LOW);
}

// External clock handlers
void onExternalClock()
{
  uClock.clockMe();
}

void onExternalStart()
{
  uClock.start();
}

void onExternalStop()
{
  uClock.stop();
}

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

  //
  // MIDI setup
  //
  MIDI_USB.begin();
  MIDI_USB.setHandleClock(onExternalClock);
  MIDI_USB.setHandleStart(onExternalStart);
  MIDI_USB.setHandleStop(onExternalStop);

  //
  // OLED setup
  // Please check you oled model to correctly init
  //
  u8x8 = new U8X8_SH1106_128X64_NONAME_HW_I2C(U8X8_PIN_NONE);
  u8x8->begin();
  u8x8->setFont(u8x8_font_pressstart2p_r); 
  u8x8->clear();
  u8x8->drawUTF8(0, 0, "uClock"); 

  //
  // uClock Setup
  //
  // Setup our clock system
  uClock.init();
  uClock.setClock96PPQNOutput(ClockOut96PPQN);
  // For MIDI Sync Start and Stop
  uClock.setOnClockStartOutput(onClockStart);
  uClock.setOnClockStopOutput(onClockStop);
  
  // uClock.setMode(uClock.EXTERNAL_CLOCK); // is this same method as uClock.setMode(1) ?

  // Teensy specific
  // make use of 250us timer to handle midi input sync
  // teensyTimer.begin(handleMidiInput, 250); 
  // teensyTimer.priority(80);

  // set to external sync mode
  // NOTE: this is used by the example here: https://github.com/midilab/uClock/blob/master/examples/TeensyUsbSlaveMidiClock/TeensyUsbSlaveMidiClock.ino
  uClock.setMode(1);

  // wait until device mounted
  while( !USBDevice.mounted() ) delay(1);
}

void handleMidiInput() {
  while (MIDI_USB.read()) {
  }
}

void printBpm(float _bpm, uint8_t col, uint8_t line) {
    int b = (int)_bpm;
    int c = (int)((_bpm-b)*10);
    itoa(b, bpm_str, 10);
    if (b > 99) {
      u8x8->drawUTF8(col, line, bpm_str);
    } else {
      bpm_str[2] = '\0';
      u8x8->drawUTF8(col, line, " ");
      u8x8->drawUTF8(col+1, line, bpm_str);
    }
    u8x8->drawUTF8(col+3, line, ".");
    itoa(c, bpm_str, 10);
    u8x8->drawUTF8(col+4, line, bpm_str);
    u8x8->drawUTF8(col+5, line, "bpm"); 
}

void loop() {

  handleMidiInput();

  if (bpm != uClock.getTempo()) {
    bpm = uClock.getTempo();
    printBpm(bpm, 8, 7);
  }
  if (clock_state != uClock.state) { 
    clock_state = uClock.state;
    if (clock_state >= 1) {
      u8x8->drawUTF8(0, 7, "playing"); 
    } else { 
      u8x8->drawUTF8(0, 7, "stopped "); 
    }
  }
}

from uclock.

Hi @midilab,

I just wanted to report back on some more testing that I tried. So far I have had no luck with resolving the clock freezing issue mentioned above for USB MIDI for the Teensy LC. However, I was able to get it working with DIN MIDI on the Teensy LC. Although the LED Blink timer was not working correctly (out of sync), changing this line in uClock.h to
EXTERNAL_CLOCK = 0 seems to resolve the clock freezing at an initial BPM beyond 42BPM. For example: starting at 300 BPM there is no freezing after initial clock bytes are read. I am not sure if this helps resolve the issue or not but perhaps this provides some clues.

from uclock.

thanks for the input!

can you tell me what is freezing? its the clock generation or only the bpm time display?

i couldn't reproduce on my side using teensy lc.

clock at 20bpm looks ok, or maybe i didn't understand the bug.

from uclock.

Hey @midilab thank you for getting back with me on this issue.

can you tell me what is freezing? its the clock generation or only the bpm time display?

To confirm, I am using the example here .

The only adjustments I have made to the code are 1) adding my specific OLED model at line 99 and 2) moving the handleMidiInput function above the setup function so it will compile on VScode (Platformio).

For external clock, I am using Logic Pro X. Additionally, I have also tested on iOS using Abelton Link app to also send out external clock.

Here are the issues:
Starting at any BPM 20 or higher from an external clock source, The only display that I get on the OLED is uClock . There is no display of playing or stopped when start or stop messages are sent. This occurs if the BPM Monitor LED indicator function is enabled or disabled.

The Midi monitor on my Mac shows all transport (Start, Stop) and Clock messages being sent and received by the Teensy LC if the following condition is met:

1. Begin by setting the external clock to below 42 bpm (it may vary slightly for you?). At this point you can raise the bpm to any amount, even stop start the transport controls.

However, if you do the following, the clock generation freezes and there is still no display of the playing or stopped transport messages on the OLED. Only uClock is displayed on OLED:

1. upload the code to Teensy LC
2. Set external clock source to 42 bpm or higher
3. Start the MIDI transport

If the BPM Indicator LED function is enabled, the LED indicator is frozen in the ON state and the clock generation is frozen after a few clock bytes are sent. If the BPM Indicator LED function is disabled, the clock generation is also frozen after a few clock bytes are sent Here is the output from the MIDI monitor:

With and without BPM LED indicator function at 42bpm
12:03:25.941	From Teensy MIDI Port 1	Start		
12:03:25.959	From Teensy MIDI Port 1	Clock		
12:03:25.980	From Teensy MIDI Port 1	Clock	

After the first few clock bytes, no more clock bytes are registered.

I hope this helps clarify the issue. Thanks!

from uclock.

i couldn't reproduce this bug.

I test using teensy LC/Teensyduino and ableton live 9 as master clock via usb using the default example for teensy usb monitor.

Starting with 20bpm and raising up doesn't trigger the bug for me.

Did you try to compile using arduino/teensyduino environment instead of platformio?

thanks for the tests and inputs so far.

from uclock.

Hey - thanks for trying!

Did you try to compile using arduino/teensyduino environment instead of platformio?

I do not think I have tried arduino/teensyduino environment yet but will test and let you know.

Starting with 20bpm and raising up doesn't trigger the bug for me.

Just to confirm: what happens when you start at external bpm of 42 or higher first? This is where I ran into the issue: first power cycle on or compile, then setting external bpm to 42 or higher.

Thanks for your patience !

from uclock.

starting with 120bpm is ok, i test starting all over from 20bpm to 120bpm and all of then run fine.

no problem, keep posting so we can figure out what could be the problem, my best guess so far is that some compiler diferences and flags between teensyduino and platformio could be the root of the problem.

from uclock.

starting with 120bpm is ok, i test starting all over from 20bpm to 120bpm and all of then run fine.

no problem, keep posting so we can figure out what could be the problem, my best guess so far is that some compiler diferences and flags between teensyduino and platformio could be the root of the problem.

Hmmmn - ok, I tested after compiling with Arduino and Teensyduino and same issue! Arduino 1.8.19 and Teensyduino 1.56. It freezes the same way. I wonder if it is my hardware setup? I am testing with a prototype circuit board I designed that has an OLED and specifically designed for DIN MIDI. The design has worked with all the examples (modified for DIN MIDI) in the uClock library so I assumed it would work for USB only. I'll need to do a quick test on the breadboard with just a Teensy LC and OLED to rule out my hardware design.

from uclock.

@midilab I've done a basic breadboard with only Teensy LC and 128x32 OLED with the default TeensyUsbSlaveMidiClockMonitor example and I am still having the same issues:

1. BPM LED indicator still freezes at around 42 bpm (a little lower on Logic Pro X, i.e. 40bpm) using Abelton link app on iOS or Logic Pro X on Mac.
2. OLED only shows uClock on display (no BPM indicator or starting / stopped) at any external BPM

I am using Arduino 1.8.19 and Teensyduino 1.56 and the latest version of the u8g2 library downloaded from u8g2 repo.

I have not modified the default example and selecting USB Type: MIDI in the Arduino IDE.

I have tried two different Teensy LCs and two different USB cables.

Maybe I am missing something really obvious here but I am not having any success unfortunately.

from uclock.

what are the version of uclock are you testing on?

from uclock.

I have been using 1.1.1 from the releases: https://github.com/midilab/uClock/archive/refs/tags/v1.1.1.zip in the Arduino IDE and when I have compiled in VScode / Platformio, I have been using the git head URL to pull the uClock library down: https://github.com/midilab/uClock.git

from uclock.

i've just release the last master changes you have pull for examples for 1.1.2 version

i will try to test again soon and report back.

from uclock.

The library versions im using:
u8g2: 2.28.10

from uclock.

Thank you @midilab ! Sounds good on testing and thanks for confirming which version of u8g2 you are using. I will try and test soon too.

from uclock.

so finaly i was able to reproduce the bug. i also find the root of the problem, i will come up with a solution soon...

once again thanks for testing out!

from uclock.

the problem was on the printBpm() function on sketches of slave midi clock monitors examples.
i change the inplementation to avoid crashing at first clocks received.

so everything is on master branch, can you please test all the examples you have found bugged? if everything is ok i will release a new bugfix version for the library.

from uclock.

@midilab thank you and glad you were able to reproduce the bug! I have tried on the Arduino IDE using the master branch lib and example for TeensyUsbSlaveMidiClockMonitor and unfortunately nothing happens beyond uclock displaying on the oled. The LED does not even turn on or freeze. My MIDI monitor indicates clock is being sent to Teensy LC but it is not responding to stop, start or clock. I tested using u8g2: 2.28.10 and the latest version (2.33.15).

I've tested the TeensyUsbMasterMidiClock and TeensyUsbSlaveMidiClock and they continue to work with no issues.

from uclock.

my bad, reviewing the changes now i see that i forgot to change the size of bpm string buffer... i've just update the master branch with the new examples and they should be work now.

from uclock.

No problem and thank you! I just tested again and the bpm indicator LED , start, stop and clock are all now working great. However, the display of the tempo still does not display on the oled. It only shows uClock on the oled as before and the bpm, playing or stopped does not display.

from uclock.

I've commited a new version of midi monitor sketches get ride off printBpm function and make use of String class to avoid problems like that.

Is that possible to do some test again with the new commited version?

On my side looks ok on teensy, but i didn't test the leonardo version.

from uclock.

Thanks so much! I tested with the Teensy LC and it works with no issues! I do not have a Leonardo to test but I will also test on the Seeed Xiao and let you know if there are any issues.

One minor caveat is that the example code in the repo will not display on a 128x32 OLED since it is a smaller display size. In other words: the bpm value and playing / stopped messages will not be visible. I've modified the code below to show where the drawUTF8 y coordinate argument needs to be 3 or less to display on 128x32 oled

if (bpm != uClock.getTempo()) {
    bpm = uClock.getTempo();
    u8x8->drawUTF8(8, 3, String(bpm, 1).c_str());
    u8x8->drawUTF8(8+5, 3, "bpm");
    // clear display ghost number for 2 digit
    // coming from 3 digit bpm changes
    if (bpm < 100) {
      u8x8->drawUTF8(8+4, 3, " ");
    }
  }
  if (clock_state != uClock.state) { 
    clock_state = uClock.state;
    if (clock_state >= 1) {
      u8x8->drawUTF8(0, 3, "playing"); 
    } else { 
      u8x8->drawUTF8(0, 3, "stopped"); 
    }
  }

from uclock.

One other thought: is it possible to display only the values after the decimal point if it is not 0? For example:
97 is 97 (not 97.0)
but 97.6, 97.7. 97.8 etc still displays? Not a big issue but thought I would ask. Thanks!

from uclock.

thanks for the tests so far.

since this is a opensource project i always encourage people to contribute. if you think there is any improvements(like this example support for 128x32 oleds), feel free to just pull the changes and then we can discuss the changes over the pull request(thats keeps a better tracking schema over repository).

from uclock.

One other thought: is it possible to display only the values after the decimal point if it is not 0? For example:
97 is 97 (not 97.0)
but 97.6, 97.7. 97.8 etc still displays? Not a big issue but thought I would ask. Thanks!

yes you can change that, just a matter of handle String() parameters over bpm tempo float without decimal values.

something like:

if (bpm != uClock.getTempo()) {
    bpm = uClock.getTempo();
    uint16_t int_bpm = (uint16_t)bpm;
    float bpm_decomponse = bpm - int_bpm;
    if (bpm_decomponse == 0) {
      u8x8->drawUTF8(8, 3, String(int_bpm).c_str());
    } else {
        u8x8->drawUTF8(8, 3, String(bpm, 1).c_str());
    }
    u8x8->drawUTF8(8+5, 3, "bpm");
  }

from uclock.

thanks for the tests so far.

since this is a opensource project i always encourage people to contribute. if you think there is any improvements(like this example support for 128x32 oleds), feel free to just pull the changes and then we can discuss the changes over the pull request(thats keeps a better tracking schema over repository).

Sounds good - thank you! I'll test with the Xiao and work on a suitable pull request. Seems like some additional code commenting should do the trick for someone using a smaller oled screen should do the trick.

from uclock.

One other thought: is it possible to display only the values after the decimal point if it is not 0? For example:
97 is 97 (not 97.0)
but 97.6, 97.7. 97.8 etc still displays? Not a big issue but thought I would ask. Thanks!

yes you can change that, just a matter of handle String() parameters over bpm tempo float without decimal values.

something like:

if (bpm != uClock.getTempo()) {
    bpm = uClock.getTempo();
    uint16_t int_bpm = (uint16_t)bpm;
    float bpm_decomponse = bpm - int_bpm;
    if (bpm_decomponse == 0) {
      u8x8->drawUTF8(8, 3, String(int_bpm).c_str());
    } else {
        u8x8->drawUTF8(8, 3, String(bpm, 1).c_str());
    }
    u8x8->drawUTF8(8+5, 3, "bpm");
  }

Ah perfect - this is super helpful! Again thank you for all the support !

from uclock.