Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server sockets accept and close connections if server.available() not called #36

Closed
drp0 opened this issue Feb 25, 2016 · 16 comments
Closed
Labels
type: imperfection Perceived defect in any part of project

Comments

@drp0
Copy link

drp0 commented Feb 25, 2016

I have written a server application which works well, up to a point.
If a html page contains more than 2 pictures, only two are uploaded.
The missing pictures can be individually uploaded.
In IE, they can forced by right click, show picture.
Refreshing the page does not alter the number of pictures uploaded.
I have had this problem before when writing servers on another platform.
The issue proved to be a shortage of sockets.

WiFi101server.txt

Code attached,
David

@sandeepmistry
Copy link
Contributor

@drp0 what board(s) are you testing with?

On the MKR1000, using the SimpleWebServerWiFi.ino example sketch and using nc <IP address> 80 from a Mac. I can only establish 2 connections at a time, on the 2rd attempt Wireshark shows the connection being accepted, immediately followed by a disconnect.

@sandeepmistry
Copy link
Contributor

With the current ASF base, there is essentially a backlog of one pending connection until server.available() is called again. Any incoming connections will be accepted by the firmware, then immediately will be disconnected by the ASF code, see this block of code: https://github.com/arduino-libraries/WiFi101/blob/master/src/socket/source/socket_buffer.c#L216-L219

@ThibaultRichard any thoughts on how this can be improved?

@drp0
Copy link
Author

drp0 commented Mar 3, 2016

Hello Sandeep,

I am working with the Arduino mega 2560.

The wifi101 firmware is 19.4.4.

The library is version 0.8.0

Your experience on the Mac is like mine.

Could this use of a maximum of (up to) 2 sockets, also be the cause of the ftp download failure in #32?

The maximum number of tcp sockets in the library is more than two.

Cheers,

David

From: Sandeep Mistry [mailto:notifications@github.com]
Sent: 02 March 2016 20:58
To: arduino-libraries/WiFi101
Cc: drp0
Subject: Re: [WiFi101] Server uploads two images on a html page, the rest are not processed (#36)

@drp0 https://github.com/drp0 what board(s) are you testing with?

On the MKR1000, using the SimpleWebServerWiFi.ino https://github.com/arduino-libraries/WiFi101/blob/master/examples/SimpleWebServerWiFi/SimpleWebServerWiFi.ino example sketch and using nc 80 from a Mac. I can only establish 2 connections at a time, on the 2rd attempt Wireshark shows the connection being accepted, immediately followed by a disconnect.


Reply to this email directly or view it on GitHub #36 (comment) . https://github.com/notifications/beacon/ANHAGrOdyCNyVIq8hDDBsisFVBzndg5rks5ppfnQgaJpZM4Hi_CH.gif

@sandeepmistry
Copy link
Contributor

@drp0 thanks for the info. The behaviour in this issue is specifically related to TCP server sockets, so unrelated to #32.

@sandeepmistry sandeepmistry changed the title Server uploads two images on a html page, the rest are not processed Server sockets accept and close connections if server.available() not called Mar 4, 2016
@sandeepmistry sandeepmistry added the type: imperfection Perceived defect in any part of project label Mar 4, 2016
@drp0
Copy link
Author

drp0 commented Mar 5, 2016

What is the socket limit on the Amtel chip?
I have a memory of reading somewhere in the arduino library that the maximum number of tcp sockets was 7, although I can-not recall which file contained the information.
I had a quick look at the code snippit [https://github.com/arduino-libraries/WiFi101/blob/master/src/socket/source/socket_buffer.c#L216-L219]
I am surprised it did not do a test of the number of sockets open, against the maximum number of sockets. Could this lead to a workround?

I have just tried commenting out line 218: close(pstrAccept->sock); in socket_buffer.c
The server will then upload all the images on a sample page of 4 pictures.
The arduino software issues a client.stop() correctly.
The library software does not appear to process the instruction and the client browser waits with a receiving data message. (I was not surprised)
If the browser cancel download "X" is selected, the server will supply the next request.
David

@sandeepmistry
Copy link
Contributor

What is the socket limit on the Amtel chip?

Correct it's 7, see socket.h.

I am surprised it did not do a test of the number of sockets open, against the maximum number of sockets. Could this lead to a workaround?

Agreed there should be a check.

Only work around I can think of is to have your sketch call server.avaible() more frequently for now.

@sandeepmistry
Copy link
Contributor

Hi @drp0, if you are still interested in this, could you please try PR #204 when you get a chance. Thanks.

@drp0
Copy link
Author

drp0 commented Dec 5, 2017 via email

@sandeepmistry
Copy link
Contributor

Hi @drp0,

You can:

  1. Backup your <sketchbook>/libraries/WiFi101 folder
  2. Download https://github.com/sandeepmistry/WiFi101/archive/socket-buffer-winc1500.zip
  3. Extract zip
  4. Place zip from library in <sketchbook>/libraries/WiFi101
  5. Restart the IDE and try it out

To restore, delete the <sketchbook>/libraries/WiFi101 folder and replace with the one backed up in step 1).

@drp0
Copy link
Author

drp0 commented Dec 5, 2017 via email

@sandeepmistry
Copy link
Contributor

Hi @drp0,

Thanks for trying and apologies for not being clear on what I wanted you to test.

  1.  I include  <TimeLib.h> as that worked in previous incarnations of yifi101
    

I seem to recall a lot of discussion on your choice of library for time support.

I get the warning “No system <time.h> header included, WiFi.getTime() wil always return 0”

So what have you changed and what are the implications for anything written in previous versions of >YIFI101?

No changes have been made for this.

  1.  The remote IP and port callback routine you gave me no longer works:
    

I've renamed socketBufferCb to socket_cb.

I hope you can maintain an option to get the remote IP for security reasons

It's easier to make have a public API for this once the PR is merged.

  1. I can now get 4 pictures back from yifi101 on one page- obviously a great improvement- further testing tomorrow

I have not yet tried to time the page serving rate- but it appears to be faster.

Ok, that's great news. The changes in the pull request allow the WiFi101 to have more than one accepted socket pending from a server. Previously as the issue is titled after one pending accepted connection, the WiFi101 library would close the socket.

@drp0
Copy link
Author

drp0 commented Dec 6, 2017

Hi Sandeep,
Good news indeed.
If the socket limit is 7, what would happen if a page contained more than 7 servable objects?

I tried altering line 10 above to
socket_cb(sock, u8Msg, pvMsg);
and got
socket_cb was not declared in this scope
Is there a temporary work round?

David

@sandeepmistry
Copy link
Contributor

If the socket limit is 7, what would happen if a page contained more than 7 servable objects?

So one socket will be taken up by WiFiServer, so that leaves 6 left for incoming connections. You'll have to carefully manage WiFiClient's and check if there is pending socket data if you are using an AVR based board.

Is there a temporary work round?

In src/WiFi.cpp there the following line:

static void socket_cb(SOCKET sock, uint8 u8Msg, void *pvMsg)

remove the static so it becomes:

void socket_cb(SOCKET sock, uint8 u8Msg, void *pvMsg)

then in your sketch add void socket_cb(SOCKET sock, uint8 u8Msg, void *pvMsg); to forward declare it.

@drp0
Copy link
Author

drp0 commented Dec 6, 2017

That all works
So far the server appears more stable than before.
Thanks,
David

@sandeepmistry
Copy link
Contributor

Great, thanks for testing and the feedback :)

@sandeepmistry
Copy link
Contributor

Now that PR #204 is merged, this is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants