Address code review feedback.

ColinA-MSFT · ColinA-MSFT · commit 00c27ad1345a · 2015-06-03T16:38:08.000-07:00
diff --git a/CLR/Libraries/Windows_Devices/windows_devices_native_Windows_Devices_Spi_SpiBusInfo.cpp b/CLR/Libraries/Windows_Devices/windows_devices_native_Windows_Devices_Spi_SpiBusInfo.cpp
@@ -10,7 +10,7 @@ HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiBusInfo::get_ChipS
 
     CLR_RT_HeapBlock* pThis = stack.This(); FAULT_ON_NULL(pThis);
 
-    SetResult_INT32( stack, static_cast<INT32>(CPU_SPI_PortsCount()) );
+	stack.SetResult_I4( static_cast<INT32>(CPU_SPI_PortsCount()) );
 
     TINYCLR_NOCLEANUP();
 }
diff --git a/CLR/Libraries/Windows_Devices/windows_devices_native_Windows_Devices_Spi_SpiDevice.cpp b/CLR/Libraries/Windows_Devices/windows_devices_native_Windows_Devices_Spi_SpiDevice.cpp
@@ -9,10 +9,10 @@ typedef Library_windows_devices_native_Windows_Devices_Spi_SpiConnectionSettings
 // NOTE: Must match enum in SpiEnums.cs
 enum SpiMode
 {
-        SpiMode_Mode0 = 0,
-        SpiMode_Mode1,
-        SpiMode_Mode2,
-        SpiMode_Mode3,
+    SpiMode_Mode0 = 0,
+    SpiMode_Mode1,
+    SpiMode_Mode2,
+    SpiMode_Mode3,
 };
 
 HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiDevice::InitNative___VOID( CLR_RT_StackFrame& stack )
@@ -228,8 +228,19 @@ HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiDevice::TransferIn
         CPU_SPI_Initialize();
 
         // If full duplex, read start offset should be 0, otherwise start at the end of the "write" operation.
-        BOOL readStartOffset = stack.Arg3().NumericByRef().u1 ? 0 : writeLength;
-        if (!CPU_SPI_nWrite8_nRead8( config, writeData, writeLength, readData, readLength, readStartOffset ))
+        int readStartOffset = stack.Arg3().NumericByRef().u1 ? 0 : writeLength;
+        BOOL result = FALSE;
+
+        if (config.MD_16bits)
+        {
+            result = CPU_SPI_nWrite16_nRead16( config, (CLR_UINT16*)writeData, writeLength / 2, (CLR_UINT16*)readData, readLength / 2, readStartOffset / 2 );
+        }
+        else
+        {
+            result = CPU_SPI_nWrite8_nRead8( config, writeData, writeLength, readData, readLength, readStartOffset );
+        }
+
+        if (!result)
         {
             TINYCLR_SET_AND_LEAVE(CLR_E_INVALID_OPERATION);
         }
diff --git a/CLR/Libraries/Windows_Devices/windows_devices_stub.cpp b/CLR/Libraries/Windows_Devices/windows_devices_stub.cpp
@@ -7,83 +7,83 @@
 HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioController::get_PinCount___I4( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::get_DebounceTimeout___mscorlibSystemTimeSpan( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::set_DebounceTimeout___VOID__mscorlibSystemTimeSpan( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::Read___WindowsDevicesGpioGpioPinValue( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::Write___VOID__WindowsDevicesGpioGpioPinValue( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::InitNative___BOOLEAN__I4( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::DisposeNative___VOID( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::SetDriveModeInternal___VOID__WindowsDevicesGpioGpioPinDriveMode( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiBusInfo::get_ChipSelectLineCount___I4( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiDevice::InitNative___VOID( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiDevice::DisposeNative___VOID( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
 
 HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiDevice::TransferInternal___VOID__SZARRAY_U1__SZARRAY_U1__BOOLEAN( CLR_RT_StackFrame& stack )
 {
     TINYCLR_HEADER();
-    TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);
+    TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());
     TINYCLR_NOCLEANUP();
 }
diff --git a/Framework/Core/Windows/Devices/Spi/SpiBusInfo.cs b/Framework/Core/Windows/Devices/Spi/SpiBusInfo.cs
@@ -9,13 +9,6 @@ namespace Windows.Devices.Spi
     /// </summary>
     public sealed class SpiBusInfo
     {
-        /// <summary>
-        /// Initializes a new instance of SpiBusInfo.
-        /// </summary>
-        internal SpiBusInfo()
-        {
-        }
-
         /// <summary>
         /// Gets the number of chip select lines available on the bus.
         /// </summary>
@@ -34,6 +27,7 @@ public int MinClockFrequency
         {
             get
             {
+                // TODO: Issue #143: Implement this in HAL.
                 throw new NotImplementedException();
             }
         }
@@ -46,6 +40,7 @@ public int MaxClockFrequency
         {
             get
             {
+                // TODO: Issue #143: Implement this in HAL.
                 throw new NotImplementedException();
             }
         }
diff --git a/Framework/Core/Windows/Devices/Spi/SpiDevice.cs b/Framework/Core/Windows/Devices/Spi/SpiDevice.cs
@@ -11,7 +11,6 @@ public sealed class SpiDevice : IDisposable
         private readonly string m_deviceId;
         private readonly SpiConnectionSettings m_settings;
 
-        private object m_syncLock = new object();
         private bool m_disposed = false;
         private int m_mskPin = -1;
         private int m_misoPin = -1;
@@ -65,15 +64,12 @@ public string DeviceId
         {
             get
             {
-                lock (m_syncLock)
+                if (m_disposed)
                 {
-                    if (m_disposed)
-                    {
-                        throw new ObjectDisposedException();
-                    }
-
-                    return m_deviceId.Substring(0);
+                    throw new ObjectDisposedException();
                 }
+
+                return m_deviceId.Substring(0);
             }
         }
 
@@ -85,16 +81,13 @@ public SpiConnectionSettings ConnectionSettings
         {
             get
             {
-                lock (m_syncLock)
+                if (m_disposed)
                 {
-                    if (m_disposed)
-                    {
-                        throw new ObjectDisposedException();
-                    }
-
-                    // We must return a copy so the caller can't accidentally mutate our internal settings.
-                    return new SpiConnectionSettings(m_settings);
+                    throw new ObjectDisposedException();
                 }
+
+                // We must return a copy so the caller can't accidentally mutate our internal settings.
+                return new SpiConnectionSettings(m_settings);
             }
         }
 
@@ -237,14 +230,11 @@ public void TransferFullDuplex(byte[] writeBuffer, byte[] readBuffer)
         /// </summary>
         public void Dispose()
         {
-            lock (m_syncLock)
+            if (!m_disposed)
             {
-                if (!m_disposed)
-                {
-                    Dispose(true);
-                    GC.SuppressFinalize(this);
-                    m_disposed = true;
-                }
+                Dispose(true);
+                GC.SuppressFinalize(this);
+                m_disposed = true;
             }
         }
 
@@ -275,10 +265,7 @@ private void Dispose(bool disposing)
         {
             if (disposing)
             {
-                lock (m_syncLock)
-                {
-                    DisposeNative();
-                }
+                DisposeNative();
             }
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiBusInfo::get_ChipS`
`10`	`10`
`11`	`11`	`CLR_RT_HeapBlock* pThis = stack.This(); FAULT_ON_NULL(pThis);`
`12`	`12`
`13`		`- SetResult_INT32( stack, static_cast<INT32>(CPU_SPI_PortsCount()) );`
	`13`	`+ stack.SetResult_I4( static_cast<INT32>(CPU_SPI_PortsCount()) );`
`14`	`14`
`15`	`15`	`TINYCLR_NOCLEANUP();`
`16`	`16`	`}`
Original file line number	Diff line number	Diff line change
`@@ -7,83 +7,83 @@`
`7`	`7`	`HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioController::get_PinCount___I4( CLR_RT_StackFrame& stack )`
`8`	`8`	`{`
`9`	`9`	`TINYCLR_HEADER();`
`10`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`10`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`11`	`11`	`TINYCLR_NOCLEANUP();`
`12`	`12`	`}`
`13`	`13`
`14`	`14`	`HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::get_DebounceTimeout___mscorlibSystemTimeSpan( CLR_RT_StackFrame& stack )`
`15`	`15`	`{`
`16`	`16`	`TINYCLR_HEADER();`
`17`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`17`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`18`	`18`	`TINYCLR_NOCLEANUP();`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::set_DebounceTimeout___VOID__mscorlibSystemTimeSpan( CLR_RT_StackFrame& stack )`
`22`	`22`	`{`
`23`	`23`	`TINYCLR_HEADER();`
`24`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`24`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`25`	`25`	`TINYCLR_NOCLEANUP();`
`26`	`26`	`}`
`27`	`27`
`28`	`28`	`HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::Read___WindowsDevicesGpioGpioPinValue( CLR_RT_StackFrame& stack )`
`29`	`29`	`{`
`30`	`30`	`TINYCLR_HEADER();`
`31`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`31`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`32`	`32`	`TINYCLR_NOCLEANUP();`
`33`	`33`	`}`
`34`	`34`
`35`	`35`	`HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::Write___VOID__WindowsDevicesGpioGpioPinValue( CLR_RT_StackFrame& stack )`
`36`	`36`	`{`
`37`	`37`	`TINYCLR_HEADER();`
`38`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`38`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`39`	`39`	`TINYCLR_NOCLEANUP();`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::InitNative___BOOLEAN__I4( CLR_RT_StackFrame& stack )`
`43`	`43`	`{`
`44`	`44`	`TINYCLR_HEADER();`
`45`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`45`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`46`	`46`	`TINYCLR_NOCLEANUP();`
`47`	`47`	`}`
`48`	`48`
`49`	`49`	`HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::DisposeNative___VOID( CLR_RT_StackFrame& stack )`
`50`	`50`	`{`
`51`	`51`	`TINYCLR_HEADER();`
`52`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`52`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`53`	`53`	`TINYCLR_NOCLEANUP();`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`HRESULT Library_windows_devices_native_Windows_Devices_Gpio_GpioPin::SetDriveModeInternal___VOID__WindowsDevicesGpioGpioPinDriveMode( CLR_RT_StackFrame& stack )`
`57`	`57`	`{`
`58`	`58`	`TINYCLR_HEADER();`
`59`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`59`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`60`	`60`	`TINYCLR_NOCLEANUP();`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiBusInfo::get_ChipSelectLineCount___I4( CLR_RT_StackFrame& stack )`
`64`	`64`	`{`
`65`	`65`	`TINYCLR_HEADER();`
`66`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`66`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`67`	`67`	`TINYCLR_NOCLEANUP();`
`68`	`68`	`}`
`69`	`69`
`70`	`70`	`HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiDevice::InitNative___VOID( CLR_RT_StackFrame& stack )`
`71`	`71`	`{`
`72`	`72`	`TINYCLR_HEADER();`
`73`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`73`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`74`	`74`	`TINYCLR_NOCLEANUP();`
`75`	`75`	`}`
`76`	`76`
`77`	`77`	`HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiDevice::DisposeNative___VOID( CLR_RT_StackFrame& stack )`
`78`	`78`	`{`
`79`	`79`	`TINYCLR_HEADER();`
`80`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`80`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`81`	`81`	`TINYCLR_NOCLEANUP();`
`82`	`82`	`}`
`83`	`83`
`84`	`84`	`HRESULT Library_windows_devices_native_Windows_Devices_Spi_SpiDevice::TransferInternal___VOID__SZARRAY_U1__SZARRAY_U1__BOOLEAN( CLR_RT_StackFrame& stack )`
`85`	`85`	`{`
`86`	`86`	`TINYCLR_HEADER();`
`87`		`- TINYCLR_SET_AND_LEAVE(CLR_E_NOT_SUPPORTED);`
	`87`	`+ TINYCLR_SET_AND_LEAVE(stack.NotImplementedStub());`
`88`	`88`	`TINYCLR_NOCLEANUP();`
`89`	`89`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,13 +9,6 @@ namespace Windows.Devices.Spi`
`9`	`9`	`/// </summary>`
`10`	`10`	`public sealed class SpiBusInfo`
`11`	`11`	`{`
`12`		`- /// <summary>`
`13`		`- /// Initializes a new instance of SpiBusInfo.`
`14`		`- /// </summary>`
`15`		`- internal SpiBusInfo()`
`16`		`- {`
`17`		`- }`
`18`		`-`
`19`	`12`	`/// <summary>`
`20`	`13`	`/// Gets the number of chip select lines available on the bus.`
`21`	`14`	`/// </summary>`
`@@ -34,6 +27,7 @@ public int MinClockFrequency`
`34`	`27`	`{`
`35`	`28`	`get`
`36`	`29`	`{`
	`30`	`+ // TODO: Issue #143: Implement this in HAL.`
`37`	`31`	`throw new NotImplementedException();`
`38`	`32`	`}`
`39`	`33`	`}`
`@@ -46,6 +40,7 @@ public int MaxClockFrequency`
`46`	`40`	`{`
`47`	`41`	`get`
`48`	`42`	`{`
	`43`	`+ // TODO: Issue #143: Implement this in HAL.`
`49`	`44`	`throw new NotImplementedException();`
`50`	`45`	`}`
`51`	`46`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,6 @@ public sealed class SpiDevice : IDisposable`
`11`	`11`	`private readonly string m_deviceId;`
`12`	`12`	`private readonly SpiConnectionSettings m_settings;`
`13`	`13`
`14`		`- private object m_syncLock = new object();`
`15`	`14`	`private bool m_disposed = false;`
`16`	`15`	`private int m_mskPin = -1;`
`17`	`16`	`private int m_misoPin = -1;`
`@@ -65,15 +64,12 @@ public string DeviceId`
`65`	`64`	`{`
`66`	`65`	`get`
`67`	`66`	`{`
`68`		`- lock (m_syncLock)`
	`67`	`+ if (m_disposed)`
`69`	`68`	`{`
`70`		`- if (m_disposed)`
`71`		`- {`
`72`		`- throw new ObjectDisposedException();`
`73`		`- }`
`74`		`-`
`75`		`- return m_deviceId.Substring(0);`
	`69`	`+ throw new ObjectDisposedException();`
`76`	`70`	`}`
	`71`	`+`
	`72`	`+ return m_deviceId.Substring(0);`
`77`	`73`	`}`
`78`	`74`	`}`
`79`	`75`
`@@ -85,16 +81,13 @@ public SpiConnectionSettings ConnectionSettings`
`85`	`81`	`{`
`86`	`82`	`get`
`87`	`83`	`{`
`88`		`- lock (m_syncLock)`
	`84`	`+ if (m_disposed)`
`89`	`85`	`{`
`90`		`- if (m_disposed)`
`91`		`- {`
`92`		`- throw new ObjectDisposedException();`
`93`		`- }`
`94`		`-`
`95`		`- // We must return a copy so the caller can't accidentally mutate our internal settings.`
`96`		`- return new SpiConnectionSettings(m_settings);`
	`86`	`+ throw new ObjectDisposedException();`
`97`	`87`	`}`
	`88`	`+`
	`89`	`+ // We must return a copy so the caller can't accidentally mutate our internal settings.`
	`90`	`+ return new SpiConnectionSettings(m_settings);`
`98`	`91`	`}`
`99`	`92`	`}`
`100`	`93`
`@@ -237,14 +230,11 @@ public void TransferFullDuplex(byte[] writeBuffer, byte[] readBuffer)`
`237`	`230`	`/// </summary>`
`238`	`231`	`public void Dispose()`
`239`	`232`	`{`
`240`		`- lock (m_syncLock)`
	`233`	`+ if (!m_disposed)`
`241`	`234`	`{`
`242`		`- if (!m_disposed)`
`243`		`- {`
`244`		`- Dispose(true);`
`245`		`- GC.SuppressFinalize(this);`
`246`		`- m_disposed = true;`
`247`		`- }`
	`235`	`+ Dispose(true);`
	`236`	`+ GC.SuppressFinalize(this);`
	`237`	`+ m_disposed = true;`
`248`	`238`	`}`
`249`	`239`	`}`
`250`	`240`
`@@ -275,10 +265,7 @@ private void Dispose(bool disposing)`
`275`	`265`	`{`
`276`	`266`	`if (disposing)`
`277`	`267`	`{`
`278`		`- lock (m_syncLock)`
`279`		`- {`
`280`		`- DisposeNative();`
`281`		`- }`
	`268`	`+ DisposeNative();`
`282`	`269`	`}`
`283`	`270`	`}`
`284`	`271`	`}`