Pages: [1] 2   Go Down
Author Topic: multiple bugs in ethernet library  (Read 3195 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 8
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi all.

I'm doing a little project and i've found a few bugs/missing features in the ethernet library that comes with arduino 1.0
in some cases the library behavior don't match the documentation.

One example:
the function W5100Class::getRXReceivedSize is used internally by the various ethernet classes in the available() function to check how much data is waiting (if any).
this is what it looks like right now:
Code:
uint16_t W5100Class::getRXReceivedSize(SOCKET s)
{
  uint16_t val=0,val1=0;
  do {
    val1 = readSnRX_RSR(s);
    if (val1 != 0)
      val = readSnRX_RSR(s);
  }
  while (val != val1);
  return val;
}
as you can see there is an infinite loop until the register changes.
so instead of available() checking the current status it will actually wait until we get something.
this is a significant difference and means you can't use it to poll for data.

in my case i need to check if i get something and there is no guarantee i will get anything at all so blocking is very bad (program lockup).


another one is that Ethernet.begin() don't match the documentation and documentation
if you use the form:
Ethernet.begin(mac, ip, gateway) or
Ethernet.begin(mac, ip, gateway, subnet)
you will set your dns server to gateway and that may not necessarily be what you wanted and in the later one also set the your gateway to the netmask.
the relevant parts of the code is like this:
Code:
  int begin(uint8_t *mac_address);
void begin(uint8_t *mac_address, IPAddress local_ip);
void begin(uint8_t *mac_address, IPAddress local_ip, IPAddress dns_server);
void begin(uint8_t *mac_address, IPAddress local_ip, IPAddress dns_server, IPAddress gateway);
void begin(uint8_t *mac_address, IPAddress local_ip, IPAddress dns_server, IPAddress gateway, IPAddress subnet);

personally i would have preferred if the dns was the last argument instead.
it would also have been nice if the dns parts was optional via either a define or some other way.
not all projects needs dns and in that case you don't need to waste code space for the dns class.
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 8
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

There probably also is another infinite loop in Ethernet\utility\socket.cpp function sendto()
if it was not possible to send data because for example the host is down (no reply to arp) then the function will get stuck waiting for SEND_OK that never arrives.
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 14
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi

I am using the Ethernet library also for a Webserver application (on a Arduino Ethernet board). All seemed to work fine, but adding more lines (Client.println) to the page makes it lock up. Specifically a new form of input type (button)

Any chance that a errata exists to report the errors found so I have a chance to complete my project? (I have limited knowledge of the Ethernet library, so finding them myself will take prohibitive long)

A lot of HTML code before, then adding these lines makes it lock up....

Code:
client.println("<form METHOD=get action=\"http://192.168.1.177/\">");
client.println("<input type=hidden name=\"cmd\" value=\"5\">");
client.println("<input type=submit value=\"Zone 2 On\">");
client.println("</form>");

Regards

Klaus
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 14
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Does a previos version of the Ethernet library exist that has less bugs (I am running Arduino 1.0)

Regards

Klaus
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 614
Posts: 49365
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
All seemed to work fine, but adding more lines (Client.println) to the page makes it lock up. Specifically a new form of input type (button)
Most likely, this is not an ethernet issue. It is a "you've run out of memory" issue. There are a couple of freeMemory() functions floating around that will tell you whether this is your problem, or not.
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 614
Posts: 49365
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Does a previos version of the Ethernet library exist that has less bugs (I am running Arduino 1.0)
Just wait until 1.1 comes out. The Arduino team is hard at work developing new bugs.
Logged

Miramar Beach, Florida
Offline Offline
Faraday Member
**
Karma: 146
Posts: 6027
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
as you can see there is an infinite loop until the register changes.
Actually, there is a loop until the register stops changing. It is in that loop until the two reads are the same.
Code:
 do {
    // read it once
    val1 = readSnRX_RSR(s);
    if (val1 != 0)
      // read it again
      val = readSnRX_RSR(s);
  }
  // keep doing it until the receiver has stopped receiving new characters (both reads are the same).
  while (val != val1);

Edit: If the client.available() function seems to lock up, there is another possible cause. Here is the bug and fix.
http://code.google.com/p/arduino/issues/detail?id=605
« Last Edit: January 15, 2012, 10:19:10 pm by SurferTim » Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 14
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Most likely, this is not an ethernet issue. It is a "you've run out of memory" issue. There are a couple of freeMemory() functions floating around that will tell you whether this is your problem, or not.
Wow. Maybe I am missing something, but aren't these lines on in flash (code), so how can it result in a memory problem?

Or is it a problem with the W5100 chip, perhaps I need to flush something to be sure the device can handle the packets to send without overflow (just guessing, I don't know how the interfacing is done)

Bytheway, I have included Ethernet.h in mo Sketch. However #include W5100.h is commented out in the Ethernet.h file(//#include "w5100.h"), so where is the hardware defines for the W5100 located?

Thanks

Klaus
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 614
Posts: 49365
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Maybe I am missing something, but aren't these lines on in flash (code), so how can it result in a memory problem?
The executable code is in flash. The strings that are defined are stored in SRAM.
Logged

Miramar Beach, Florida
Offline Offline
Faraday Member
**
Karma: 146
Posts: 6027
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Maybe I am missing something, but aren't these lines on in flash (code), so how can it result in a memory problem?
The executable code is in flash. The strings that are defined are stored in SRAM.
@PaulS: Thanks for that bit of info. I was under a misconception that any strings were in program flash only. After some quick tests, I find it not only increases the flash (program) code by that amount, it also decreases the SRAM by that amount when the code refers to that string. So, in fact, it uses both.

I guess that SD card experiment will pay off for me about now.  smiley
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 8
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
as you can see there is an infinite loop until the register changes.
Actually, there is a loop until the register stops changing. It is in that loop until the two reads are the same.
yes you are right, i misread the code a bit

Edit: If the client.available() function seems to lock up, there is another possible cause. Here is the bug and fix.
http://code.google.com/p/arduino/issues/detail?id=605
interesting i will have to take a look at that.

the issue i have currently is that the later part of sendto() in socket.cpp never gets the timeout flag.
my current test case is raw ip to a non existent host, timeout bit in SnIR should be set in this case and i can see that multiple arp requests is being sent on the network and then the device stops doing anything useful.
adding a few Serial.println() showed that sendto() never completed.
did a little workaround to make it exit quicker but then the check to see if there was any data got stuck instead (available())

anyway, i will have to do some more debugging to find the root of my issues.
it may also be possible that i'm running out of ram as suggested elsewhere in this thread.


another question, is there a reason why for example EthernetClient don't have a destructor?
it is very easy to leak sockets by forgetting to call EthernetClient::stop() and a simple fix is to just add a destructor that calls stop().
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 8
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The Arduino team is hard at work developing new bugs.
aren't we all ;-)
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 8
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

i've found another bug but this is not realy an arduino bug, more like an unfortunate circumstance.

what i have is a class for sending commands to lirc on my linux box over the network.
when i use EthernetClient.print() (all data is sent at once, not multiple calls to print) to send the data it gets broken up into multiple packets even when it doesn't have to.

the result is that lirc on the linux side gets it wrong, it probably is making some bad assumptions and assumes all data will be in one packet (one read) instead of reading until it gets a new line character.

if i use EthernetClient.write() it works fine, code gets a bit ugly because the String class don't have a function to gain direct access to the internal buffer.
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 614
Posts: 49365
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
if i use EthernetClient.write() it works fine, code gets a bit ugly because the String class don't have a function to gain direct access to the internal buffer.
The String class has a toCharArray() method to get the whole array and a charAt() method to get one character. Use a for loop to get one character at a time, or make a copy of the whole buffer.

Better yet, quit using the String class. People wrote extensive text processing applications for decades before the String class wrapped the C code that really does all the work.
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 14
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Thanks PaulS

I have move many of my client.println("HTML text") strings into PROGMEM, and released already 250bytes of RAM (I was down to 166 bytes), so no wonder it misbehaved. You suggestion was correct, it was stack overflow into the RAM space.

Now I just get funny errors in the HTML send back to the client when the Arduino server gets a HTTP request. Sometimes the browser just fills with text, up to 30 pages of text, eventhough my application only fill maximum ½ page.

I have a click counter, to count the number of times the page is loaded and it is not loaded many times, just repeated many times on the same browser page (a mixture of current click, and also some older clicks - like the browser by itself takes in old data and mix with new). Funny stuff :-(

Regards

Klaus
Logged

Pages: [1] 2   Go Up
Jump to: