FujiNetWIFI / fujinet-firmware

8-bit systems to ESP32 WiFi Multifunction Firmware
https://fujinet.online
GNU General Public License v3.0
218 stars 66 forks source link

Improve the UDP stability #745

Closed trekawek closed 1 month ago

trekawek commented 1 month ago

This PR refactors the _tnfs_transaction() function, splitting it into smaller units and fixing bugs in the routines responsible for handling invalid responses and sessions recovery. These bugs, described in the #741 are probably the reason why the UDP protocol doesn't work correctly for some FN users.

Split the _tnfs_transaction()

In the b488003d2625b7ae5d11c867bdf6d1243f9567c8, the large _tnfs_transaction() method is split into smaller units. This commit is a pure refactoring and it shouldn't change any behavior of the function. The bug fixes are built on top of it, in the following commits.

Separate request and response buffer

The commit a037a53b5f09d3b5e291108f83529c5a82493db6 separates request and response buffers in the _tnfs_transaction(). This is required to retry the command - otherwise we wouldn't have the original request that can be send again.

Exhausting recv buffer on invalid response

If the sequence number is not valid, the recv buffer is cleared out in 9d728752ec6fd3643af32b03a644dfad49c6ca46, to be sure that the retried request will receive a clean answer, rather than random data coming from the previous, invalid transaction.

Simulating UDP issues

In order to test such conditions, there was a new flag introduced in 1f0f8e3e0ea93b722e26bc008f7bd3b09a34bc09: TNFS_UDP_SIMULATE_POOR_CONNECTION. If set with:

./build.sh -p ATARI -- -DCMAKE_CXX_FLAGS="-DTNFS_UDP_SIMULATE_POOR_CONNECTION"

or

# platformio.local.ini
[env]
build_flags +=
    -D VERBOSE_TNFS
    -D TNFS_UDP_SIMULATE_POOR_CONNECTION

it'll trigger random UDP connectivity issues:

The probability for these events can be set here: https://github.com/trekawek/fujinet-firmware/blob/9d728752ec6fd3643af32b03a644dfad49c6ca46/lib/TNFSlib/tnfslibMountInfo.h#L37-L40

After applying fixes from this PR, the FN is able to deal with the connectivity issues above and successfully access the TNFS server.

tschak909 commented 1 month ago

This worked out great! Thank you!