MAX_SOCK_NUM is faulty determined in combination with Mega board

[BertHav]

It seems that the number of sockets is determined in ethernet.h based on RAMSTART and RAMEND. But these facts are both defined for the mega chip instead of the W5100.
They are both defined in arduino15/packages/arduino/tools/avr-gcc/4.9.2-atmel3.5.4-arduino2/avr/include/avr/iom2560.h Due to the fact that the Arduino Mega has more than 2048 bytes RAM the W5100 is faulty defined with 8 sockets.
Arduino IDE 1.8.7 Ethernet library 2.0.0

[Rotzbua]

I agree that this misbehavior should be documented. I already mentioned it at #68 (comment) .

The reason is the chip type detection while run time and this do not work with variables while compile time ...

[SapientHetero]

Similarly, RAMSTART and RAMEND are not defined at all for the Adafruit Metro M4, which causes the library to allocate only 4 sockets for the W5500 instead of 8.

[PaulStoffregen]

Are you sure about that? The code is:

#if defined(RAMEND) && defined(RAMSTART) && ((RAMEND - RAMSTART) <= 2048)
#define MAX_SOCK_NUM 4
#else
#define MAX_SOCK_NUM 8
#endif

On SAMD or any board where either of these aren't defined, the first 2 checks should be false and it should end up as 8. All 3 have to be true for only 4.

[SapientHetero]

I am. Segger debugger showed sockindex with 4 array elements in EthernetClient. When I commented out the compiler directives and just set MAX_SOCK_NUM to 8, it showed 8 elements. I found this while chasing a "Connection refused" issue with my web server. After fixing this and adding "Connection: keep-alive" to response headers I thought I'd fixed it but it still happens occasionally. Any suggestions for where I should look next? Where in the library are connection requests refused?

…

On Tue, Oct 2, 2018, 2:29 PM Paul Stoffregen ***@***.***> wrote: Are you sure about that? The code is: #if defined(RAMEND) && defined(RAMSTART) && ((RAMEND - RAMSTART) <= 2048) #define MAX_SOCK_NUM 4 #else #define MAX_SOCK_NUM 8 #endif On SAMD or any board where either of these aren't defined, the first 2 checks should be false and it should end up as 8. All 3 have to be true for only 4. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#70 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AlmIIwwL2GSngTREwhaPpoEmGXVktLQFks5ug7B-gaJpZM4XCyxS> .

[PaulStoffregen]

Regarding this:

Due to the fact that the Arduino Mega has more than 2048 bytes RAM the W5100 is faulty defined with 8 sockets.

The difficult design problem is Ethernet really does now support 8 sockets on Arduino Mega, if you connect a shield with the W5200 or W5500 chips.

[SapientHetero]

I'm running an Adafruit Metro M4. I posted here hoping to get this resolved for others who may lack the tools to find it. I moved to the M4 after Mega RAM became the limiting factor for my application, so I appreciate the need for a solution. Do you think increasing buffer size for my M4 would improve performance? I've got RAM to burn.

…

On Tue, Oct 2, 2018, 2:42 PM Paul Stoffregen ***@***.***> wrote: Regarding this: Due to the fact that the Arduino Mega has more than 2048 bytes RAM the W5100 is faulty defined with 8 sockets. The difficult design problem is Ethernet really does now support 8 sockets on Arduino Mega, if you connect a shield with the W5200 or W5500 chips. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#70 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AlmII3V8XSL6N0Z6IOduXho_GOsJeu_Hks5ug7OcgaJpZM4XCyxS> .

[PaulStoffregen]

On this issue, please keep the conversation to MAX_SOCK_NUM.

For help with designing your application, post on the forums. For an Adafruit product, post on their forum first. This github issue tracker is for reporting defects in the Ethernet library. If you've found another defect, and you're sure it's an Ethernet library bug, start a new issue.

Edit: if you really have found a server socket connection bug, please invest some time to create a complete test case before you open another issue. I will not investigate if the issue has only code fragments or a vague description lacking specific details of how to reproduce the problem.

[BertHav]

Due to the fact that the Arduino Mega has more than 2048 bytes RAM the W5100 is faulty defined with 8 sockets.

The difficult design problem is Ethernet really does now support 8 sockets on Arduino Mega, if you connect a shield with the W5200 or W5500 chips.

Yes indeed, but the number of sockets does not depend on the processor type. So to my opinion this is a nice try, but a bogus definition. I also agree MAX_SOCK_NUM is widely spread, so removing is not an option. My proposal would be to let it static defined, as it was. The developer who applies an 8 socket chip understands how she/he can define 8 sockets. The socket definition in the current implementation method is weird and produces unpredictable results/instability.

[PaulStoffregen]

My proposal would be to let it static defined, as it was.

Could you be more specific?

Are you suggesting to forever make only half the sockets of the current generation chips available on all boards, without going into edit the library source?

[SapientHetero]

I agree. The existing code determines the number of hardware sockets available based on chip type. Let that be the default behavior. Keep #define MAX_SOCK_NUM as a user-settable manual limit but eliminate the compiler directive logic surrounding it. It makes sense to limit RAM consumption based on processor type, but I like the idea of an easy-to-use override. After a processor-based RAM use limit calculation, add a commented-out #define that lets the user override it if their application needs a different value.

…

On Wed, Oct 3, 2018, 4:03 PM BertHav ***@***.***> wrote: Due to the fact that the Arduino Mega has more than 2048 bytes RAM the W5100 is faulty defined with 8 sockets. The difficult design problem is Ethernet really does now support 8 sockets on Arduino Mega, if you connect a shield with the W5200 or W5500 chips. Yes indeed, but the number of sockets does not depend on the processor type. So to my opinion this is a nice try, but a bogus definition. I also agree MAX_SOCK_NUM is widely spread, so removing is not an option. My proposal would be to let it static defined, as it was. The developer who applies an 8 socket chip understands how she/he can define 8 sockets. The socket definition in the current implementation method is weird and produces unpredictable results/instability. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#70 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AlmII5dN0QLVQKTrGc8m36cPB3nRbUj2ks5uhRgugaJpZM4XCyxS> .

[PaulStoffregen]

I still have no idea what you're specifically suggesting.

[BertHav]

Are you suggesting to forever make only half the sockets of the current generation chips available on all boards, without going into edit the library source?

Yes, for instance for the next 3 years. It is very easy to (re)define to 8 sockets. Defaulting to 4 breaks nothing, defaulting to 8 produces errors and bogus information in a 4 socket chip.

[SapientHetero]

I disagree with the suggestion of hard coding a max of 4 sockets. If someone buys a W5500-based board specifically to get the advertised 8 sockets, why shouldn't the library automatically support them if it can? The existing library already detects the chip type and correctly sets max_socks for the chip detected. The only thing preventing the library from getting it right for W5100, W5200 & W5500 is the precompiler logic that limits sockets based on RAMSTART & RAMEND. Remove the RAMSTART logic & let the existing logic determine max_socks unless overridden manually by a user defining a lower value by setting MAX_SOCK_NUM.

…

On Thu, Oct 4, 2018, 1:14 PM BertHav ***@***.***> wrote: Are you suggesting to forever make only half the sockets of the current generation chips available on all boards, without going into edit the library source? Yes, for instance for the next 3 years. It is very easy to (re)define to 8 sockets. Defaulting to 4 breaks nothing, defaulting to 8 produces errors and bogus information in a 4 socket chip. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#70 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AlmII2dM7iBFiQYwukeMzpv-MVK1swVgks5uhkHdgaJpZM4XCyxS> .

[SapientHetero]

Are you sure about that? The code is:

#if defined(RAMEND) && defined(RAMSTART) && ((RAMEND - RAMSTART) <= 2048)
#define MAX_SOCK_NUM 4
#else
#define MAX_SOCK_NUM 8
#endif

On SAMD or any board where either of these aren't defined, the first 2 checks should be false and it should end up as 8. All 3 have to be true for only 4.

Just realized why this logic doesn't work.

"Conditionals written like this: #if defined BUFSIZE && BUFSIZE >= 1024 can generally be simplified to just #if BUFSIZE >= 1024, since if BUFSIZE is not defined, it will be interpreted as having the value zero. "

https://gcc.gnu.org/onlinedocs/cpp/Defined.html

[PaulStoffregen]

Remove the RAMSTART logic

If we do this, users with Arduino Uno & Nano (the most popular boards) will have too much RAM used. Those chips have only 2K RAM.