[PATCH] Wire library: coding unnecessarily complex

[hfvogt]

In the wire library there are several functions where an - unnecessarily - complex coding has been used:
endTransmission: the availability of data is already checked in while(...), therefore need not be checked again in the loop.
requestFrom: the for-loop has a predefined and fixed number of loops. Therefore a check whether the last element has been reached is unnecessary and does not add any benefit.

Attached are two patches to simplify these functions:

--- ArduinoCore-samd.orig/libraries/Wire/Wire.cpp   2015-07-16 10:36:01.416937191 +0200
+++ ArduinoCore-samd/libraries/Wire/Wire.cpp    2015-07-16 19:17:07.675799549 +0200
@@ -132,12 +132,8 @@ uint8_t TwoWire::endTransmission(bool st
       sercom->prepareCommandBitsWire(WIRE_MASTER_ACT_STOP);
       return 3 ;  // Nack or error
     }
-
-    if(txBuffer.available() == 0)
-    {
-      sercom->prepareCommandBitsWire(WIRE_MASTER_ACT_STOP);
-    }
   }
+  sercom->prepareCommandBitsWire(WIRE_MASTER_ACT_STOP);

   return 0;
 }

--- ArduinoCore-samd.orig/libraries/Wire/Wire.cpp   2015-07-16 19:17:07.675799549 +0200
+++ ArduinoCore-samd/libraries/Wire/Wire.cpp    2015-07-16 19:24:33.179778166 +0200
@@ -66,22 +66,15 @@ uint8_t TwoWire::requestFrom(uint8_t add
     rxBuffer.store_char(sercom->readDataWIRE());

     // Connected to slave
-    //while(toRead--)
-    for(byteRead = 0; byteRead < quantity; ++byteRead)
+    for(byteRead = 1; byteRead < quantity; ++byteRead)
     {
-      if( byteRead == quantity - 1)  // Stop transmission
-      {
-        sercom->prepareNackBitWIRE(); // Prepare NACK to stop slave transmission
-        //sercom->readDataWIRE(); // Clear data register to send NACK
-        sercom->prepareCommandBitsWire(WIRE_MASTER_ACT_STOP); // Send Stop
-      }
-      else // Continue transmission
-      {
-        sercom->prepareAckBitWIRE();  // Prepare Acknowledge
-        sercom->prepareCommandBitsWire(WIRE_MASTER_ACT_READ); // Prepare the ACK command for the slave
-        rxBuffer.store_char( sercom->readDataWIRE() );  // Read data and send the ACK
-      }
+      sercom->prepareAckBitWIRE();  // Prepare Acknowledge
+      sercom->prepareCommandBitsWire(WIRE_MASTER_ACT_READ); // Prepare the ACK command for the slave
+      rxBuffer.store_char( sercom->readDataWIRE() );  // Read data and send the ACK
     }
+    sercom->prepareNackBitWIRE(); // Prepare NACK to stop slave transmission
+    //sercom->readDataWIRE(); // Clear data register to send NACK
+    sercom->prepareCommandBitsWire(WIRE_MASTER_ACT_STOP); // Send Stop
   }

   return byteRead;

If this is the wrong place to provide patched please guide me to the right place. I have more patches that I would like to share.

[cmaglie]

Hi @hfvogt
I've edited and cleaned up your previous comments.

This is the right place for submitting patches, if you have others I'll surely take a look at them (but probably this won't happen before Monday).

Our preferred way to receive contributions is through Github's pull requests, because it simplifies the review by reducing the amount of cut&paste. Moreover a Pull Request can be automatically merged using the Github website, so if you have other patches I'd ask you to submit them as Pull Request.

Thank you!

[cmaglie]

LGTM I've opened a pull request on #25 for review.