[FIXED] Note messages to controller disappear

mike · October 2, 2020, 1:22pm

I’m getting some very strange results with trying to control my Yaeltex via java code. One issue is that notes I send to the controller just seem to get ignored or dropped just before I shut down the program. In this case, I set all my grid buttons to some color, and then run initialize (which sends a note to each button with velocity=0 to shut them) and exit the program. I can see all the notes sent to the midi object, but the buttons do not turn off. occasionally, the first few buttons will be turned off.

if I run the debugger and step through the final initialize() call, then the notes go off, presumably because the program has not shut down. but if I wait with a Thread.sleep() after the initialize call, to give the controller time to respond, it doesn’t work. I should say I’ve never had an issue like this with other controllers.

what is even going on here? the only explanation I can think of is something like the midi notes being sent asynchronously and then getting killed when the program shuts down, but then the sleep() should take care of it.

Weirdly, if I send another note to the controller after the initialize (like to turn a button back on), then the whole initialize is applied, as is the additional note. it seems like some kind of buffer flush, but there’s no obvious reason why that last extra note should make a difference.

I’m running on firmware 0.13, downloaded from Yaeltex this morning.

Does the controller have some kind of debug log that could be enabled and accessed?

francoytx · September 20, 2020, 11:08pm

Hello @mike!

There can be a log, but rebuilding the firmware, and since you’ve done it successfully already, you could test this:

In line 440-444 there is a commented Serial print piece of code that will print all the incoming MIDI messages.

Uncommenting these lines, You can start by checking if all the notes you are sending to the controller get printed when they’re supposed to or if there are some missing.
Right below (lines 447-449) there is a message counter.

You could add this piece of code right below line 449 to print and reset the counter each second:

static uint32_t prevMillisCount = 0;

if (countOn && millis()-prevMillisCount > 1000){
    SerialUSB.print("Received "); 
    SerialUSB.print(msgCount); 
    SerialUSB.println(" msgs in the last second.");
    msgCount = 0;
    prevMillisCount = millis();
    countOn = false;
}

This adds less overhead than printing each message individually, if they are being sent really fast, but gives less information.

Hope this helps! If it doesn’t, let me know and we can further look into it.

mike · September 21, 2020, 12:44am

I see those lines in comms.ino, but how do I get at the logs?

francoytx · September 21, 2020, 1:41am

Sorry @mike, my bad, while in the Arduino IDE go to Tools -> Port and select the one that is named “Yaeltex Kilomux”

If you are on an apple OS, it will show up as dev/tty.usbmodem* and some number.
In Linux it will show up as /dev/ttyACM* or /dev/ttyUSB*

Then go to Tools -> Serial Monitor and open it.

Set the baud rate at the bottom of the screen to 250.000 bps.

Now you should see the printouts on the screen, just like a console.

mike · September 21, 2020, 7:45am

I haven’t tried the logs yet, but I just narrowed down what causes the problem, though I have no idea why.

My initialize function does this:

send note velocity=0 to all the buttons on ch1 (the 8x16 grid)
send note velocity=0 to the other buttons, on ch2
send value=0 to the 8 encoders, on ch1
send note velocity=0 to the encoder switches, on ch2

if I comment out the last part, sending notes to turn off the encoder switch feedback, then it works fine and everything is turned off. with that line in, it sometimes doesn’t work. if I move the first part, sending notes on ch1, to after the encoder notes, it works fine. or if I just send one more note after the encoders part, it works fine.

very confusing!

francoytx · September 21, 2020, 12:11pm

I’m confused as well

I will try to do the same you do and see if I get anything from the test.

francoytx · September 21, 2020, 5:06pm

@mike, using RtMIDI in Windows and Qt creator, I am using the following code to send notes like you stated. First notes 0-127 ch1 (grid), then notes 0-15 and 24-67 (other buttons) ch2, then CC 0-7 ch1, then notes 16-23 ch2 (enc switches).

I am sending values 127 to turn them all on first, and then 0 to turn them all off.
In the controller side I can’t detect any missing messages. I have less buttons than you, but at far as receiving them, the controller seems to be getting them all.

Maybe we have something similar to the bug discussed in Apparent overloading when moving RGB rotary encoder, which I’m still working on, and might be getting there soon.

        for(unsigned char i = 0; i < 128; i++){
            message.clear();
            message.push_back(144);
            message.push_back(i);
            message.push_back(127);
            midiout->sendMessage( &message );
        }

        for(unsigned char i = 0; i < 16; i++){
            message.clear();
            message.push_back(145);
            message.push_back(i);
            message.push_back(127);
            midiout->sendMessage( &message );
        }
        for(unsigned char i = 24; i < 68; i++){
            message.clear();
            message.push_back(145);
            message.push_back(i);
            message.push_back(127);
            midiout->sendMessage( &message );
        }
        for(unsigned char i = 0; i < 8; i++){
            message.clear();
            message.push_back(176);
            message.push_back(i);
            message.push_back(127);
            midiout->sendMessage( &message );
        }
        for(unsigned char i = 16; i < 24; i++){
            message.clear();
            message.push_back(145);
            message.push_back(i);
            message.push_back(127);
            midiout->sendMessage( &message );
        }

        Sleep(1000);

        for(unsigned char i = 0; i < 128; i++){
            message.clear();
            message.push_back(144);
            message.push_back(i);
            message.push_back(0);
            midiout->sendMessage( &message );
        }

        for(unsigned char i = 0; i < 16; i++){
            message.clear();
            message.push_back(145);
            message.push_back(i);
            message.push_back(0);
            midiout->sendMessage( &message );
        }
        for(unsigned char i = 24; i < 68; i++){
            message.clear();
            message.push_back(145);
            message.push_back(i);
            message.push_back(0);
            midiout->sendMessage( &message );
        }
        for(unsigned char i = 0; i < 8; i++){
            message.clear();
            message.push_back(176);
            message.push_back(i);
            message.push_back(0);
            midiout->sendMessage( &message );
        }
        for(unsigned char i = 16; i < 24; i++){
            message.clear();
            message.push_back(145);
            message.push_back(i);
            message.push_back(0);
            midiout->sendMessage( &message );
        }

mike · September 21, 2020, 5:29pm

Thanks for trying it out. I don’t think it would make a difference, but I changed the CCs to 16-23 on ch1. I also changed the switch settings to used the indexed color. Otherwise unchanged I think.

Do you think perhaps the messages to change the encoder values are interfering with other commands, as in the other bug? but it seems to work okay if I send just the CCs.

francoytx · September 21, 2020, 10:03pm

The bug in the other post, I think, seems to have to do with the communication between the main and the aux microcontrollers, not really with the MIDI reception.

If you can view the log of the messages arriving to your controller, then we’ll be able to have a clue as to where the messages are getting lost.

You said you have a Mac computer right?

This program lets you spy on an output MIDI port, to see the messages that are being sent to this output. It’d be very helpful to see what messages are actually coming out of your Java app, and then compare with the log of the messages going into the controller.

mike · September 22, 2020, 5:45am

Okay, I enabled the logging and turned on the console, and it looks like all the notes I’m sending are being logged there, even when the corresponding buttons don’t light up.

Also, currently, whether I send the knob values or switches doesn’t seem to matter, but adding a sleep(500 millis) before the pattern I send (which is setting each of the 128 buttons to its corresponding color in the color palette) makes it work.

francoytx · September 22, 2020, 2:30pm

Well that’s a good start, the controller receives everything.

Maybe the feedback updating then has to do with the stucked pixels issue on the other post.

You can try the test version of firmware 0.14 and check if it improves!

ytx-v2-firmware-aux-0-14-testing.bin (11.1 KB)
ytx-v2-firmware-main-0-14-testing.bin (74.5 KB)

gc3 · September 23, 2020, 7:14pm

EDIT: apparent fix a few posts down

I’m noticing something similar (possibly the same thing) with v0.14 main/aux (the binaries in the other thread, which may not be the most current commit in the bugfix branch).

If I send a bunch of feedback messages at maximum speed (like blanking my 16x6 grid, for example) then none of them update on the HW. But if I then pause for an arbitrarily long time and send another message, they all update together.

The obvious hypothesis is that during the initial flood of feedback commands, everything is making it to the SAMD11 and UpdateLED() is getting called correctly (and therefore updating the *pixels memory malloc'd by the NeoPixel library via setPixelColor()), but that pixelsShow() is not getting called on the relevant strip(s).

Once the flood is over, the next feedback command hits UpdateLED() (as the previous commands did), but this time pixelsShow() fires successfully, and the NeoPixels get the previous commands and the last command at the same time. So it is a delayed buffer flush, but the buffer is the one on the SAMD11.

gc3 · September 23, 2020, 9:22am

And here’s something really interesting: this problem goes away entirely (meaning that a much higher update rate is allowed) when I set the buttons back to fixed color feedback. The difference is dramatic.

Since the SAMD11 doesn’t know the difference between fixed color and value-to-color, the difference presumably has to be on the SAMD21. (EDIT: disproved my earlier hypothesis).

It’s puzzling because when I replace the code in the colorRangeEnable block:

colorR/G/B = pgm_read_byte(&gamma8[pgm_read_byte(&colorRangeTable[colorIndex][R/G/B_INDEX])]);

with the fixed-value code below:

colorR/G/B = pgm_read_byte(&gamma8[digital[indexChanged].feedback.color[R/G/B_INDEX]]);

the colors update as if fixed-value (of course) but the performance difference is identical. (I originally thought that the problem was with the colorRangeTable code, but it doesn’t seem to be).

If this is all right, different behavior in fillFrameWithDigitalData combined with a burst of feedback messages causes pixelsShow to get indefinitely deferred (which would almost be the opposite of the other problem!)

@mike, I gather you’re using value-to-color for everything. If you reset to fixed color (highly recommend editing the Kilowhat JSON, at least until batch update is added!) does that change anything?

francoytx · September 23, 2020, 12:15pm

You’re right! They are not I’m not the best with git, but the binaries were a stable version of the fix, and now I’m trying a few more stuff to improve the sync between MAIN and AUX (and a few more other fixes)

Ok, one thought that pops into my mind, is that the PROGMEM reads are taking too long.
Will try to prove this right or wrong soon.

If this is the case, then moving the colorRangeTable to RAM (as a temporary fix until we make this table editable in Kilowhat, and then there will be a RAM copy for sure) might make it better, and also make the gamma conversion happen somewhere else (before the colors leave Kilowhat for example).

Thanks for this finding @gc3!

gc3 · September 23, 2020, 4:53pm

Quick note: it’s baffling, but I don’t think it’s a timing difference (nothing’s ever simple! ).

I replaced the gamma and lookup in the VtoC mode with simple bitwise arithmetic implementing the 2-bit R, 2-bit G, 3-bit B table that @mike experimented with, and I even copied over the special case for 0 (since I noted that I was flooding the controller with set-to-0 messages and wondered if it was affected by the direct assignment). Neither of those changes made any difference in terms of the problem. It’s still an immediate correct update when the digitals are in Fixed mode and a deferred update when they’re in VtoC mode.

  if(digital[indexChanged].feedback.colorRangeEnable && !isShifter){
    if (colorIndex) {
      colorR = (colorIndex & 0b01100000) << 1;
      colorG = (colorIndex & 0b00011000) << 3;
      colorB = (colorIndex & 0b00000111) << 5;
    } else {
      colorR = 0;
      colorG = 0;
      colorB = 0;
    } ...

This is on top of noting (above) that moving the fixed-value code into the VtoC code block also doesn’t solve anything.

But what else regarding the digital encoders is affected by whether colorRangeEnable is set? I can’t find anything except the branch when assigning colorR/G/B and yet that doesn’t seem to matter. Maybe I’m doing the test wrong…

gc3 · September 23, 2020, 7:29pm

(prev message removed for clarity)

I figured this out: I still had color_range_enable set on the encoder switches, even though I wasn’t focused on them. When I turn off color_range_enable for the encoders, but leave it on for the digitals, my bit shift code works (and probably the original digitals code–I’ll check).

So it looks as though the problem is in the encoder handling of the VtoC mode, and whatever that problem is holds up all updates, including the digitals updates.

EDIT: Got it. encoderSwitchChanged = true; was not getting set in the VtoC case (note that it’s inside the final else). With this, I get VtoC on encoders AND buttons, and instant updates:

if(colorIndex != encFbData[currentBank][indexChanged].colorIndexPrev || bankUpdate){
        colorIndexChanged = true;
        encFbData[currentBank][indexChanged].colorIndexPrev = colorIndex;
        colorR = pgm_read_byte(&gamma8[pgm_read_byte(&colorRangeTable[colorIndex][R_INDEX])]);
        colorG = pgm_read_byte(&gamma8[pgm_read_byte(&colorRangeTable[colorIndex][G_INDEX])]);
        colorB = pgm_read_byte(&gamma8[pgm_read_byte(&colorRangeTable[colorIndex][B_INDEX])]);
      }
      encoderSwitchChanged = true; // NEW; SEEMS TO FIX BUG
    }else{   // No color range, no special function, might be normal encoder switch or shifter button

EDIT 2: It looks like this was the only problem. I put the original &gamma...&colorRangeTable lookup back into the digitals routine, and it still works perfectly. I built a color selector where pushing a grid button toggles ALL the grid buttons on/off, using the color associated with the button number, and it’s great–this thing is SNAPPY on update! At least on my size of controller I can barely see the sweep (and I suspect the performance can get tuned even higher after the current round of bugfixes).

Extremely excited about the possibilities here!

francoytx · September 23, 2020, 7:48pm

Amazing @gc3!

I am really happy you got it!
I was sure my code was a total mess and I could only understand it. It can still be a mess, but people can read it and fix it, that is huge for me. Thanks for that!

This part is a mess, I know, too many cases to consider, I left this flag off.

So when encoder switch messages were at the end of @mike’s code, it wouldn’t update, but when he commented those messages, it worked. Makes sense now

Just one thingy, the line that sets encoderSwitchChanged to true, should actually go inside the if right above, like this:

if(colorIndex != encFbData[currentBank][indexChanged].colorIndexPrev || bankUpdate){
        colorIndexChanged = true;
        encFbData[currentBank][indexChanged].colorIndexPrev = colorIndex;
        colorR = pgm_read_byte(&gamma8[pgm_read_byte(&colorRangeTable[colorIndex][R_INDEX])]);
        colorG = pgm_read_byte(&gamma8[pgm_read_byte(&colorRangeTable[colorIndex][G_INDEX])]);
        colorB = pgm_read_byte(&gamma8[pgm_read_byte(&colorRangeTable[colorIndex][B_INDEX])]);
        encoderSwitchChanged = true; // NEW; SEEMS TO FIX BUG
      }
      
    }else{   // No color range, no special function, might be normal encoder switch or shifter button

The idea is that it updates only if the color changed.

I am still not able to make it fail here, so whenever you have a moment, if you can try this small change, I’d appreciate it

gc3 · September 23, 2020, 8:15pm

Your code is great, and very readable, especially for a beta! First it needs to work, then refactoring

I think your snippet was the same as mine? But I think you mean:

if(colorIndex != encFbData[currentBank][indexChanged].colorIndexPrev || bankUpdate){
        colorIndexChanged = true;
        encFbData[currentBank][indexChanged].colorIndexPrev = colorIndex;
        colorR = pgm_read_byte(&gamma8[pgm_read_byte(&colorRangeTable[colorIndex][R_INDEX])]);
        colorG = pgm_read_byte(&gamma8[pgm_read_byte(&colorRangeTable[colorIndex][G_INDEX])]);
        colorB = pgm_read_byte(&gamma8[pgm_read_byte(&colorRangeTable[colorIndex][B_INDEX])]);
      encoderSwitchChanged = true; // MOVED UP
      }

    }else{   // No color range, no special function, might be normal encoder switch or shifter button

right? I believe that I tried it that way first, and it didn’t work, but let me try again now.

EDIT: Yep, I remembered correctly; if it’s set inside the if the problem is exactly the same.

And I have a lead as to why (just from reading, haven’t tested it):

I notice the problem when I update the whole button set (digitals + encoder switches) with 0s, which means that the colorIndex is the same as the colorIndexPrev on the encoder switches (for a freshly rebooted board).

I’m also updating the encoders last (arbitrarily, just b/c the button numbers are higher).

So, if neither colorIndexChanged nor encoderSwitchChanged is set for VtoC mode (because both are inside that if), since we’re not doing banks, 2CC or VUMETER, the final if in the function isn’t entered, so:

encRingStatePrev isn’t being updated;
the pre-encoded serial buffer isn’t being updated;
feedbackDataToSend isn’t set to true

My guess is that when a message flood of 0s hits encoders that are already 0 and set to VtoC mode (which isn’t actually that uncommon for the sort of on-host app that @mike and I are both writing), one or more of those things needs to happen for the previous updates to be shown immediately. Otherwise, they’re held until an update happens after a pause. (Nothing special about 0, I suspect; this could probably be replicated with encoders being set to their existing color, no matter what it was).

I don’t know exactly what the problem is, but I suspect that fixing it may stamp out a few more bugs

This is fun!

francoytx · September 23, 2020, 8:03pm

Yes!
Sorry! I edited it now.

mmmm should still work, since to change the color it still has to go inside that if statement.

If it doesn’t, then you wouldn’t notice an update in color, I’d think.

Really hoping you can upload a pic/video of this!

mike · September 23, 2020, 8:28pm

I haven’t had a chance to test out everything y’all are saying, but this is great investigative work!