From 686f5d9fef0eb88205aadd29d5e0aa29013ce0cb Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Wed, 23 Mar 2016 22:34:50 -0700 Subject: [PATCH] Fix incorrect comments in the HAL I2C functions and incorrect return values for some I2C class member functions. These issues were found and fixed while interfacing with an ITG3200 gyroscope. Fixes artf4173. Change-Id: Id3c6b34aa707650480e90605e98ae1b44d7a7b98 --- hal/lib/Athena/Digital.cpp | 6 +++--- wpilibc/Athena/src/I2C.cpp | 17 ++++++---------- .../java/edu/wpi/first/wpilibj/I2C.java | 20 +++++++++---------- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/hal/lib/Athena/Digital.cpp b/hal/lib/Athena/Digital.cpp index e82beca4ee..353933bbd8 100644 --- a/hal/lib/Athena/Digital.cpp +++ b/hal/lib/Athena/Digital.cpp @@ -1793,7 +1793,7 @@ void i2CInitialize(uint8_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 Transfer Aborted... false for success, true for aborted. + * @return The number of bytes read (>= 0) or -1 on transfer abort. */ int32_t i2CTransaction(uint8_t port, uint8_t deviceAddress, uint8_t *dataToSend, uint8_t sendSize, uint8_t *dataReceived, uint8_t receiveSize) { @@ -1819,7 +1819,7 @@ int32_t i2CTransaction(uint8_t port, uint8_t deviceAddress, uint8_t *dataToSend, * * @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. + * @return The number of bytes written (>= 0) or -1 on transfer abort. */ int32_t i2CWrite(uint8_t port, uint8_t deviceAddress, uint8_t* dataToSend, uint8_t sendSize) { @@ -1846,7 +1846,7 @@ int32_t i2CWrite(uint8_t port, uint8_t deviceAddress, uint8_t* dataToSend, uint8 * @param registerAddress The register to read first in the transaction. * @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 Transfer Aborted... false for success, true for aborted. + * @return The number of bytes read (>= 0) or -1 on transfer abort. */ int32_t i2CRead(uint8_t port, uint8_t deviceAddress, uint8_t *buffer, uint8_t count) { diff --git a/wpilibc/Athena/src/I2C.cpp b/wpilibc/Athena/src/I2C.cpp index d70a132a5d..da7828cb12 100644 --- a/wpilibc/Athena/src/I2C.cpp +++ b/wpilibc/Athena/src/I2C.cpp @@ -48,7 +48,7 @@ bool I2C::Transaction(uint8_t *dataToSend, uint8_t sendSize, status = i2CTransaction(m_port, m_deviceAddress, dataToSend, sendSize, dataReceived, receiveSize); // wpi_setErrorWithContext(status, getHALErrorMessage(status)); - return status < 0; + return status < receiveSize; } /** @@ -60,9 +60,7 @@ bool I2C::Transaction(uint8_t *dataToSend, uint8_t sendSize, * @return Transfer Aborted... false for success, true for aborted. */ bool I2C::AddressOnly() { - int32_t status = 0; - status = Transaction(nullptr, 0, nullptr, 0); - return status < 0; + return Transaction(nullptr, 0, nullptr, 0); } /** @@ -82,7 +80,7 @@ bool I2C::Write(uint8_t registerAddress, uint8_t data) { buffer[1] = data; int32_t status = 0; status = i2CWrite(m_port, m_deviceAddress, buffer, sizeof(buffer)); - return status < 0; + return status < static_cast(sizeof(buffer)); } /** @@ -98,7 +96,7 @@ bool I2C::Write(uint8_t registerAddress, uint8_t data) { bool I2C::WriteBulk(uint8_t *data, uint8_t count) { int32_t status = 0; status = i2CWrite(m_port, m_deviceAddress, data, count); - return status < 0; + return status < count; } /** @@ -123,10 +121,7 @@ bool I2C::Read(uint8_t registerAddress, uint8_t count, uint8_t *buffer) { wpi_setWPIErrorWithContext(NullParameter, "buffer"); return true; } - int32_t status = 0; - status = - Transaction(®isterAddress, sizeof(registerAddress), buffer, count); - return status < 0; + return Transaction(®isterAddress, sizeof(registerAddress), buffer, count); } /** @@ -153,7 +148,7 @@ bool I2C::ReadOnly(uint8_t count, uint8_t *buffer) { } int32_t status = 0; status = i2CRead(m_port, m_deviceAddress, buffer, count); - return status < 0; + return status < count; } /** diff --git a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/I2C.java b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/I2C.java index 470904de38..eecafe48ec 100644 --- a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/I2C.java +++ b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/I2C.java @@ -73,7 +73,7 @@ public class I2C extends SensorBase { */ public synchronized boolean transaction(byte[] dataToSend, int sendSize, byte[] dataReceived, int receiveSize) { - boolean aborted = true; + int status = -1; ByteBuffer dataToSendBuffer = ByteBuffer.allocateDirect(sendSize); if (sendSize > 0 && dataToSend != null) { @@ -81,13 +81,13 @@ public class I2C extends SensorBase { } ByteBuffer dataReceivedBuffer = ByteBuffer.allocateDirect(receiveSize); - aborted = + status = I2CJNI.i2CTransaction((byte) m_port.getValue(), (byte) m_deviceAddress, dataToSendBuffer, - (byte) sendSize, dataReceivedBuffer, (byte) receiveSize) != 0; + (byte) sendSize, dataReceivedBuffer, (byte) receiveSize); if (receiveSize > 0 && dataReceived != null) { dataReceivedBuffer.get(dataReceived); } - return aborted; + return status < receiveSize; } /** @@ -117,7 +117,7 @@ public class I2C extends SensorBase { if (dataReceived.capacity() < receiveSize) throw new IllegalArgumentException("dataReceived is too small, must be at least " + receiveSize); - return I2CJNI.i2CTransaction((byte) m_port.getValue(), (byte) m_deviceAddress, dataToSend, (byte) sendSize, dataReceived, (byte) receiveSize) != 0; + return I2CJNI.i2CTransaction((byte) m_port.getValue(), (byte) m_deviceAddress, dataToSend, (byte) sendSize, dataReceived, (byte) receiveSize) < receiveSize; } /** @@ -151,7 +151,7 @@ public class I2C extends SensorBase { dataToSendBuffer.put(buffer); return I2CJNI.i2CWrite((byte) m_port.getValue(), (byte) m_deviceAddress, dataToSendBuffer, - (byte) buffer.length) < 0; + (byte) buffer.length) < buffer.length; } /** @@ -167,7 +167,7 @@ public class I2C extends SensorBase { dataToSendBuffer.put(data); return I2CJNI.i2CWrite((byte) m_port.getValue(), (byte) m_deviceAddress, dataToSendBuffer, - (byte) data.length) < 0; + (byte) data.length) < data.length; } /** @@ -186,7 +186,7 @@ public class I2C extends SensorBase { throw new IllegalArgumentException("buffer is too small, must be at least " + size); return I2CJNI.i2CWrite((byte) m_port.getValue(), (byte) m_deviceAddress, data, - (byte) size) < 0; + (byte) size) < size; } /** @@ -274,7 +274,7 @@ public class I2C extends SensorBase { I2CJNI.i2CRead((byte) m_port.getValue(), (byte) m_deviceAddress, dataReceivedBuffer, (byte) count); dataReceivedBuffer.get(buffer); - return retVal < 0; + return retVal < count; } /** @@ -300,7 +300,7 @@ public class I2C extends SensorBase { throw new IllegalArgumentException("buffer is too small, must be at least " + count); return I2CJNI.i2CRead((byte) m_port.getValue(), (byte) m_deviceAddress, buffer, - (byte) count) < 0; + (byte) count) < count; } /**