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.

[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.

[SapientHetero]

Perhaps detecting the board type directly instead of inferring it is a better answer. #if defined(TEENSYDUINO) // --------------- Teensy ----------------- #if defined(__AVR_ATmega32U4__) #define BOARD "Teensy 2.0" #elif defined(__AVR_AT90USB1286__) #define BOARD "Teensy++ 2.0" #elif defined(__MK20DX128__) #define BOARD "Teensy 3.0" #elif defined(__MK20DX256__) #define BOARD "Teensy 3.2" // and Teensy 3.1 (obsolete) #elif defined(__MKL26Z64__) #define BOARD "Teensy LC" #elif defined(__MK64FX512__) #define BOARD "Teensy 3.5" #elif defined(__MK66FX1M0__) #define BOARD "Teensy 3.6" #else #error "Unknown board" #endif #else // --------------- Arduino ------------------ #if defined(ARDUINO_AVR_ADK) #define BOARD "Mega Adk" #elif defined(ARDUINO_AVR_BT) // Bluetooth #define BOARD "Bt" #elif defined(ARDUINO_AVR_DUEMILANOVE) #define BOARD "Duemilanove" #elif defined(ARDUINO_AVR_ESPLORA) #define BOARD "Esplora" #elif defined(ARDUINO_AVR_ETHERNET) #define BOARD "Ethernet" #elif defined(ARDUINO_AVR_FIO) #define BOARD "Fio" #elif defined(ARDUINO_AVR_GEMMA) #define BOARD "Gemma" #elif defined(ARDUINO_AVR_LEONARDO) #define BOARD "Leonardo" #elif defined(ARDUINO_AVR_LILYPAD) #define BOARD "Lilypad" #elif defined(ARDUINO_AVR_LILYPAD_USB) #define BOARD "Lilypad Usb" #elif defined(ARDUINO_AVR_MEGA) #define BOARD "Mega" #elif defined(ARDUINO_AVR_MEGA2560) #define BOARD "Mega 2560" #elif defined(ARDUINO_AVR_MICRO) #define BOARD "Micro" #elif defined(ARDUINO_AVR_MINI) #define BOARD "Mini" #elif defined(ARDUINO_AVR_NANO) #define BOARD "Nano" #elif defined(ARDUINO_AVR_NG) #define BOARD "NG" #elif defined(ARDUINO_AVR_PRO) #define BOARD "Pro" #elif defined(ARDUINO_AVR_ROBOT_CONTROL) #define BOARD "Robot Ctrl" #elif defined(ARDUINO_AVR_ROBOT_MOTOR) #define BOARD "Robot Motor" #elif defined(ARDUINO_AVR_UNO) #define BOARD "Uno" #elif defined(ARDUINO_AVR_YUN) #define BOARD "Yun" // These boards must be installed separately: #elif defined(ARDUINO_SAM_DUE) #define BOARD "Due" #elif defined(ARDUINO_SAMD_ZERO) #define BOARD "Zero" #elif defined(ARDUINO_ARC32_TOOLS) #define BOARD "101" #else #error "Unknown board" #endif #endif On Oct 8, 2018 11:11 AM, "Paul Stoffregen" <notifications@github.com> wrote: 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.