GHI SoftwareI2CBus class: unnecessary checks?

Archive
1

I think the following checks that are inside [em]public bool WriteRead(byte address, byte[] writeBuffer, int writeOffset, int writeLength, byte[] readBuffer, int readOffset, int readLength, out int numWritten, out int numRead)[/em] function are not needed and could be safely removed:

if (this.clockPin == Cpu.Pin.GPIO_NONE)
throw new ArgumentException("clockPin cannot be Cpu.Pin.GPIO_NONE.", "clockPin");
if (this.dataPin == Cpu.Pin.GPIO_NONE)
throw new ArgumentException("dataPin cannot be Cpu.Pin.GPIO_NONE.", "dataPin");

The following code ensure that those pins cannot be set to GPIO_NONE:

public Cpu.Pin ClockPin
{
  get
  {
	return this.clockPin;
  }
  set
  {
	if (value == Cpu.Pin.GPIO_NONE)
	  throw new ArgumentException("value cannot be Cpu.Pin.GPIO_NONE.", "value");
	this.clockPin = value;
  }
}

public Cpu.Pin DataPin
{
  get
  {
	return this.dataPin;
  }
  set
  {
	if (value == Cpu.Pin.GPIO_NONE)
	  throw new ArgumentException("value cannot be Cpu.Pin.GPIO_NONE.", "value");
	this.dataPin = value;
  }
}

public SoftwareI2CBus(Cpu.Pin clockPin, Cpu.Pin dataPin)
{
  if (clockPin == Cpu.Pin.GPIO_NONE)
	throw new ArgumentException("clockPin cannot be Cpu.Pin.GPIO_NONE.", "clockPin");
  if (dataPin == Cpu.Pin.GPIO_NONE)
	throw new ArgumentException("dataPin cannot be Cpu.Pin.GPIO_NONE.", "dataPin");
2

@ iamin, I think they specifically need to check when they are are attempting WriteRead. There are cases (I believe) to have the variable be GPIO_NONE, sometimes.

3

I don’t see how [em]WriteRead[/em] can be called while [em]clockPin[/em] or [em]dataPin[/em] are set to GPIO_NONE.

4

@ iamin - You are correct, the exceptions are redundant. They’re removed for the next SDK. Thanks.

5

@ John - Great! I would also suggest you to review other checks (in other functions) as well. For example:

if (writeOffset < 0)
  throw new ArgumentOutOfRangeException("writeOffset", "writeOffset must be non-negative.");
if (writeLength < 0)
  throw new ArgumentOutOfRangeException("writeLength", "writeLength must be non-negative.");
if (writeOffset + writeLength > writeBuffer.Length)
  throw new ArgumentOutOfRangeException("writeBuffer", "writeOffset + writeLength must be no more than writeBuffer.Length.");
if (readBuffer == null)
  throw new ArgumentNullException("readBuffer");
if (readOffset < 0)
  throw new ArgumentOutOfRangeException("readOffset", "readOffset must be non-negative.");
if (readLength < 0)
  throw new ArgumentOutOfRangeException("readLength", "readLength must be non-negative.");
if (readOffset + readLength > readBuffer.Length)
  throw new ArgumentOutOfRangeException("readBuffer", "readOffset + readLength must be no more than readBuffer.Length.");
else
  return this.bus.WriteRead(this.address, writeBuffer, writeOffset, writeLength, readBuffer, readOffset, readLength, out numWritten, out numRead);

The last line calls function that does the same checks again.

6

@ iamin - I’ll take a look and see if there are any more that can be removed.

7

Well, every clock cycle counts :slight_smile: Thanks iamin