Go Down

Topic: Multibyte Ethernet Client::read() (Read 2262 times) previous topic - next topic

Ellen W

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())

PaulS

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.

Ellen W

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.

arian


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

Code: [Select]

/**
* @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 ;) Above description for return value is wrong ;)

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);
+ }

Ellen W

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.

Go Up