Corrects assumptions about return values from i2c-lib (#484)

Fixes #478

hal/lib/athena/I2C.cpp

@@ -78,7 +78,7 @@ void HAL_InitializeI2C(int32_t port, int32_t* status) {
  * @param sendSize Number of bytes to send as part of the transaction.
  * @param dataReceived Buffer to read data into.
  * @param receiveSize Number of bytes to read from the device.
- * @return The number of bytes read (>= 0) or -1 on transfer abort.
+ * @return >= 0 on success or -1 on transfer abort.
  */
 int32_t HAL_TransactionI2C(int32_t port, int32_t deviceAddress,
                            uint8_t* dataToSend, int32_t sendSize,
@@ -110,7 +110,7 @@ int32_t HAL_TransactionI2C(int32_t port, int32_t deviceAddress,
  * @param registerAddress The address of the register on the device to be
  * written.
  * @param data The byte to write to the register on the device.
- * @return The number of bytes written (>= 0) or -1 on transfer abort.
+ * @return >= 0 on success or -1 on transfer abort.
  */
 int32_t HAL_WriteI2C(int32_t port, int32_t deviceAddress, uint8_t* dataToSend,
                      int32_t sendSize) {
@@ -140,7 +140,7 @@ int32_t HAL_WriteI2C(int32_t port, int32_t deviceAddress, uint8_t* dataToSend,
  * @param count The number of bytes to read in the transaction.
  * @param buffer A pointer to the array of bytes to store the data read from the
  * device.
- * @return The number of bytes read (>= 0) or -1 on transfer abort.
+ * @return >= 0 on success or -1 on transfer abort.
  */
 int32_t HAL_ReadI2C(int32_t port, int32_t deviceAddress, uint8_t* buffer,
                     int32_t count) {

wpilibc/athena/src/I2C.cpp

@@ -51,7 +51,7 @@ bool I2C::Transaction(uint8_t* dataToSend, int sendSize, uint8_t* dataReceived,
   status = HAL_TransactionI2C(m_port, m_deviceAddress, dataToSend, sendSize,
                               dataReceived, receiveSize);
   // wpi_setErrorWithContext(status, HAL_GetErrorMessage(status));
-  return status < receiveSize;
+  return status < 0;
 }
 
 /**
@@ -81,7 +81,7 @@ bool I2C::Write(int registerAddress, uint8_t data) {
   buffer[1] = data;
   int32_t status = 0;
   status = HAL_WriteI2C(m_port, m_deviceAddress, buffer, sizeof(buffer));
-  return status < static_cast<int>(sizeof(buffer));
+  return status < 0;
 }
 
 /**
@@ -97,7 +97,7 @@ bool I2C::Write(int registerAddress, uint8_t data) {
 bool I2C::WriteBulk(uint8_t* data, int count) {
   int32_t status = 0;
   status = HAL_WriteI2C(m_port, m_deviceAddress, data, count);
-  return status < count;
+  return status < 0;
 }
 
 /**
@@ -122,8 +122,8 @@ bool I2C::Read(int registerAddress, int count, uint8_t* buffer) {
     wpi_setWPIErrorWithContext(NullParameter, "buffer");
     return true;
   }
-  return Transaction(reinterpret_cast<uint8_t*>(&registerAddress),
+  uint8_t regAddr = registerAddress;
-                     sizeof(registerAddress), buffer, count);
+  return Transaction(&regAddr, sizeof(regAddr), buffer, count);
 }
 
 /**
@@ -146,9 +146,7 @@ bool I2C::ReadOnly(int count, uint8_t* buffer) {
     wpi_setWPIErrorWithContext(NullParameter, "buffer");
     return true;
   }
-  int32_t status = 0;
+  return HAL_ReadI2C(m_port, m_deviceAddress, buffer, count) < 0;
-  status = HAL_ReadI2C(m_port, m_deviceAddress, buffer, count);
-  return status < count;
 }
 
 /**

wpilibc/athena/src/RobotDrive.cpp

@@ -140,14 +140,14 @@ RobotDrive::RobotDrive(std::shared_ptr<SpeedController> leftMotor,
  *
  * Speed controller input version of RobotDrive (see previous comments).
  *
- * @param rearLeftMotor   The back left SpeedController object used to drive
- *                        the robot.
  * @param frontLeftMotor  The front left SpeedController object used to drive
  *                        the robot.
- * @param rearRightMotor  The back right SpeedController object used to drive
+ * @param rearLeftMotor   The back left SpeedController object used to drive
  *                        the robot.
  * @param frontRightMotor The front right SpeedController object used to drive
  *                        the robot.
+ * @param rearRightMotor  The back right SpeedController object used to drive
+ *                        the robot.
  */
 RobotDrive::RobotDrive(SpeedController* frontLeftMotor,
                        SpeedController* rearLeftMotor,

wpilibc/athena/src/SPI.cpp

@@ -12,6 +12,7 @@
 
 #include "HAL/HAL.h"
 #include "WPIErrors.h"
+#include "llvm/SmallVector.h"
 
 using namespace frc;
 
@@ -147,9 +148,9 @@ int SPI::Write(uint8_t* data, int size) {
 int SPI::Read(bool initiate, uint8_t* dataReceived, int size) {
   int retVal = 0;
   if (initiate) {
-    auto dataToSend = new uint8_t[size];
+    llvm::SmallVector<uint8_t, 32> dataToSend;
-    std::memset(dataToSend, 0, size);
+    dataToSend.resize(size);
-    retVal = HAL_TransactionSPI(m_port, dataToSend, dataReceived, size);
+    retVal = HAL_TransactionSPI(m_port, dataToSend.data(), dataReceived, size);
   } else {
     retVal = HAL_ReadSPI(m_port, dataReceived, size);
   }

wpilibj/src/athena/java/edu/wpi/first/wpilibj/I2C.java

@@ -83,7 +83,7 @@ public class I2C extends SensorBase {
     if (receiveSize > 0 && dataReceived != null) {
       dataReceivedBuffer.get(dataReceived);
     }
-    return status < receiveSize;
+    return status < 0;
   }
 
   /**
@@ -117,7 +117,7 @@ public class I2C extends SensorBase {
     }
 
     return I2CJNI.i2CTransaction((byte) m_port.value, (byte) m_deviceAddress, dataToSend,
-                                 (byte) sendSize, dataReceived, (byte) receiveSize) < receiveSize;
+                                 (byte) sendSize, dataReceived, (byte) receiveSize) < 0;
   }
 
   /**
@@ -139,6 +139,7 @@ public class I2C extends SensorBase {
    *
    * @param registerAddress The address of the register on the device to be written.
    * @param data            The byte to write to the register on the device.
+   * @return Transfer Aborted... false for success, true for aborted.
    */
   public synchronized boolean write(int registerAddress, int data) {
     byte[] buffer = new byte[2];
@@ -149,7 +150,7 @@ public class I2C extends SensorBase {
     dataToSendBuffer.put(buffer);
 
     return I2CJNI.i2CWrite((byte) m_port.value, (byte) m_deviceAddress, dataToSendBuffer,
-                           (byte) buffer.length) < buffer.length;
+                           (byte) buffer.length) < 0;
   }
 
   /**
@@ -158,13 +159,14 @@ public class I2C extends SensorBase {
    * <p>Write multiple bytes to a register on a device and wait until the transaction is complete.
    *
    * @param data The data to write to the device.
+   * @return Transfer Aborted... false for success, true for aborted.
    */
   public synchronized boolean writeBulk(byte[] data) {
     ByteBuffer dataToSendBuffer = ByteBuffer.allocateDirect(data.length);
     dataToSendBuffer.put(data);
 
     return I2CJNI.i2CWrite((byte) m_port.value, (byte) m_deviceAddress, dataToSendBuffer,
-                           (byte) data.length) < data.length;
+                           (byte) data.length) < 0;
   }
 
   /**
@@ -173,6 +175,7 @@ public class I2C extends SensorBase {
    * <p>Write multiple bytes to a register on a device and wait until the transaction is complete.
    *
    * @param data The data to write to the device. Must be created using ByteBuffer.allocateDirect().
+   * @return Transfer Aborted... false for success, true for aborted.
    */
   public synchronized boolean writeBulk(ByteBuffer data, int size) {
     if (!data.isDirect()) {
@@ -183,7 +186,7 @@ public class I2C extends SensorBase {
           "buffer is too small, must be at least " + size);
     }
 
-    return I2CJNI.i2CWrite((byte) m_port.value, (byte) m_deviceAddress, data, (byte) size) < size;
+    return I2CJNI.i2CWrite((byte) m_port.value, (byte) m_deviceAddress, data, (byte) size) < 0;
   }
 
   /**
@@ -264,7 +267,7 @@ public class I2C extends SensorBase {
     int retVal = I2CJNI.i2CRead((byte) m_port.value, (byte) m_deviceAddress, dataReceivedBuffer,
                                 (byte) count);
     dataReceivedBuffer.get(buffer);
-    return retVal < count;
+    return retVal < 0;
   }
 
   /**
@@ -291,7 +294,7 @@ public class I2C extends SensorBase {
     }
 
     return I2CJNI.i2CRead((byte) m_port.value, (byte) m_deviceAddress, buffer, (byte) count)
-        < count;
+        < 0;
   }
 
   /*

wpilibj/src/athena/java/edu/wpi/first/wpilibj/RobotDrive.java

@@ -128,10 +128,10 @@ public class RobotDrive implements MotorSafety {
    * Constructor for RobotDrive with 4 motors specified as SpeedController objects. Speed controller
    * input version of RobotDrive (see previous comments).
    *
-   * @param rearLeftMotor   The back left SpeedController object used to drive the robot.
    * @param frontLeftMotor  The front left SpeedController object used to drive the robot
-   * @param rearRightMotor  The back right SpeedController object used to drive the robot.
+   * @param rearLeftMotor   The back left SpeedController object used to drive the robot.
    * @param frontRightMotor The front right SpeedController object used to drive the robot.
+   * @param rearRightMotor  The back right SpeedController object used to drive the robot.
    */
   public RobotDrive(SpeedController frontLeftMotor, SpeedController rearLeftMotor,
                     SpeedController frontRightMotor, SpeedController rearRightMotor) {

wpilibj/src/shared/java/edu/wpi/first/wpilibj/PIDController.java

@@ -727,7 +727,7 @@ public class PIDController implements PIDInterface, LiveWindowSendable, Controll
         setSetpoint((Double) value);
       }
     } else if (key.equals("enabled")) {
-      if (isEnable() != (Boolean) value) {
+      if (isEnabled() != (Boolean) value) {
         if ((Boolean) value) {
           enable();
         } else {
@@ -750,7 +750,7 @@ public class PIDController implements PIDInterface, LiveWindowSendable, Controll
       table.putNumber("d", getD());
       table.putNumber("f", getF());
       table.putNumber("setpoint", getSetpoint());
-      table.putBoolean("enabled", isEnable());
+      table.putBoolean("enabled", isEnabled());
       table.addTableListener(m_listener, false);
     }
   }