itsanjan / arduino

Automatically exported from code.google.com/p/arduino
Other
0 stars 0 forks source link

Ethernet library fix for avr-gcc v4.5.1 #605

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I am using a Linux version compiler.
Arduino V0022
avr-gcc v4.5.1
avr-libc v1.7.1
GCC v4.5.2

Here is the subject at the Arduino forum
http://arduino.cc/forum/index.php/topic,68624.0.html

The ethernet shield failed when I upgraded everything. The value returned by 
Client::available() was always larger than 1024.

I found the cause to be the 16 bit socket register read function. This is the 
old code at line 253 of w5100.h:

#define __SOCKET_REGISTER16(name, address)                   \
  static void write##name(SOCKET _s, uint16_t _data) {       \
    writeSn(_s, address,   _data >> 8);                      \
    writeSn(_s, address+1, _data & 0xFF);                    \
  }                                                          \
  static uint16_t read##name(SOCKET _s) {                    \
    uint16_t res = readSn(_s, address);                      \
    res = (res << 8) + readSn(_s, address + 1);              \
    return res;                                              \
  } 

This is the new code that corrected the problem:

#define __SOCKET_REGISTER16(name, address)                   \
  static void write##name(SOCKET _s, uint16_t _data) {       \
    writeSn(_s, address,   _data >> 8);                      \
    writeSn(_s, address+1, _data & 0xFF);                    \
  }                                                          \
  static uint16_t read##name(SOCKET _s) {                    \
    uint16_t res = readSn(_s, address);                      \
    uint16_t res2 = readSn(_s,address + 1);             \
    res = res << 8;                     \
    res2 = res2 & 0xFF;                     \
    res = res | res2;                       \
    return res;                                              \
  }

Original issue reported on code.google.com by t...@prolectron.com on 28 Aug 2011 at 5:16

GoogleCodeExporter commented 9 years ago
Weird.  Do you why the old code doesn't work right (i.e. is it a problem with 
the values returned by readSn() or in the way the bitmath is done to combine 
them)?  

Adrian, can you take a look at this?  (Although there are other problems with 
newer version of the toolchain, so I would do the other refactorings we've been 
discussing first.)

Original comment by dmel...@gmail.com on 28 Aug 2011 at 6:22

GoogleCodeExporter commented 9 years ago
I think it is a partially uninitialized variable/register. The readSn returns 
only a byte (8 bits). But it is being put into a 16 bit variable/register.

The shift 8 bits left takes care of the first read. But it appears when the 
second read is done (integer math by now), the upper 8 bits are not cleared 
before doing the add. It appears it leaves bit 10 set. I can't explain that tho.

By reading it into a variable and "& 0xFF", that clears the errant bit.

Original comment by t...@prolectron.com on 28 Aug 2011 at 6:50

GoogleCodeExporter commented 9 years ago
I can confirm this bug, it also affects AVR Studio 5 on windows. This patch 
fixes it for me.

Original comment by robindegen on 11 Oct 2011 at 6:41

GoogleCodeExporter commented 9 years ago
I just wanted to point out the cli made a pull request for this bug:
https://github.com/arduino/Arduino/pull/42

Original comment by showard...@gmail.com on 16 Oct 2011 at 10:21

GoogleCodeExporter commented 9 years ago
I used this code to fix my Arduino-project using MacOSX Lion. Since then, my 
Arduino has been running for 5+ days, whereas it would 'hang' after a day or so.

Original comment by wout...@gmail.com on 24 Oct 2011 at 8:46

GoogleCodeExporter commented 9 years ago
Bug still in Arduino-1.0, so 1.0 isn't working on Ubuntu 11.10 from stock.

Original comment by TobiasBr...@gmail.com on 2 Dec 2011 at 8:04

GoogleCodeExporter commented 9 years ago
The Ubuntu repository version (Arduino V0022) works fine now. Arduino V0023 and 
V1.0 (not from the repository) apparently do not. This is not really a Ubuntu 
problem. That would be up to the Arduino crew to fix those two versions.

Original comment by t...@prolectron.com on 2 Dec 2011 at 8:47

GoogleCodeExporter commented 9 years ago
the ubuntu repository version of 1.0 will have the patch and work too

Original comment by showard...@gmail.com on 2 Dec 2011 at 10:32

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
> Comment #9 on issue 605 by migaxmoi...@gmail.com: Ethernet library fix for
> avr-gcc v4.5.1
> http://code.google.com/p/arduino/issues/detail?id=605
>
> and do you know when will it be on the ubuntu repos? I need this patch but
> for some reason can't build the source (incompetence most probably)

It's already in Debian, and probably will be ~1 week before it's in Ubuntu 
(they take it from Debian after about a 10 day delay)

You can just grab the debian package from here
http://packages.debian.org/sid/all/arduino-core/download
and
http://packages.debian.org/sid/all/arduino/download
save both
arduino-core_1.0+dfsg-1_all.deb
and
arduino_1.0+dfsg-1_all.deb

you can just double click on the arduino-core deb first, then on arduino one. 
Ubuntu should just install it for you.

Original comment by showard...@gmail.com on 8 Dec 2011 at 2:08

GoogleCodeExporter commented 9 years ago
https://github.com/arduino/Arduino/commit/597da2e45d63abd27debea17ec9065fd0657da
b2

Original comment by dmel...@gmail.com on 8 Dec 2011 at 9:54

GoogleCodeExporter commented 9 years ago
I logged a gcc bug for this against gcc-4.6.1 where I saw the problem:

   http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51445

But the problem does not seem to manifest in gcc-4.6.2.

Original comment by dhowell...@googlemail.com on 11 Dec 2011 at 2:00

GoogleCodeExporter commented 9 years ago
If the problem does not exist in gcc 4.6.2, then you may leave out this line:
res2 = res2 & 0xFF;     
Now it is faster executing than the fix above, and the fix above is faster than 
the original code. Unless things have changed a lot, then binary shift is 
faster than integer multiply, and binary OR is faster than integer add.

Original comment by t...@prolectron.com on 11 Dec 2011 at 6:52

GoogleCodeExporter commented 9 years ago
> Unless things have changed a lot, then binary shift is faster than integer 
multiply,

The compiler will turn an integer multiply into a binary shift if it can.

> and binary OR is faster than integer add.

More likely they take exactly the same amount of time as far as the CPU is 
concerned. According to the AVR instruction set manual, both instruction take 1 
cycle.

Original comment by dhowell...@googlemail.com on 11 Dec 2011 at 11:45

GoogleCodeExporter commented 9 years ago
This bug is still present in V1.0 ethernet library. It still locks up in the 
while(client.available()) loop.

If it doesn't work, it isn't faster.

Original comment by t...@prolectron.com on 12 Jan 2012 at 8:51

GoogleCodeExporter commented 9 years ago
Yes, the patch was applied after the release of 1.0 and will be in Arduino 
1.0.1.

Original comment by dmel...@gmail.com on 12 Jan 2012 at 11:20

GoogleCodeExporter commented 9 years ago
I download the zip of Arduino 1.0 for linux and it doesn't have this changes

Original comment by atomsm...@gmail.com on 3 Feb 2012 at 2:19

GoogleCodeExporter commented 9 years ago
The comment immediately above your comment says:
"Yes, the patch was applied after the release of 1.0 and will be in Arduino 
1.0.1."

Original comment by showard...@gmail.com on 3 Feb 2012 at 2:24

GoogleCodeExporter commented 9 years ago
Quote:

The comment immediately above your comment says:
"Yes, the patch was applied after the release of 1.0 and will be in Arduino 
1.0.1."

...and that will be when? I'm getting a bit tired of patching this for you!

Original comment by t...@prolectron.com on 3 Feb 2012 at 1:00

GoogleCodeExporter commented 9 years ago
1+ week later, can someone confirm that release 1.0 (or later) works in Ubuntu 
11.10? Where is the Arduino 1.0.1 for Ubuntu? Certainly not in 
http://arduino.cc/en/Main/Software. Of course, if you look in the repository 
you will find v022 :-/

Oh, well, I've updated "manually" to Arduino 1.0 and I can't manage to get the 
webserver example sketch up and running, doesn't work (yes it works in v022 
just fine and yes I've been careful picking up the proper example for each 
version, only changing MAC address and IP to suit)...

Original comment by almah...@googlemail.com on 13 Feb 2012 at 1:41

GoogleCodeExporter commented 9 years ago
I'm not on the dev team, but hopefully I can point someone in the right 
direction

1.0 does work on Ubuntu. However, the ethernet library and release 1.0 will not 
work on ubuntu 11.10. That has been confirmed.

1.0.1 has not been released on any platform yet.

However, the "arduino" package in 12.04 (precise) is version 1.0 and does have 
the ethernet library fix built into it. You can get it here:
http://packages.ubuntu.com/precise/all/arduino-core/download
http://packages.ubuntu.com/precise/all/arduino/download
just click on a server near you, download the file somewhere. Double click 
arduino-core first and choose "install", then double click on "arduino" and 
choose install. You now will have an arduino icon in your menus and can call 
arduino from the terminal or just by entering it into Dash (Unity).

Original comment by showard...@gmail.com on 13 Feb 2012 at 2:02

GoogleCodeExporter commented 9 years ago
IDE V1.0 works with Ubuntu 11.10 if you apply this patch. Verified by me.

Also, this bug will affect udp and dhcp, so
Ethernet.begin(mac)
may never return pass or fail. It locks in the routine. It is due to this code 
in EthernetUdp.cpp

int EthernetUDP::available() {
  return W5100.getRXReceivedSize(_sock);
}

Original comment by t...@prolectron.com on 26 Feb 2012 at 10:56

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
This patch worked for me!
Helped me get rid of random UDP packets.

Original comment by jitesh.p...@gmail.com on 11 Mar 2012 at 6:59

GoogleCodeExporter commented 9 years ago
Issue 872 has been merged into this issue.

Original comment by dmel...@gmail.com on 8 Apr 2012 at 9:50