Pages: [1]   Go Down
Author Topic: Multibyte Ethernet Client::read()  (Read 2048 times)
0 Members and 1 Guest are viewing this topic.
California
Offline Offline
Newbie
*
Karma: 0
Posts: 12
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The single-byte Client::read() function in the Ethernet module is horrendously expensive.  It seems to be able to read less than 3k bytes per second.  Since there's already a multibyte write() function, it makes sense to also have a multibyte read().  Patch follows:


--- Client.h#   2010-10-02 13:17:42.000000000 -0700
+++ Client.h    2010-12-30 01:24:31.000000000 -0800
@@ -17,6 +17,7 @@
   virtual void write(const uint8_t *buf, size_t size);
   virtual int available();
   virtual int read();
+  virtual size_t read(uint8_t *buf, size_t size);
   virtual int peek();
   virtual void flush();
   void stop();


--- Client.cpp# 2010-10-02 13:17:42.000000000 -0700
+++ Client.cpp  2010-12-30 03:14:40.000000000 -0800
@@ -83,6 +83,18 @@
   return b;
 }

+size_t Client::read(uint8_t *buf, size_t size) {
+  size_t av = available();
+  if (size > av) {
+    size = av;
+  }
+  if (size == 0) {
+    return 0;
+  }
+  recv(_sock, buf, size);
+  return size;
+}
+
 int Client::peek() {
   uint8_t b;
   if (!available())
Logged

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

The return code from this version of read, when there is not data to read, is 0. For other read methods, when there is nothing to read, the return code is -1.

I would prefer consistency.
Logged

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

Well.  It returns the number for bytes read.  Returning 0 for reading 0 bytes is consistent with that.

Also, see "man 2 read" on any Linux/Unix machine: "On success, the number of bytes read is returned (zero indicates end of file)..."  OK.  I know Client::read() doesn't have to be consistent with Unix read(), but it does indicate that's the sensible thing to do.

Client::read(void) returns -1 because 0 is a valid return value, which is the behavior of Unix getchar(), also following the same design necessities.
Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 24
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


Please see socket.cpp file and recv function (arduino ide ver 21):

Code:
/**
 * @brief      This function is an application I/F function which is used to receive the data in TCP mode.
 *             It continues to wait for data as much as the application wants to receive.
 *             
 * @return      received data size for success else -1.
 */
uint16_t recv(SOCKET s, uint8_t *buf, uint16_t len)
{
  uint16_t ret=0;

  if ( len > 0 )
  {
    W5100.recv_data_processing(s, buf, len);
    W5100.execCmdSn(s, Sock_RECV);
    ret = len;
  }
  return ret;
}

It will not return -1 in any case smiley-wink Above description for return value is wrong smiley-wink

Anyway, I think that is optimized version for this function:

+size_t Client::read(uint8_t *buf, size_t size) {
+  size_t av = available();
+  if (size > av) {
+    size = av;
+  }
+  return (size_t)recv(_sock, buf, size);
+ }
Logged

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

Thanks.  You might want to add this to the Google Code issue for this feature: http://code.google.com/p/arduino/issues/detail?id=416&q=read%20stream

However, I think everything you pointed out may already have been done.
Logged

Pages: [1]   Go Up
Jump to: