arduino-libraries / Ethernet

Ethernet Library for Arduino
http://arduino.cc/
257 stars 262 forks source link

No way to set SPI_ETHERNET_SETTINGS or MAX_SOCK_NUM without modifying the library #267

Open joeyparrish opened 3 months ago

joeyparrish commented 3 months ago

My project needs high throughput, and I can achieve it with these settings:

#define ETHERNET_LARGE_BUFFERS
#define MAX_SOCK_NUM 1
#define SPI_ETHERNET_SETTINGS SPISettings(80000000, MSBFIRST, SPI_MODE0)

But there's no way to do this configuration purely from my project. I have to edit the installed Ethernet library, which is ugly and makes it harder for others to build my project.

Trying to set these on the command line, for example, with:

arduino-cli compile -b rp2040:rp2040:rpipicow --build-property "build.extra_flags=-DETHERNET_LARGE_BUFFERS -DMAX_SOCK_NUM=1 \"-DSPI_ETHERNET_SETTINGS=SPISettings(80000000, MSBFIRST, SPI_MODE0)\""

results in warning like these:

In file included from /home/joey/Arduino/libraries/Ethernet/src/Dhcp.cpp:5:
/home/joey/Arduino/libraries/Ethernet/src/Ethernet.h:39: warning: "MAX_SOCK_NUM" redefined
   39 | #define MAX_SOCK_NUM 8
      | 
<command-line>: note: this is the location of the previous definition

Then my command-line settings do nothing, because the header definitions occurred "later".

The solution is to make the defaults for MAX_SOCK_NUM and SPI_ETHERNET_SETTINGS conditional on there not being a previous definition. Would that be acceptable? For example:

diff --git a/src/Ethernet.h b/src/Ethernet.h
index 0045de8..45ec527 100644
--- a/src/Ethernet.h
+++ b/src/Ethernet.h
@@ -33,10 +33,12 @@
 // up to 4 sockets.  W5200 & W5500 can have up to 8 sockets.  Several bytes
 // of RAM are used for each socket.  Reducing the maximum can save RAM, but
 // you are limited to fewer simultaneous connections.
-#if defined(RAMEND) && defined(RAMSTART) && ((RAMEND - RAMSTART) <= 2048)
-#define MAX_SOCK_NUM 4
-#else
-#define MAX_SOCK_NUM 8
+#if !defined(MAX_SOCK_NUM)
+# if defined(RAMEND) && defined(RAMSTART) && ((RAMEND - RAMSTART) <= 2048)
+#  define MAX_SOCK_NUM 4
+# else
+#  define MAX_SOCK_NUM 8
+# endif
 #endif

 // By default, each socket uses 2K buffers inside the WIZnet chip.  If
diff --git a/src/utility/w5100.h b/src/utility/w5100.h
index b2e8ec8..b5b815a 100644
--- a/src/utility/w5100.h
+++ b/src/utility/w5100.h
@@ -18,7 +18,9 @@
 #include <SPI.h>

 // Safe for all chips
-#define SPI_ETHERNET_SETTINGS SPISettings(14000000, MSBFIRST, SPI_MODE0)
+#if !defined(SPI_ETHERNET_SETTINGS)
+# define SPI_ETHERNET_SETTINGS SPISettings(14000000, MSBFIRST, SPI_MODE0)
+#endif

 // Safe for W5200 and W5500, but too fast for W5100
 // Uncomment this if you know you'll never need W5100 support.
joeyparrish commented 3 months ago

Sent a PR in case the above solution is acceptable.