CNMAT / OSC

OSC: Arduino and Teensy implementation of OSC encoding
cnmat.berkeley.edu/oscuino
Other
734 stars 137 forks source link

OSCBundle.fill with an OSCMessage #83

Open peternewman opened 6 years ago

peternewman commented 6 years ago

It would be great if it was possible to do OSCBundle.fill() like the UDPReceive example and when it's been filled it senses if #bundle is missing and therefore reprocesses it as a message/converts it to a bundle without timestamp. Depending on the capabilities of the sending software, it may or may not send bundles, and it's a shame if the user has to deal with detecting this rather than the library.

I guess this may need to be a function call after fill() e.g. maybeConvertMessageToBundle() or something more user friendly.

oori commented 5 years ago

@smiarx Wasn't https://github.com/CNMAT/OSC/pull/62 supposed to resolve this? But, it doesn't...

I'm facing the same issue now, having an ESP8266 device that receives OSC messages AND bundles from different sources (3rd party apps, I can't control). @peternewman , what's the current trick, how can this library handle both ? Thanks.

peternewman commented 5 years ago

I may have been using an older version @oori , or have you also tested that #62 doesn't fix it? From a quick look, it looks like it should work.

oori commented 5 years ago

Yeap, double checked by validating those two lines were in my local OSCBundle.cpp (OSC v1.3.5)

Using this pattern of use:

void oscHandler(){

  int size = Udp.parsePacket();
  if (size > 0) {
    OSCMessage msg;
    //OSCBundle msg;
    .
    .  (rest of code is same)
    .

When using OSCBundle msg; and a message comes in, I get OSC ERROR 2. When using OSCMessage msg; and a bundle comes in, I get OSC ERROR 0. (obviously...)

How could one detect if it's a bundle or message at that point ? I couldn't see how.

peternewman commented 5 years ago

Ah, I did wonder and that confirms it, that error is being generated here: https://github.com/CNMAT/OSC/blob/master/OSCBundle.cpp#L280-L282

The OSCMessage handling is here: https://github.com/CNMAT/OSC/blob/master/OSCBundle.cpp#L293-L296

But incomingMessageSize is only set here: https://github.com/CNMAT/OSC/blob/master/OSCBundle.cpp#L322

I don't think the message size is known for a normal OSC message, so you probably want a bool variable to flag that it's actually an OSCMessage in the bundle and to not do the size check (or possibly check that the size is zero at that point, having previously errored if the size was zero when it was actually checked if it was an OSCBundle).

oori commented 5 years ago

Thanks for your reply. So, https://github.com/CNMAT/OSC/pull/62 doesn't resolve it, am I the only one in the world who gets multiple uncontrolled inputs into my device ?! I didn't get exactly your suggestion, I'm checking the size of the packet, not the message/bundle.

not do the size check

requires change in the library, if I get you right.

check that the size is zero at that point, having previously errored if the size was zero when it was

actually checked if it was an OSCBundle you mean catch if error == 2 then handle it as oscMessage?

here's the full handler function, I'd appreciate your suggestion for changes:

void oscHandler(){

  int size = Udp.parsePacket();
  if (size > 0) {
    OSCMessage msg;
    // OSCBundle msg;
    while (size--) {
      msg.fill(Udp.read());
    }
    if (!msg.hasError()) {
      msg.dispatch("/someRoute", routeHandler);
      // and more msg.dispatch()
      msg.empty();
    } else {
      error = msg.getError();
      DEBUG_PRINT(error);
    }
  }
}

Thanks!

peternewman commented 5 years ago

From the looks of things it resolves it if you don't check for errors...

I suspect not, the quick test I was doing on it, when it didn't work I realised it was the wrong message type and just switched wholesale.

Sorry, they were both suggestions for fixing the library. I'd suggest that's the best place to fix it, rather than trying to hack around it in your code. Does my suggestion make enough sense in that case for you to attempt a fix?

Saying that if you wanted to hack around, if the error == 2 and it's not a bundle, you could safely ignore the message; I don't believe you'd need to reprocess it, the bundle should have decoded it.

adrianfreed commented 5 years ago

I agree that applications should be ready to accept messages or bundles. It's rather easy to test whether a packet might be a message or bundle (we designed OSC to make this easy). From the spec: "The contents of an OSC packet must be either an OSC Message or an OSC Bundle. The first byte of the packet's contents unambiguously distinguishes between these two alternatives."

It's probably good practice to make this basic test before passing the packet bytes to the fill routine. That way you make your application robust against people sending any old packet . One could argue that this is part of the libraries job and indeed we have to parse the packet suspiciously. The challenge is returning detailed information back to the application as to why the parse failed. I regret we didn't put these defensive tests into the examples. I will review this as I upload some WIFI examples I am working on .

oori commented 5 years ago

Thank you both for your replies. @adrianfreed I'd be happy to get a glimpse at the wifi examples you mentioned, to see your strategy for resolving this clean and safe. are they resolving this issue in the application level if (first8Bytes = "#bundle") then {...} or this check would be incorporated in the library?

Thanks again.

stevefal commented 5 years ago

I'm in a similar situation. I'm trying to integrate with a device (via WiFi) that sends bundles continually, and messages only occasionally. I am only interested in the messages. My current code, based on the ESP8266ReceiveMessage example is swamped with the bundle data, and does not dispatch from the messages as it does successfully when testing from a device that only sends messages. I first need th messages to be carved out, so I can then process them with the dispatch.

So I am likewise trying to switch initially based on the presence of bundles versus messages.

For me as somewhat a novice, I'd prefer if the library further abstracted receipt of bundles and messages, so that a global OSCReceiver object contained arrays of the last N bundles and messages, already parsed, subject to some maximum. I could grab from either, ready-made bundles or messages for processing, without having to worry about parsing the fire-hose of raw data from the stream.

In the meantime, I'll try to figure out how to ignore the bundle data. I see clues above, but if anyone has that bit working, I'd appreciate seeing your code.

stevefal commented 5 years ago

Well, filtering out bundles turned out to be easy. Unfortunately, it turns out that the device I'm listening to is putting the message of interest into a bundle after all. In either case, here's how I ignore bundles - flush the UDP message when it contains hex 35 ("#") in the first byte:

void loop() {
  digitalWrite(ledPin, HIGH);
  OSCMessage msg;
  int size = Udp.parsePacket();

  if (size > 0) {
    if (Udp.peek() == 35) {
      Udp.flush();
    }
    else {
      digitalWrite(ledPin, LOW);
      while (size--) {
        msg.fill(Udp.read());
      }

      if (!msg.hasError()) {
        msg.dispatch("/dev/0/0/mon", cmdVol);
      } else {
        myerror = msg.getError();
      }
    }
  }
}
stevefal commented 5 years ago

Now I'm full circle back trying to process a message within a bundle, and I don't understand how the ESP8266ReceiveBundle sample code is supposed to work in the first place. How can the bundle dispatch an OSCBundle into the led() function that is expecting an OSCMessage as its input?

//from ESP8266ReceiveBundle example

void led(OSCMessage &msg) {
  ledState = msg.getInt(0);
  digitalWrite(BUILTIN_LED, ledState);
  Serial.print("/led: ");
  Serial.println(ledState);
}

void loop() {
  OSCBundle bundle;
  int size = Udp.parsePacket();

  if (size > 0) {
    while (size--) {
      bundle.fill(Udp.read());
    }
    if (!bundle.hasError()) {
      bundle.dispatch("/led", led);
    } else {
      error = bundle.getError();
      Serial.print("error: ");
      Serial.println(error);
    }
  }
}
SebastienPitt commented 5 years ago

@adrianfreed If you could provide a function exemple to add to ESP8266ReceiveMessage.ino or ESP8266ReceiveBundle.ino that could help us distinguish between a bundle or a message, that would be greatly appreciated. Thank you.

Blueloop commented 3 months ago

I fixed it for me like this:

void loop() {
  OSCBundle bundle;
  OSCMessage message;

  int size = Udp.parsePacket();

  if (size > 0) {
    if (Udp.peek() == 35) {
      Serial.println("Bundle:");
      while (size--) {
        bundle.fill(Udp.read());
      }
      if (!bundle.hasError()) {
        bundle.dispatch(ID1, fooX);
        bundle.dispatch(ID2, fooY);
      } else {
        OSCerror = bundle.getError();
        Serial.print("error: ");
        Serial.println(OSCerror);
      }
    } else {
      Serial.println("Message:");
      while (size--) {
        message.fill(Udp.read());
      }
      if (!message.hasError()) {
        Serial.print("Msg Found: ");
        Serial.print(message.getAddress());
        Serial.print(" ");

        message.dispatch(ID1, fooX);
        message.dispatch(ID2, fooY);
      } else {
        OSCerror = bundle.getError();
        Serial.print("error: ");
        Serial.println(OSCerror);
      }
    }
  }
}
EchterAgo commented 3 weeks ago

Try #150