Fix: Ethernet_STM-master fails with W5200 chip

Hi,
I noticed that Vassilis mentioned that he could not test the W5200 version before submitting. But Murphy takes care !
The two methods
uint8_t W5200Class::read(uint16_t _addr)
and
uint16_t W5200Class::read(uint16_t _addr, uint8_t *_buf, uint16_t _len)
missed the proper setting of the CS signal.
I fixed it as shown below.
Vassili, or whoever is in charge of it, could your please re-submit the library including this fix?

********************************************

uint8_t W5200Class::read(uint16_t _addr)
{

#if defined(ARDUINO_ARCH_AVR)
setSS();
SPI.transfer(_addr >> 8);
SPI.transfer(_addr & 0xFF);
SPI.transfer(0x00);
SPI.transfer(0x01);
uint8_t _data = SPI.transfer(0);
resetSS();
#elif defined(__STM32F1__)
[b]digitalWrite(STM32_SPI_CS, LOW);[/b] SPI.transfer(_addr >> 8);
SPI.transfer(_addr & 0xFF);
SPI.transfer(0x00);
SPI.transfer(0x01);
uint8_t _data = SPI.transfer(0);
[b]digitalWrite(STM32_SPI_CS, HIGH);[/b] #else
SPI.transfer(SPI_CS, _addr >> 8, SPI_CONTINUE);
SPI.transfer(SPI_CS, _addr & 0xFF, SPI_CONTINUE);
SPI.transfer(SPI_CS, 0x00, SPI_CONTINUE);
SPI.transfer(SPI_CS, 0x01, SPI_CONTINUE);
uint8_t _data = SPI.transfer(SPI_CS, 0);
#endif
return _data;
}

uint16_t W5200Class::read(uint16_t _addr, uint8_t *_buf, uint16_t _len)
{

#if defined(ARDUINO_ARCH_AVR)
setSS();
SPI.transfer(_addr >> 8);
SPI.transfer(_addr & 0xFF);
SPI.transfer((0x00 | ((_len & 0x7F00) >> 8)));
SPI.transfer(_len & 0x00FF);

for (uint16_t i=0; i<_len; i++)
{
_buf[i] = SPI.transfer(0);

}
resetSS();
#elif defined(__STM32F1__)
[b] digitalWrite(STM32_SPI_CS, LOW);[/b] SPI.transfer(_addr >> 8);
SPI.transfer(_addr & 0xFF);
SPI.transfer(0x00 | ((_len & 0x7F00) >> 8));
SPI.transfer(_len & 0x00FF);
for (uint16_t i=0; i<(_len-1); i++)
{
_buf[i] = SPI.transfer(0);
}
_buf[_len-1] = SPI.transfer(0);
[b]digitalWrite(STM32_SPI_CS, HIGH);[/b] #else
SPI.transfer(SPI_CS, _addr >> 8, SPI_CONTINUE);
SPI.transfer(SPI_CS, _addr & 0xFF, SPI_CONTINUE);
SPI.transfer(SPI_CS, (0x00 | ((_len & 0x7F00) >> 8)), SPI_CONTINUE);
SPI.transfer(SPI_CS, _len & 0x00FF, SPI_CONTINUE);
for (uint16_t i=0; i<(_len-1); i++)
{
_buf[i] = SPI.transfer(SPI_CS, 0, SPI_CONTINUE);
}
_buf[_len-1] = SPI.transfer(SPI_CS, 0);

#endif
return _len;
}

Thanks Phono
Did you check the library with a W5200 chip ?
Works ok ?

Vassili,
I checked the fix with a W5200 chip shield on an olimex STM 32 board. Works well. I also checked it on a W5100 chip. Worked well at once, no fix required.
I must tell that using the Maple Leaflabs IDE, I had written a variant of the Ethernet library to make it fully efficient when using FreeRTOS. Indeed, if you protect the accesses to the W5xx chip through SPI using critical sections, you can have several tasks handle several sockets without interference (the traffic of one socket does not freeze the other sockets traffic). I think this is a great improvement, and I suggest that we add this variant to the STM 32 for Arduino libraries.

Phono wrote:<…>
you can have several tasks handle several sockets without interference (the traffic of one socket does not freeze the other sockets traffic). I think this is a great improvement, and I suggest that we add this variant to the STM 32 for Arduino libraries.

To do so, we could produce a dedicated version of the library; but what would be nice, would be to have the library automatically handle the presence or the absence of te RTOS library. Do you know how to do this? The point is to add calls to “start critical section” and “end critical section” at various places of the Ethernet library, and in the case the RTOS library is not used in the sketch, these calls would just do nothing.

The Chip Select fix into the read() functions is done (w5200.cpp file).

Phono wrote:<…> The point is to add calls to “start critical section” and “end critical section” at various places of the Ethernet library, <…>

Phono wrote:To do so, we could produce a dedicated version of the library; but what would be nice, would be to have the library automatically handle the presence or the absence of te RTOS library. Do you know how to do this? The point is to add calls to “start critical section” and “end critical section” at various places of the Ethernet library, and in the case the RTOS library is not used in the sketch, these calls would just do nothing.

In the process of tracking a bug in my own version of the Ethernet library, that I call Ethernet_RTOS, I checked today its code against that of the main branch of the repository V1.0.0 beta.
I was surprised to find that the mistake I reported in my post of june 7, 2016 (in this topic), about missing actuation of the CS line in both read() methods of class W5200Class is still present in the repository version 1.0.0 Beta!

Another thing I do not understand : in the repository, in methods W5200Class::write() and W5200Class::read() there is the following code:
for (uint16_t i=0; i<(_len-1); i++)

{
_buf[i] = SPI.transfer(0);
}
_buf[_len-1] = SPI.transfer(0);


I write a separate post because I have met a bug in the operation of the ethernet shield, so this is distinct from the various comments of my previous post.
I am using the RTOS-friendly version I made of the Ethernet_STM library. Basically, I added semaphores to protect the accesses to the SPI to guarantee that the transfers are atomic, even if two or more tasks want to used the Ethernet interface at the same time.
I have built a heating regulation system for my house, that can be monitored and controlled from outside. For so, I have implemented a simple web interface. There is thus a simple web server in my code, and I have also coded a NTP client that retrieves the date and time for timestamping the data that is logged in a SD card.
I have connected the system to my optical fiber modem with the appropriate port redirection rules, so that I can access the web server from the WAN.
My system is installed in France. It works perfectly when I connect from anywhere in France. I have done extensive stress tests with more than 10 google chrome tabs each querying the web page every 5 seconds (a REFRESH=5 is included in the HTML code). This test ran for several days without any problem.
BUT, if I travel to another country (I tried England and Colombia), and I connect using my smartphone on a 3G connection, after a few queries (maybe a dozen), the web server fails and does not respond any more. When I get home, I notice that the NTP client still works, as well as the global regulation of my central heating. Only the web server is frozen. I have to reboot the system to recover the web access.
Any ideas about the difference between connecting from France and from another country?
How could I simulate connecting from another country while being in France, to perform testing?

[Phono – Mon Feb 18, 2019 9:11 pm] –
How could I simulate connecting from another country while being in France, to perform testing?

Sign up for a VPN service. The one I use (TorGuard) has about 70 different servers listed in about 30 different countries. I know they have an Apple client that I use on an iPad. I assume they also have one for Android. Right after I signed up, I was stuck in a hotel where they blocked nearly everything so I couldn’t connect to my home network. Once I connected to the VPN, I got in immediately. I pay $15 USD for 6 months of service.

I also have a similar setup and a similar issue.
I think the difference is that you use a PC in France and a mobile phone abroad.
I guess the Android web viewer can make the difference.
In my case the W5500 is not reachable in any case when hangs, independent on the client platform. Reset helps.

You are right saying that from abroad I connect on my android smartphone. But when I connect on my smartphone from France, it works just as well as on my computer.
So far, my concern is to conduct a stress test from abroad, and log the communication event here in France, until I see a freeze to see where the process is stuck. I have already attached a laptop to my system, and it will run 24/7 until the communication fails.
I just miss a site abroad that could connect to my system for several days, hopefully expecting a communication failure.

[Phono – Wed Feb 20, 2019 2:23 pm] –
I just miss a site abroad that could connect to my system for several days, hopefully expecting a communication failure.

Well, I could program one of my bluepills with W5500 to stress your site 24/7 if you want.
Just give me some more details.

Thanks for your offer. i am setting up a test rig, and when it is ready, I may accept your kind offer.

I eventually found the cause of the blocking i sometimes observe.
My application has a web server and also a NTP client. The latter uses UDP.
I spotted the problem in a TakeSemaphore() call in the web server module, that blocked because of a missing GiveSemaphore() in the UDP module.
So please change the following function as shown:
/* Start EthernetUDP socket, listening at local port PORT */
uint8_t EthernetUDP::begin(uint16_t port) {
if (_sock != MAX_SOCK_NUM)
return 0;

W5100.TakeSemaphore () ;
for (int i = 0; i < MAX_SOCK_NUM; i++) {
uint8_t s = W5100.readSnSR(i);
if (s == SnSR::CLOSED || s == SnSR::FIN_WAIT) {
_sock = i;
break;
}
}

if (_sock == MAX_SOCK_NUM)
{
W5100.GiveSemaphore () ; // This one was missing
return 0;
}

_port = port;
_remaining = 0;
socket(_sock, SnMR::UDP, _port, 0);
W5100.GiveSemaphore () ;
return 1;
}


[Phono – Wed Feb 27, 2019 10:52 pm] –
I eventually found the cause of the blocking i sometimes observe.
My application has a web server and also a NTP client. The latter uses UDP.
I spotted the problem in a TakeSemaphore() call in the web server module, that blocked because of a missing GiveSemaphore() in the UDP module.
So please change the following function as shown:
/* Start EthernetUDP socket, listening at local port PORT */
uint8_t EthernetUDP::begin(uint16_t port) {
if (_sock != MAX_SOCK_NUM)
return 0;

W5100.TakeSemaphore () ;
for (int i = 0; i < MAX_SOCK_NUM; i++) {
uint8_t s = W5100.readSnSR(i);
if (s == SnSR::CLOSED || s == SnSR::FIN_WAIT) {
_sock = i;
break;
}
}

if (_sock == MAX_SOCK_NUM)
{
W5100.GiveSemaphore () ; // This one was missing
return 0;
}

_port = port;
_remaining = 0;
socket(_sock, SnMR::UDP, _port, 0);
W5100.GiveSemaphore () ;
return 1;
}