thomasfredericks / MicroOsc

MicroOsc is a minimal Open Sound Control (OSC) library for Arduino
MIT License
16 stars 2 forks source link

OSCmessage list ordered as [float, string] causes hangup or skipping string values, while [string, float] works ok #6

Closed Lytrix closed 5 months ago

Lytrix commented 5 months ago

Current behaviour If you send an OSCmessage list as a float and then a string item it causes a crash when using a char array or when using a string array the display is showing empty string values very erratically when using the magnetic encoder, while the string item is not empty.

Current setup where it occured I use an SSL EQ VST in Max which sends out a pair of [float, string] values to use on an ssd1306 display to display the slider length of 0-1 and the actual value, like 0.1234 "12.0". This order of float, string causes issues on the Arduino end. The string value sometimes does not show up, creating an empty variable, while the float slider always remains in sync. The weird thing also is, it was only occurring on my first row of 3 of these pairs where I loop over. And also on all 4 displays I am using.

When using a char array with strcpy to store the nextAsString values, my displays are turned off after 5 seconds, possibly a memory overflow?

Example of the code

String displayTxtKnob[4][3][3];
float sliderValue[4][3];

// osAddressParser generates this address:
char oscAddress = "/c/1/p/1/slider";

// OSC MESSAGE LISTENER
void myOnOscMessageReceived(MicroOscMessage& oscMessage) {

  // Loop over displayTxt and as5600List for each parameter osc address and as5600 instance.
  for (int c = 0; c <= 3; c++)  { // Loop over each of the 4 /c controllers
    for (int p = 2; p >= 0; p--)  { // Loop over each of 3 /p oscAddresses
      //oscAddressParser(oscParam[0], c, oscParam[1], p, "/slider");
      if (oscMessage.checkOscAddress(oscAddress, "fs")) {
        sliderValue[c][p]=oscMessage.nextAsFloat();
        displayTxtKnob[c][p][1] = oscMessage.nextAsString();

        //strcpy(displayTxtKnob[c][p][1], oscMessage.nextAsString());
        updateDisplay(displayList[c][0], c, tcaDisplayAddress[c]); 
      }
   }
}

Possible causes It might be due to a wrong rounding off of the floats using my magnetic encoder and the scale function to scale 0-4096 to the VST value between 0-1.

It might the reason the OSC o.compose object creates a double as an OSC type instead of a float (while it states float weirdly), maybe to also fix this problem?

My 12 bit magnetic encoder, creates mostly 6 decimal floats but sometimes 5. As a pattern it seems to mostly goes wrong then.

Possible way to reproduce Below are examples when using an String array (which I though was the solution to the issue) to store the display values instead of a char array, then the values disappeared on my display very erratically:

/c/1/p/1/slider 0.50293 "0.1" (not showing the string value)
/c/1/p/1/slider 0.502441 "0.1" (does show the string value)

/c/1/p/1/slider 0.59668 "3.9"  (not showing the string value)
/c/1/p/1/slider 0.596436 "3.9" (does show the string value)

first test, but might be the wrong values :

/c/1/p/1/slider 0.624023 "5.0" (not showing the string value)
/c/1/p/1/slider 0.65918 "6.4" (not showing the string value)
/c/1/p/1/slider 0.663086 "6.5" (not showing the string value)
/c/1/p/1/slider 0.745117 "9.8" (not showing the string value)
/c/1/p/1/slider 0.725586 "9.0" (not showing the string value)

There are no issues when a minus sign is present:

/c/1/p/1/slider 0.214111 "-11.4" (does show the string value)

When using the mouse to drive the OSC message it also rounds them off, but then it always displays the value properly:

/c/1/p/1/slider "5.6" 0.64 (does show the string value)
/c/1/p/1/slider "5.7" 0.645 (does show the string value)
/c/1/p/1/slider "5.8" 0.6425 (does show the string value)
/c/1/p/1/slider "5.9" 0.6475 (does show the string value)
/c/1/p/1/slider "5.9" 0.648438 (does show the string value)
/c/1/p/1/slider "6.0" 0.65 (does show the string value)
/c/1/p/1/slider "8.1" 0.705 (does show the string value)
/c/1/p/1/slider "8.2" 0.7025 (does show the string value)

Current solution Luckily there is no issue when reversing the order of the list to [string, float]. Then I can also use char again as an array with strcpy. So I have done that for now and it is working really snappy and quick now! Let me know if I can gather more information if needed.

thomasfredericks commented 5 months ago

I think I know what the problem could be.

MicroOsc is optimized for speed and memory use. One of the trade-offs, is that the internal buffer is reused instead of being copied. That means that when you use displayTxtKnob[c][p][1] = oscMessage.nextAsString(); you are storing a pointer to the internal buffer. The content of this buffer can change on the next loop and could result in a non nullterminated string.

To solve this, you need to copy the string returned by nextAsString(). Maybe a solution would be that MicroOsc provide a method for copying the string.

Lytrix commented 5 months ago

Cool, thanks for the update. It does seem to occur more if I turn my encoder really fast so increasing the number of messages exponentially. So the buffer could just be emptied before it is filled set again to read.

I was using strcpy to store the nextAsString(), but then the teensy would crash after 5 seconds of rotating the encoders when using the float, string.

Another solution I tested was to skip storing the string osc message if the nextAsString was less than a length of 2, but it still crashed.

thomasfredericks commented 5 months ago

Hum, MicroOsc only has an input buffer no output buffer. For output, it writes the data directly to the UDP or Serial class. So maybe it is the UDP or Serial that is crashing...

Lytrix commented 5 months ago

It was not only happening when a lot of data was being send. When I gathered examples I moved the encoder slower around a spot to grab the value from Max when the value wasn't displaying and then it also occurred. So it is not happening only when there is a lot of data being send also when little data is send.

thomasfredericks commented 5 months ago

Hi, I made the following code and I cannot make it crash. Could you test it out?

Thanks!

Lytrix commented 5 months ago

Thanks for the updated code! I tested it on my code using [float, string] but still the same behaviour unfortunately:

  1. The displays crashes after 1 second when turning the encoder with using strcpy to store the value as char array (but it still works ok when using [string, float] luckily)
  2. The same behaviour of no values being displayed when using displayTxtKnob[c][p][1] = oscMessage.nextAsString() and storing the value as String.

I also tested your maxpat example and sending that OSC message to the display. That is working perfectly luckily.

So I tried to see if I could see any changes if I printed the string value to the serial.

I did see:

  1. Double/cutoff values, but that might be due to the slower speed of the serial.
    09:28:00.454 -> 0.9
    09:28:00.454 -> 0.0.9
    09:28:00.487 -> 0.9
    ....
    09:28:00.487 -> 0.9
    09:28:00.487 -> 0.90.9
    09:28:00.487 -> 0.9
  2. Sometimes the OSC address gets printed when it happens.
    ...
    09:44:44.462 -> 8.9
    ...
    09:44:44.462 -> 8.9
    09:44:44.462 -> �/c/1/p/2,i���/c/2/p/1,i
    09:44:44.462 -> 7��/c/2/p/2,i��/c/2/p/3,ia��/c/3/p/1,i   W��/c/3/p/2,i@��/c/3/p/3,i��/c/4/p/1,i��/c/4/p/2,i �
    09:44:44.462 -> 8.9
    ...
    09:44:44.462 -> 8.9
    ...
  3. The value of encoder 1 can also disappear when I turned encoder 2.

In relation of observation 2,3: I tried another test by reversing the loop function to see if the error would move to encoder 3 from 1, but the issue still remains on the first encoder value /c/1/p/1 and is not happening on /c/1/p/3.

Let me know if you have any tips on how to further investigate.

This might also be an edge case, so I am also fine with closing the issue for now because using string, float does work fine.

Lytrix commented 5 months ago

Hi, I've got some interesting new info: I am still using one "fs" if statement in my working code which gives a similar issue as above. It sometimes gets the string of the osc address of a previous if statement as string value.

I only use the /c/1/p/1/title address to use as title. But the display sometimes showed the/c/1/p/3/title on the button string which I accidentally still was sending from Max, but was not selecting in the if statements. This occurred when the title osc message is put before the button osc message.

Lytrix commented 5 months ago

Might it be an issue how I constructed the for loops to generate the osc addresses and array positions? This is the basic construct:

4x each controller
  3x potentiometer values
  2x button values
  1x title
thomasfredericks commented 5 months ago

I read quickly through your m4live_ctrl_microosc.ino code and here are some of my comments.

Do not put myMicroOsc.onOscMessageReceived(myOnOscMessageReceived) in a condition. Run it as fast and often as possible in loop() ideally right at the beginning of it. Otherwise the receive buffer might accumulate messages and overflow.

For the size of messages your are sending, MicroOscSlip<128> myMicroOsc(&Serial); seems maybe a bit too small. I would easily increase it to 1024!

You are doing some heavy lifting with the live construction of c strings,oscAddressParser() and the for loops! That is a lot of nested loops going on. I know it feels tedious, but it would be more effective if you wrote out each of the addresses instead of creating them on the fly. I know that OSC was designed to be compatible with long adresses like /c/1/p/1/title and pattern matching but in my experience I find that impracticable and less adapted to micro-controllers.

Here are two different options to greatly increase the speed and reliability :

1) I would suggest you move as much as possible of the address pattern to the arguments. For example, if you want to set the title of potentiometer 2 of controller 1, you could send /title 1 2 "title". The first int could be the controller id and the second the potentiometer id.

2) I created new advanced methods to copy the address and type tags. You can split (i.e. tokenize) the address to then build a conditional strcmp() decision tree from there. Here is an example on the new methods and how to tokenize : microosc_slip_copy-address-and-typetags.ino

Also a possible issue could with the fact that your are using multiple if structures instead of an if...else structure and that could trigger multiple interpretations... but I don't think that is likely as all pattern matching seems to be different.

Lytrix commented 5 months ago

Hi Thomas,

Awesome! Thanks for taking the time to give tips and feedback on my code, much appreciated as a beginner to Arduino/Teensy/MaxMsp, coming from a python/eurorack diy background.

I've removed oscAddressParser() and the for loops and created 4 osc addresses as suggested with typetags from option 1. Wow, that works sooo much better, my display updates are much more stable now. Awesome!

See the updated code here: https://github.com/Lytrix/max4live_controller_w_oled/blob/main/Arduino/m4live_ctrl_microosc/m4live_ctrl_microosc.ino#L279

Option 2 to tokenize and use strcmp() sounds like a good approach if I would like to keep using the /c/1/p/1 logic, but I did not yet have a good idea how to best implement a decision tree, so for now I went for option 1.

I did keep the sending address /c/1/p/1 logic the same for in the Max4live plugin for now and simply grabbed the numbers to use them as typetags. I will decide later if I would also want to change them to typetags, because then I also need to revisit the Max4live UI which I would like to keep more generic, so other future controllers can be setup with it as well.

Lytrix commented 5 months ago

Btw, in regards to the raised issue, I think it was mostly due to my for loops code, so I think you can close this issue on that part.