Using WIFI-NINA with FtpServer; no accept()

Hello,

This is a first-post from an old dog trying to learn a new trick. I'm very experienced with microprocessors and C, but not so-much with C++ and low-level networking (but I can get by and I'm improving).

My immediate issue is that I am unable to modify @gallegojm FTP server library to work with the WIFI-NINA library for use on my MKR WIFI 1010 unit. I have no issues when I use the assumed Ethernet shield and WinSCP.

I initially found what I thought to be the few changes required to the FTP server to use the WIFI classes. One change - and which I believe is the critical issue - is that the WIFI class does not contain accept() to detect an incoming socket. Rather, the WIFI class contains available() which indicates when a connected socket has data available. There does not appear to be a simple accept().

The issue with WinSCP (and I assume other FTP clients), is that they connect and then wait for the server to "Welcome" them before providing the LOGIN and PASSWORD. FTP server won't send the "Welcome" until it sees the connection, and if WIFI-NINA and available() is used it won't see the connection until >it< receives data from the client. So it's a classic "Deadly Embrace"; no one talks and everyone is listening for the other guy to speak first. :frowning: Consequently the connection hangs and times-out.

So ...:

  • Is there a fundamental reason why WIFI-NINA couldn't report that a socket has been connected to rather than connected to and with data? Clearly this is where my knowledge of networking is limited. If this is a possibility I'm prepared to learn how to rebuild WIFI-NINA and make the enhancement.
  • As an alternative, I can make use of HTTP, but at the moment I'm intrigued why this FTP server can't make use of WIFI-NINA.
  • That being said, are there perhaps other fundamental reasons why WIFI-NINA can't be used by FTP server to facilitate FTP transfers?

Thanks for any and all assistance as I further my knowledge.

FYI, I have searched the forum and the WEB for related info, and I did find snippets, but nothing definitive. There was one reference to "WIFI-NINA can't support FTP" but the reasoning wasn't called-out.

Dale

it is a problem of the library.

Thank you Juraj.

Are you able to elaborate?

  • Is it a bug in the library?
  • Is it a limitation of the library?
  • Is it a limitation of the NINA device?

Basically, where is the problem?

Dale

it is a flaw in 'design' of the Arduino networking API. in Ethernet and in my networking libraries accept() fixed the problem (3 years ago).
for esp8266 and esp32 WiFi library available() does what accept() does (which can be confusing).
WiFiNINA and WiFi101 still can't do accept()

Hello,
Sorry, I have no experience with WifiNina and my ftp server libraries.
May be you can look at the difference in the code when using Esp8266
instead of Wiz5500 ethernet module. (ESP8266 defined in FtpServer.h)
In that case, ftpServer.hasClient() and ftpServer.available() are used
instead of client.connected() and ftpServer.accept()
See lines starting with #ifdef ESP8266 in ftpServer.cpp
Sorry for my bad english
Good luck
Jean-Michel (alias gallegojm on arduino forum)

Thank you Juraj and Jean-Michel,

I will take your answers to mean "Well, it should be able to work (or made to work)." :slightly_smiling_face:

I'll review the esp libraries and see how they implement accept().

I think I'm going to have to better my knowledge of C++; a good problem to have. :wink:

Dale

For reference, here's a snippet of my modifications to FtpServer::service(). It's a hybrid of the conditional codes for the ESP and the W5500 and makes use of the methods from the WIFI-NINA classes. The comments are notes to myself. This code does NOT work as expected. Note that I can get the FtpServer/WIFI-NINA to be somewhat "alive" if I trick the FtpServer to respond to an initial character from a PuTTY session connected to port 21.

		  #ifdef ESP8266
		  if( ftpServer.hasClient())
		  {
		    client.stop();
		    client = ftpServer.available();
		  }
		  #elif defined(WIFININA) // DDW
		  if( client && ! client.connected())
			client.stop();
		  client = ftpServer.available(); // This only seems to return a client if one is present AND there's data available
		  // This is odd. I think we want to have the server create a socket to which a client can connect. But then I think
		  // we may want to check the state of the >client< to see if it's ESTABLISHED (client.status()). Look at the Ethernet
		  // server.accept().
		  // Nope, I think we want to check the state of the server socket.
		  // But I think the WiFi server class only maintains the socket # if the client is sending data. So in order to ask
		  // for the state of the server socket it may be required to maintain a copy of the socket just like they do in the
		  // Ethernet library.
		  //if (client) Serial.println(client.status()); // >>> I only get a client once I press return (and the status of ESTABLISHED is return).
		  //Serial.println(ftpServer.status()); // The server ever only prints 1/LISTENING
		  // Can I determine the state of a client socket before it's ESTABLISHED?
		  #else
		  if( client && ! client.connected())
			client.stop();
		  client = ftpServer.accept();
		  #endif

Hello,

OK, so I now seem to have a working FTP server over the MKR WIFI NINA; my sessions using WINSCP seem to work as expected. But I'm still testing.

I had to make changes to the ESP32 (aka NINA), the WIFI NINA library, as well as the FtpServer library.

Now ...

  1. Should I make my solution available to others?
  2. If so, how do I go about doing that?
  3. Is there a peer review process?

Clearly I've never contributed to software in the public domain.

Thanks.

Dale

can I see the changes?

Juraj,

I'm happy to share them with you. What's the best way to do so? Only a small number of files in the original ESP32/WIFI NINA/FtpServer distributions were changed. Should I just send the changed files? And how do I best do that? Does the forum let me attach a zip'ed archive?

try to attach files or a zip to forum post

Juraj, here's a zip of my changes. I believe you're very experienced with the ESP32 NINA and MKR WiFiNINA libraries, so I'll leave it to you to figure out where the various files in the zip need to be placed. I made changes to the the FtpServer libraries as well. I provided a small Changes.txt file to document the changes and the issues I experienced and the reasoning behind the changes. Just search for "DDW"; those are my initials and all changes are marked with "DDW".

Get back to me to discuss the changes.

Thanks.

Dale
Dale's changes.zip (28.0 KB)

Oh, and here's the main sketch which I use for testing. I've been able to transfer files using WinSCP and FileZilla.

FtpServerSdFat2.ino (8.0 KB)

you only half changed available() to work as accept() by removing if (c.available()) in firmware. that is not useful. "half changed" because now WiFiServer always returns only the first Client. all other clients of the server are not handled.

WiFiNINA must have available() as documented for all Arduino WiFi libraries. Properly adding accept() as a new function would be the right way.

But I think that is something Arduino team must do because doing it right would be a larger change. The fw should do accept and available can then be implemented in WiFiNINA library.

Yes, I think I can see the issue; the "solution" of returning the first client is acceptable when all you're trying to do is determine if any client is present, and then set the "available-GPIO" to cause the SAM to poll the NINA. But if you're looking for data from a client as in availDataTcp(), you need to find any client with data and not just the first. So, you're right in that we seem to need two functions within the ESP32; one to accept() and one to "available". That seems very doable (but the result may be non-standard as far as Arduino is concerned). :slight_smile:

And as I dig deeper into understanding this issue, I can see that the "half changed" solutions is acceptable >>> for my needs <<< as the FTP Server which makes use of the WiFi NINA library only supports a single client (technically, it supports two clients, but each is has its own port [21, 55600]). So for this application the limitation of a single client per port isn't an issue.

But I agree that that this limitation should be corrected in the "official" WiFiNINA and the best solution would be to provide an accept() function like that found in the Ethernet library.

Dale

An update: To make my "half-solution" a bit more palatable, I enhance the FTP Server/WIFI NINA libraries/ESP32 code so that the "half-solution" logic within the ESP is conditional upon a parameter which originates from the FTP Server; so the WIFI library/ESP32 should now operate as they did before the change if the parameter is not specified.

2022-01-02T08:00:00Z

And ... always trying to make the code even better ... I took Jaraj's comment a bit further and re-structured my code so that it would prioritize sockets with data, and would only give you the first socket (without data) if you were "OK" with that (by way of an "accept" flag from the FTP server).

WiFiClient WiFiServer::available(uint8_t* status, bool accept) // DDW
{
  int result = lwip_accept(_socket, NULL, 0);

  if (status) {
    *status = (result != -1);
  }

  if (result != -1) {
    // store the connected socket
    for (int i = 0; i < CONFIG_LWIP_MAX_SOCKETS; i++) {
      if (_spawnedSockets[i] == -1) {
        _spawnedSockets[i] = result;
        break;
      }
    }
  }

  result = -1;

  // find an existing socket with data
  for (int i = 0; i < CONFIG_LWIP_MAX_SOCKETS; i++) {
    if (_spawnedSockets[i] != -1) {
      WiFiClient c(_spawnedSockets[i]);

      if (!c.connected()) {
        // socket not connected, clear from book keeping
        _spawnedSockets[i] = -1;
      } else if (c.available()) { // Socket connected and has data
        result = _spawnedSockets[i];
        break;
	  }
    }
  }
  
  // DDW If no connected socket has data, but you're really just interested in if any socket is connected ...
  if ((result == -1) && accept)
  {
	for (int i = 0; i < CONFIG_LWIP_MAX_SOCKETS; i++)
	{
	  if (_spawnedSockets[i] != -1)
	  {
		result = _spawnedSockets[i]; // Return the first connected socket (with no data)
		break;
	  }
	}
  }

  return WiFiClient(result);
}


1 Like
1 Like

@Juraj,

Thank you for doing this. I hope to review and better understand your solution. I am not proficient at Git/GitHub, but I was able to see some of your changes and I can recognize many of them (as I had a similar solution). One change which may not be complete is in server_drv.cpp(); since you've added two bytes to the command for the accept flag. you can remove the padding statements. I don't know if the additional padding will mess up the command. As a guess, the padding may be utilized to keep the memory accesses aligned (to 32 bits for efficiency of DMA operations).

the alignment is an artifact from the old Arduino WiFi library on which code base the WiFiNINA is made. that code was never nice and the reusing makes it a mess

@Juraj I was able to review your proposed changes and they are superior to mine; you clearly understand the allocation of sockets better than me. :slight_smile: Be aware that ServerDrv::availData(uint8_t sock) also sends AVAIL_DATA_TCP_CMD, and so it should pass an accept flag (of zero) as well. But your code likely works as the legacy alignment padding adds zeros to the outgoing command. And yes, the legacy alignment padding is ugly.

in fw accept parameter is read only in if (tcpServers[socket]).
command is initialized to zeros before receiving.

padding: sendCmd sends 3 bytes, then there are 2 params of size 1 sent with their length and for LAST there is one more ending byte. so 8. it is ideal. I can remove the padding.

I think we're in agreement. :slight_smile: My point is that in your code which I have, availServer() sends the accept flag in byte [6], while availData() doesn't send the accept flag, but they both are processed by AVAIL_DATA_TCP_CMD and which will read byte [6]. In fact, AVAIL_DATA_TCP_CMD might interpret LAST_BYTE as accept when sent from availData.