Code Monkey home page Code Monkey logo

Comments (19)

sandeepmistry avatar sandeepmistry commented on June 20, 2024

@Axagthoth thank you for bringing this to our attention.

Could you please provide a minimal sketch to reproduce the issue? (ideally without OSC lib etc).

from wifi101.

Axagthoth avatar Axagthoth commented on June 20, 2024

@sandeepmistry thanks for answering so fast! Sure, I can provide an example sketch, but I'm not sure I understand what you mean by "without OSC lib". That's precisely what generates the issue. I can send "normal" UDP packets just fine (just like the WiFiUDPSendReceiveString example does). What should I attach, exactly?

from wifi101.

sandeepmistry avatar sandeepmistry commented on June 20, 2024

It would be nice to isolate the issue from the Oscuino library. So it would be great if you could reproduce just using the WiFi101 library (by using WiFiUDP to send similar data).

Anyways, how about you start with sharing a sketch that others can run to reproduce the issue, even if it includes Oscuino.

from wifi101.

Axagthoth avatar Axagthoth commented on June 20, 2024

Oh, right! Excuse my denseness, it's getting late on this side of the ocean :)

Here's the sketch WITH OSC. In a while I'll share the one without.

#include <SPI.h>
#include <WiFi101.h>
#include <WiFiUdp.h>
#include <OSCMessage.h>

int status = WL_IDLE_STATUS;
char ssid[] = "yourSsid";
char pass[] = "yourWiFiPass";
int locPort = 12000;
int remPort = 10000;
int ledPin = 6;
int ledState = LOW;
WiFiUDP Udp;

void setup() {
  Serial.begin(9600);
  Serial.println("OSC test");

 // check for the presence of the shield:
  if (WiFi.status() == WL_NO_SHIELD) {
    Serial.println("WiFi shield not present");
    // don't continue:
    while (true);
  }

  // attempt to connect to Wifi network:
  while ( status != WL_CONNECTED) {
    Serial.print("Attempting to connect to SSID: ");
    Serial.println(ssid);
    // Connect to WPA/WPA2 network. Change this line if using open or WEP network:
    status = WiFi.begin(ssid, pass);

    // wait 10 seconds for connection:
    delay(5000);
  }
  Serial.println("Connected to wifi");
  printWifiStatus();

  Udp.begin(locPort);
}

void loop() {
  delay(5000);
  OSCMessage msg("/layer1/clip1/connect");
  msg.add((int32_t)1);
  Udp.beginPacket("192.168.1.6", remPort);
  msg.send(Udp);
  Udp.endPacket();
  msg.empty();

}

void printWifiStatus() {
  // print the SSID of the network you're attached to:
  Serial.print("SSID: ");
  Serial.println(WiFi.SSID());

  // print your WiFi shield's IP address:
  IPAddress ip = WiFi.localIP();
  Serial.print("IP Address: ");
  Serial.println(ip);

  // print the received signal strength:
  long rssi = WiFi.RSSI();
  Serial.print("signal strength (RSSI):");
  Serial.print(rssi);
  Serial.println(" dBm");
}

from wifi101.

Axagthoth avatar Axagthoth commented on June 20, 2024

Ok, this should be the same exact message I'm trying to send with OSC, but translated to raw ASCII chars, as per this specification. Maybe you are right, and it isn't Oscuino after all! I've been checking the packets with Wireshark on my side, and I get the same packet as in my first picture, with 17 bytes of data! Even though now they can be clearly counted as being 32 bytes.

#include <SPI.h>
#include <WiFi101.h>
#include <WiFiUdp.h>

int status = WL_IDLE_STATUS;
char ssid[] = "yourSsid";
char pass[] = "yourWiFiPass";
int locPort = 12000;
int remPort = 10000;
int ledPin = 6;
int ledState = LOW;
WiFiUDP Udp;

void setup() {
  Serial.begin(9600);
  Serial.println("OSC test");

 // check for the presence of the shield:
  if (WiFi.status() == WL_NO_SHIELD) {
    Serial.println("WiFi shield not present");
    // don't continue:
    while (true);
  }

  // attempt to connect to Wifi network:
  while ( status != WL_CONNECTED) {
    Serial.print("Attempting to connect to SSID: ");
    Serial.println(ssid);
    // Connect to WPA/WPA2 network. Change this line if using open or WEP network:
    status = WiFi.begin(ssid, pass);

    // wait 10 seconds for connection:
    delay(5000);
  }
  Serial.println("Connected to wifi");
  printWifiStatus();

  Udp.begin(locPort);
}

void loop() {
  delay(5000);
  // /layer1/clip1/connect 1 int
  char rawOSC[] = { 47, 108,  97, 121,
                   101, 114,  49,  47,
                    99, 108, 105, 112,
                    49,  47,  99, 111,
                   110, 110, 101,  99,
                   116,   0,   0,   0,
                    44, 105,   0,   0,
                     0,   0,   0,   1
                   };
  Udp.beginPacket("192.168.1.6", remPort);
  Udp.write(rawOSC);
  Udp.endPacket();  
}

void printWifiStatus() {
  // print the SSID of the network you're attached to:
  Serial.print("SSID: ");
  Serial.println(WiFi.SSID());

  // print your WiFi shield's IP address:
  IPAddress ip = WiFi.localIP();
  Serial.print("IP Address: ");
  Serial.println(ip);

  // print the received signal strength:
  long rssi = WiFi.RSSI();
  Serial.print("signal strength (RSSI):");
  Serial.print(rssi);
  Serial.println(" dBm");
}

from wifi101.

sandeepmistry avatar sandeepmistry commented on June 20, 2024

@Axagthoth nice work!

from wifi101.

Axagthoth avatar Axagthoth commented on June 20, 2024

@sandeepmistry @adrianfreed Ok, looking into the code in WiFiUDP.cpp I decided to change this line in my code:

Udp.write(rawOSC);

for this line (the second definition of the write function, with a pointer and the size):

Udp.write(rawOSC, 32);

And things got a little better. Now the message data doesn't get truncated at the end, thus reaching the ending 28 bytes of correct hexadecimal values, but it somehow gets cut at the head (the first 4 bytes are still somehow considered part of the header and not the data). Maybe it has something to do with the defines in socket_buffer.h? I hope this is helpful and not a bother; like I said earlier I don't know enough about sockets to pinpoint the exact problem.

cap4

from wifi101.

sandeepmistry avatar sandeepmistry commented on June 20, 2024

Hi @Axagthoth,

I think everything is ok, I tried it out locally. Initially I was seeing issues, but then I disabled the LLC protocol decoding. This can be done by:

  1. Right clicking on a packet
  2. Selecting "Protocol Preferences"
  3. "Disable LLC ..."

Now I see this:

screen shot 2016-04-11 at 10 10 38 am

So, Wireshark is just confusing us by auto-enabling LLC decoding.

You should definitely be using Udp.write(rawOSC, 32); instead of Udp.write(rawOSC); because rawOSC is not a null terminated string.

This is why you are seeing: screenshot 3 Udp.write(rawOSC) will loop through the char array until a 0x00 character is found.

I'm going to close this now. Thank you for providing a detailed issue report and also investigating.

from wifi101.

Axagthoth avatar Axagthoth commented on June 20, 2024

I disagree with the issue being marked as resolved, as it is not. It's interesting to have found out that Wireshark wrongly interpreted the headers, but the fact remains that something fishy is going on when trying to encode the messages with OSCMessage::send(Print &p) from the Oscuino library (which was the original issue); and other programs than Wireshark aren't receiving the messages either.
If you compare the bytes in my first post with the ones in my fifth post, you will clearly notice that in the Oscuino case they get truncated, even though they should be the same (independently of how Wireshark distributes them):

Oscuino:
2f 6c 61 79 65 72 31 2f 63 6c 69 70 2f 63 6f 6e 6e 65 63 74 00

rawOSC:
2f 6c 61 79 65 72 31 2f 63 6c 69 70 2f 63 6f 6e 6e 65 63 74 00 00 00 2c 69 00 00 00 00 00 01

That's twenty bytes missing at the tail. I'd really rather being able to leverage the power of the Oscuino library (which has worked before) than having to write the raw OSC messages each time.

from wifi101.

sandeepmistry avatar sandeepmistry commented on June 20, 2024

Please see my comment about the use of write(const char *str) vs write(const uint8_t *buffer, size_t size).

The OSC library needs to use write(const uint8_t *buffer, size_t size) otherwise only the data before a 0x00 byte will be sent. Which lines up with what you are seeing.

from wifi101.

cmaglie avatar cmaglie commented on June 20, 2024

@Axagthoth
It would be nice to have a look at the OSCuino library too, can you provide a link to the library?

from wifi101.

sandeepmistry avatar sandeepmistry commented on June 20, 2024

I'm re-opening this, WiFiUDP is not buffering UDP TX packets.

from wifi101.

Axagthoth avatar Axagthoth commented on June 20, 2024

Yep, sure: Oscuino

I was gonna post the code of the OSCMessage::send(Print &p) function too:

OSCMessage& OSCMessage::send(Print &p){
    //don't send a message with errors
    if (hasError()){
        return *this;
    }
    uint8_t nullChar = '\0';
    //send the address
    int addrLen = strlen(address) + 1;
    //padding amount
    int addrPad = padSize(addrLen);
    //write it to the stream
    p.write((uint8_t *) address, addrLen);
    //add the padding
    while(addrPad--){
        p.write(nullChar);
    }
    //add the comma seperator
    p.write((uint8_t) ',');
    //add the types
#ifdef PAULSSUGGESTION
    // Paul suggested buffering on the stack
    // to improve performance. The problem is this could exhaust the stack
    // for long complex messages
    {
        uint8_t typstr[dataCount];

        for (int i = 0; i < dataCount; i++){
            typstr[i] =  getType(i);
        }
        p.write(typstr,dataCount);
    }
#else
    for (int i = 0; i < dataCount; i++){
        p.write((uint8_t) getType(i));
    }
#endif
    //pad the types
    int typePad = padSize(dataCount + 1); // 1 is for the comma
    if (typePad == 0){
            typePad = 4;  // This is because the type string has to be null terminated
    }
    while(typePad--){
        p.write(nullChar);
    }
    //write the data
    for (int i = 0; i < dataCount; i++){
        OSCData * datum = getOSCData(i);
        if ((datum->type == 's') || (datum->type == 'b')){
            p.write(datum->data.b, datum->bytes);
            int dataPad = padSize(datum->bytes);
            while(dataPad--){
                p.write(nullChar);
            }
        } else if (datum->type == 'd'){
            double d = BigEndian(datum->data.d);
            uint8_t * ptr = (uint8_t *) &d;
            p.write(ptr, 8);
        } else if (datum->type == 't'){
            osctime_t time =  datum->data.time;
            uint32_t d = BigEndian(time.seconds);
            uint8_t * ptr = (uint8_t *)    &d;
            p.write(ptr, 4);
            d = BigEndian(time.fractionofseconds);
            ptr = (uint8_t *)    &d;
            p.write(ptr, 4);

        } else if (datum->type == 'T' || datum->type == 'F')
                    { }
        else { // float or int
            uint32_t i = BigEndian(datum->data.i);
            uint8_t * ptr = (uint8_t *) &i;
            p.write(ptr, datum->bytes);
        }
    }
    return *this;
}

So, as you can see it use both signatures of the write function, but whenever it uses write(const char *str) its because the data is exactly one byte. Maybe printing '\0' to the stream is a problem? Perhaps a better implementation would be writing to a buffer, and the just using one write(const uint8_t *buffer, size_t size) call?

from wifi101.

sandeepmistry avatar sandeepmistry commented on June 20, 2024

@Axagthoth thank for this.

Maybe printing '\0' to the stream is a problem? Perhaps a better implementation would be writing to a buffer, and the just using one write(const uint8_t *buffer, size_t size) call?

This would be more of a work around. WiFiUDP is not correctly buffering writes between beginPacket(addressOrHost, port) and endPacket(), this needs to be fixed. Each write is sent as a separate packet, which is incorrect.

from wifi101.

Axagthoth avatar Axagthoth commented on June 20, 2024

@sandeepmistry Aaah I see. Thanks a lot for all the info! What you say about the buffering certainly sounds coherent with the actual problem. I'm afraid I can't be of further help there, though :( Should I change the title of the issue?

from wifi101.

sandeepmistry avatar sandeepmistry commented on June 20, 2024

@Axagthoth

Should I change the title of the issue?

No it's ok, we can leave it as is.

I've submitted #59 to resolve this problem. We've also contacted Atmel to see if buffer UDP data inside the WINC1500, then we can save RAM on the Arduino side. If this is possible I'll open another PR instead of #59.

from wifi101.

Axagthoth avatar Axagthoth commented on June 20, 2024

@sandeepmistry ok, I've tested #59 and the issue seems resolved! Thanks a lot! Gonna make some further tests with beast-sized packets just to be on the safe side...

from wifi101.

sandeepmistry avatar sandeepmistry commented on June 20, 2024

@Axagthoth excellent, thanks for trying it out!

I've made some notes on the buffer size here: #59 (comment).

from wifi101.

cmaglie avatar cmaglie commented on June 20, 2024

Fixed by #59

from wifi101.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.