UDP sending quirks

[drtutut]

Here is a description of a problem with UDP I managed to solve.

Board : STM32 Discovery B-L475E-IOT01A
ISM43362-M3G-L44 Firmware version: C3.5.2.3.BETA9

I am currently testing the sending of OSC (Open Sound Control) messages over UDP datagrams.
I use the LiteOSCParser library to build the buffer containing the message, and then send it via a WiFiUDP object and its write method.

My setup() function initialises the WiFi connection and everything works like charm.

My loop() function is as follows :

  static int v = 0;
  Osc.init("/testosc");
  Osc.addInt((int32_t)v++);
  v = v % 1024;
  
  if (Udp.beginPacket(sendIP, sendPort) == 1)
    {
      size_t wr = Udp.write(Osc.getMessageBuf(), Osc.getMessageSize());
      Udp.endPacket(); 
    }
  Osc.clear();

  delay(20);

(I'm concious that endPacket() method does nothing for the moment but wait...

When running this code, the first datagram is sent, and received by the application which listens sendIP and sendPort but then, no subsequent UDP datagrams are sent.

Some tracing shows that the first loop execution effectively writes 20 bytes (my message size), but the susequent writes just write zero bytes.

Then I realized that the sending process developped in the driver is based on a timeout design : if the data takes to much time to be sent, the write operation (the IsmDrvClass::ES_WIFI_SendResp method indeed) is considered as faulty.

beginPacked starts a client connection (by calling ES_WIFI_StartClientConnection(_sock)), but this connection is never closed, and I suspect that after the first write it is no n longer valid (because of the timeout design).

So I modified the endPacket() method, and added a DrvWiFi->ES_WIFI_StopClientConnection(_sock); call into it, and voila ! It works now and my messages are written and received on the other side.

But I have an aside question : despite a short 20ms delay at the end of the loop, the overall loop is very slow and, after a tracking of the problem, I managed to find that the beginPacket() call takes approximately 246ms, the sending of the datagram (`write) takes 160ms and closing the connection client takes 80ms. If somebody has an idea to shrink down these times, this would be great, because my future application needs lower transmission times in order to be useful (sending sensor data to control sound devices parameters).

Regards

Eric

[drtutut]

After digging more deeply in the driver code, questions arise:

• After the first write, as this method returns the size, it means that the interface returned an AT "OK" string (see code of ES_WIFI_SendResp), so the interface should be ready to receive further S3 and S0 AT commands.
• I think there is a semantics problem with beginPacket. So far, this method opens a client connection, which means that each packet needs a new client connection. Isn't this too heavy ? (and incidentally induce my hack in the endPacket method)

[drtutut]

To continue previous comment : if I make a single beginPacket call in my setup() function and then only perform write calls in the loop (and never call endPacket) it works very well, and the transmission of each packet takes 160ms, which remains quite a lot but far better than the preceding solution (and incidently I do not see any mean to reduce this time, it seems it is essentially the response time of the ISM43362-M3G-L44 to AT commands).

But once again, it breaks the semantics of the UDP abstract class as initially provided.

• beginPacket() should "declare" the begining of a new packet.
• write() should concatenate new data to a buffer, but not actually sending the data
• endPacket() should send the data accumulated so far.

But beginPacket() should not initiate a new connection client. The fact is that the original UDP abstract class provides no mean to "initiate a new connection client". This client mode switching seems to be very specific to the ISM43362-M3G-L44.

A possible solution would be to implement startClient() method in WiFiUPD class, which starts a client connection (by calling DrvWiFi->ES_WIFI_SetConnectionParam(_sock, ES_WIFI_UDP_CONNECTION, port, ip); and DrvWiFi->ES_WIFI_StartClientConnection(_sock); ), and a endClient() method which stops the cleint connection (by calling DrvWiFi->ES_WIFI_StopClientConnection(_sock);).

Then, rewrite beginPacket which should only initialize a new sending buffer. Rewrite write so it concatenates new data to the sending buffer (and manage possible reallocation). Rewrite endPacket() so it really sends the data in the sending buffer.

The main problem with this solution is that it breaks the UDP class interface by introducing two new methods.

[fpistm]

Hi @drtutut
thanks for this detailed report.
I've no specific answer yet as it needs some investigation before answer you and I will not be able to do this until august at least.

As a first feedback, the High level API should follow the Arduino one :
For example: https://www.arduino.cc/en/Reference/WiFiUDPBeginPacket
If this is not the case then I guess there is a first issue here.

About the response time of the ISM43362 then yes I've already observed that.

Do not hesitate to provide a PR with your proposal to fix/enhance this. This is a community project so all contribution are welcome.

[drtutut]

Thank you for your answer @fpistm

I agree that the API has to follow the WifiUDP class you mention. I would be glad to contribute to the enhancement of this library, but I think it need further discussion before devising a solution.

The problem for me is that its specification is very unclear, as is suggests that :

• beginPacket starts a connexion to an IP:port
• write writes the data to the remote connection
• endPacket : Called after writing UDP data to the remote connection. It finishes off the packet and send it.

So first it suggests that one have to use this call sequence for every UDP datagram sent. At least this is my interpretation, as the words used in the specs of the API are particularly vague.

Another problem with this API is that, while stating that beginPacket starts a connexion, it does not say that endPacket ends it.

In order to stick to the API, perhaps the way to go is

• beginPacket: if a client UDP connection to the IP:port is not already opened, open it (and potentially close a pending connection on another IP:port, which mean that this info has to be tracked), otherwise go on. Initialize an empty buffer.
• write: add data to the buffer, reallocating it if necessary
• endPacket : send the data in the buffer using ES_WIFI_SendResp
• destructor of WiFiUDP checks : if a client connection is opened, close it.

I fork the project and try to develop on this proposal basis. No guarantee to finish it before holidays.

[fpistm]

Agree that the API is not very precise and not mainly consistent Begin vs end.

No worry for holidays. Have a nice one.

[drtutut]

Ok, here a first proposal. I paste a link pointing on my fork, I think it's too early to issue a pull request:

https://github.com/drtutut/WiFi-ISM43362-M3G-L44

The beginPacket, write and endPacket methods conforms to the proposal in my previous message.

With the current release, I'm able to execute sequences of beginPacket, multiple write calls to build up the datagram, and endPacket to send the datagram. Only the first call to beginPacket opens the client connection (thus saving time on subsequent calls).

The main evolutions are located in in ISM43362_M3G_L44_driver.(h|cpp):

• modification of the ES_WIFI_Conn_t structure:
  • added a buffer of ES_WIFI_PAYLOAD_SIZE bytes for datagram construction, which grows the RAM footprint of the library of 4800 bytes. This is the most questionable part of the evolution, but I thought that managing dynamic buffers added a useless code complexity (and potential memory leaks risks, not a good idea for a driver).
  • added a size attribute to keep track of the current filling of the buffer
  • added a flag to keep track of the UDP client status.
• IsmDrvClass constructor initializes this felds for each socket.
• ES_WIFI_StartClientConnection method initializes the buffer and updates the start flag . So does ES_WIFI_StopClientConnection.
• new method ES_WIFI_SendBuf sends the content of the buffer associated with an UDP socket (it just calls ES_WIFI_SendResp with the appropriate parameters, and then resets the buffer).
• ES_WIFI_SetConnectionParam resets the buffer
• new method addToUDPBuffer adds data to the UDP buffer (with size checks).
• new method isClientStarted returns the state of the client UDP connection.

Then WiFiUdpST.(h|cpp) reimplements beginPacket, write and endPacket to conform more closely to the expected behaviour.

You are welcome to perform additional tests.

[fpistm]

We will check that after holidays, moreover we had input from Inventek about USB socket management.
It seems we could enhance this. 😉

[fpistm]

I've got time to check this issue,
In fact calling beginPacket() a second time while the client connection is not closed return an error:

UDP  RC] Socket opened on port 3001
UDP  RC] unable to create socket for UDP
UDP  RC] Error connecting
RROR: Unknown Error
sage: P6 <0 = Stop, 1 = Start> 

This cause the firmware to never send data.

I've made a new PR #16 which prevent this use case.
I do not use your code because it made a lot changes and using a buffer consume 4 * 1400 bytes which is huge.
Moreover closing the connection each loop increase the time which is not what we want.

Here the code which works with th PR #16

  if (Udp.beginPacket(sendIP, 3001) == 1) {
    size_t wr = Udp.write(Osc.getMessageBuf(), Osc.getMessageSize());
    Serial.println(wr);
    Udp.endPacket();
  }

Note that it should answer your issue with timing as beginPacket() do nothing if the connection is already started.
You can also use endPacket(true)\ if (wr === 0)`, this will reset the connection and ten will be properly restarted.