arduino / nina-fw

Firmware for u-blox NINA W102 WiFi/BT module
135 stars 117 forks source link

Replace peek() with available() during connection status check to avoid loosing messages #50

Closed giulcioffi closed 4 years ago

giulcioffi commented 4 years ago

When trying to perform an OTA update via the cloud servers using the nina as OTA storage, the connection randomly breaks down during download. A lower number of bytes are indeed received since some of them go lost during the disconnection-reconnection phases. During an OTA update the binary is sent by chunks of 256 bytes. These chunks are received by the Nina and then forwarded via SPI and handled by the libraries. When the ArduinoIoTCloud library receives a chunk of data, it tries to decrypt it (this is done by recvrec_ack() function in ssl_engine.c ). If the decryption fails, an error is raised and the disconnection state is reached. The reason why the decryption is sometimes failing is not related to data corruption, but happens because an entire chunk of data go lost. This same error systematically happens at each disconnection. Each chunk is identified in its header by an increasing number. Most likely the decryption is failing because it is expecting a certain number but reading the next one. By dumping these headers from the nina-fw, it is possible to notice that the chuck of data does not go lost in the SPI connection, but is already skipped by the Nina itself. This happens because the message reception and the buffer handling on the Nina side is managed from both WiFiClient::read() and WiFiClient::peek() functions. WiFiClient::read() is the one used to actually read the messages. WiFiClient::peek() is, instead, improperly called from getClientStateTcp(). Then, if inside WiFiClient::connected() in the nina-fw we replace peek() with available() it is possible to perform a "safe" check without dropping chucks of messages.

Jauwaad commented 3 years ago

This change causes the following issue https://github.com/arduino/nina-fw/issues/65