digitalRead()/analogRead() interference (#1006)

* Save PinMode in STM32_PIN_MAP.user_property (HAL_GPIO_Save_Pin_Mode/HAL_GPIO_Recall_Pin_Mode) * adds https://github.com/spark/firmware/issues/993 to changelog
particle-iot · Jun 10, 2016 · 79995f3 · 79995f3
1 parent 9919536
commit 79995f3
Show file tree

Hide file tree

Showing 10 changed files with 132 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,7 +20,7 @@
 - Fixes to I2C Slave mode implementation with clock stretching enabled [#931](https://github.com/spark/firmware/pull/931)
 - `millis()`/`micros()` are now atomic to ensure monotonic values. Fixes [#916](https://github.com/spark/firmware/issues/916) and [#925](https://github.com/spark/firmware/issues/925)
 - availableForWrite() was reporting bytes available instead of bytes available for write [#1020](https://github.com/spark/firmware/pull/1020) and [#1017](https://github.com/spark/firmware/issues/1017)
-
+- `digitalRead()` interferes with `analogRead()` [#993](https://github.com/spark/firmware/issues/993)
 
 
 

diff --git a/hal/src/core/adc_hal.c b/hal/src/core/adc_hal.c
@@ -86,9 +86,9 @@ int32_t HAL_ADC_Read(uint16_t pin)
 
   int i = 0;
 
-  if (adcChannelConfigured != PIN_MAP[pin].adc_channel)
+  if (PIN_MAP[pin].pin_mode != AN_INPUT)
   {
-    HAL_GPIO_Save_Pin_Mode(PIN_MAP[pin].pin_mode);
+    HAL_GPIO_Save_Pin_Mode(pin);
     HAL_Pin_Mode(pin, AN_INPUT);
   }
 

diff --git a/hal/src/core/gpio_hal.c b/hal/src/core/gpio_hal.c
@@ -38,8 +38,6 @@
 
 /* Private variables --------------------------------------------------------*/
 
-PinMode digitalPinModeSaved = PIN_MODE_NONE;
-
 /* Extern variables ---------------------------------------------------------*/
 
 /* Private function prototypes ----------------------------------------------*/
@@ -140,17 +138,47 @@ void HAL_Pin_Mode(pin_t pin, PinMode setMode)
 /*
  * @brief Saves a pin mode to be recalled later.
  */
-void HAL_GPIO_Save_Pin_Mode(PinMode mode)
+void HAL_GPIO_Save_Pin_Mode(uint16_t pin)
 {
-  digitalPinModeSaved = mode;
+  // Save pin mode in STM32_Pin_Info.user_property
+  STM32_Pin_Info* PIN_MAP = HAL_Pin_Map();
+  uint32_t uprop = (uint32_t)PIN_MAP[pin].user_property;
+  uprop = (uprop & 0xFFFF) | (((uint32_t)PIN_MAP[pin].pin_mode & 0xFF) << 16) | (0xAA << 24);
+  PIN_MAP[pin].user_property = (int32_t)uprop;
 }
 
 /*
  * @brief Recalls a saved pin mode.
  */
-PinMode HAL_GPIO_Recall_Pin_Mode()
+PinMode HAL_GPIO_Recall_Pin_Mode(uint16_t pin)
 {
-  return digitalPinModeSaved;
+  // Recall pin mode in STM32_Pin_Info.user_property
+  STM32_Pin_Info* PIN_MAP = HAL_Pin_Map();
+  uint32_t uprop = (uint32_t)PIN_MAP[pin].user_property;
+  if ((uprop & 0xFF000000) != 0xAA000000)
+      return PIN_MODE_NONE;
+
+  PinMode pm = (PinMode)((uprop & 0x00FF0000) >> 16);
+
+  // Safety check
+  switch(pm)
+  {
+      case INPUT:
+      case OUTPUT:
+      case INPUT_PULLUP:
+      case INPUT_PULLDOWN:
+      case AF_OUTPUT_PUSHPULL:
+      case AF_OUTPUT_DRAIN:
+      case AN_INPUT:
+      case AN_OUTPUT:
+      break;
+
+      default:
+      pm = PIN_MODE_NONE;
+      break;
+  }
+
+  return pm;
 }
 
 /*
@@ -181,7 +209,7 @@ int32_t HAL_GPIO_Read(uint16_t pin)
 {
   if(PIN_MAP[pin].pin_mode == AN_INPUT)
   {
-    PinMode pm = HAL_GPIO_Recall_Pin_Mode();
+    PinMode pm = HAL_GPIO_Recall_Pin_Mode(pin);
     if(pm == PIN_MODE_NONE)
     {
       return 0;

diff --git a/hal/src/core/pinmap_impl.h b/hal/src/core/pinmap_impl.h
@@ -50,8 +50,8 @@ typedef struct STM32_Pin_Info {
 extern STM32_Pin_Info PIN_MAP[];
 STM32_Pin_Info* HAL_Pin_Map(void);
 
-extern void HAL_GPIO_Save_Pin_Mode(PinMode mode);
-extern PinMode HAL_GPIO_Recall_Pin_Mode();
+extern void HAL_GPIO_Save_Pin_Mode(uint16_t pin);
+extern PinMode HAL_GPIO_Recall_Pin_Mode(uint16_t pin);
 
 #define NONE ((uint8_t)0xFF)
 #define ADC_CHANNEL_NONE NONE

diff --git a/hal/src/stm32f2xx/adc_hal.c b/hal/src/stm32f2xx/adc_hal.c
@@ -87,9 +87,9 @@ int32_t HAL_ADC_Read(uint16_t pin)
     int i = 0;
     STM32_Pin_Info* PIN_MAP = HAL_Pin_Map();
 
-    if (adcChannelConfigured != PIN_MAP[pin].adc_channel)
+    if (PIN_MAP[pin].pin_mode != AN_INPUT)
     {
-        HAL_GPIO_Save_Pin_Mode(PIN_MAP[pin].pin_mode);
+        HAL_GPIO_Save_Pin_Mode(pin);
         HAL_Pin_Mode(pin, AN_INPUT);
     }
 

diff --git a/hal/src/stm32f2xx/dac_hal.c b/hal/src/stm32f2xx/dac_hal.c
@@ -82,6 +82,7 @@ void HAL_DAC_Write(pin_t pin, uint16_t value)
 
     if (HAL_Get_Pin_Mode(pin) != AN_OUTPUT)
     {
+        HAL_GPIO_Save_Pin_Mode(pin);
         HAL_Pin_Mode(pin, AN_OUTPUT);
         HAL_DAC_Enable(pin, 1);
     }

diff --git a/hal/src/stm32f2xx/gpio_hal.c b/hal/src/stm32f2xx/gpio_hal.c
@@ -39,8 +39,6 @@
 
 /* Private variables --------------------------------------------------------*/
 
-PinMode digitalPinModeSaved = PIN_MODE_NONE;
-
 /* Extern variables ---------------------------------------------------------*/
 
 /* Private function prototypes ----------------------------------------------*/
@@ -171,17 +169,46 @@ void HAL_Pin_Mode(pin_t pin, PinMode setMode)
 /*
  * @brief Saves a pin mode to be recalled later.
  */
-void HAL_GPIO_Save_Pin_Mode(PinMode mode)
+void HAL_GPIO_Save_Pin_Mode(uint16_t pin)
 {
-    digitalPinModeSaved = mode;
+    // Save pin mode in STM32_Pin_Info.user_property
+    STM32_Pin_Info* PIN_MAP = HAL_Pin_Map();
+    uint32_t uprop = (uint32_t)PIN_MAP[pin].user_property;
+    uprop = (uprop & 0xFFFF) | (((uint32_t)PIN_MAP[pin].pin_mode & 0xFF) << 16) | (0xAA << 24);
+    PIN_MAP[pin].user_property = (int32_t)uprop;
 }
 
 /*
  * @brief Recalls a saved pin mode.
  */
-PinMode HAL_GPIO_Recall_Pin_Mode()
+PinMode HAL_GPIO_Recall_Pin_Mode(uint16_t pin)
 {
-    return digitalPinModeSaved;
+    // Recall pin mode in STM32_Pin_Info.user_property
+    STM32_Pin_Info* PIN_MAP = HAL_Pin_Map();
+    uint32_t uprop = (uint32_t)PIN_MAP[pin].user_property;
+    if ((uprop & 0xFF000000) != 0xAA000000)
+        return PIN_MODE_NONE;
+    PinMode pm = (PinMode)((uprop & 0x00FF0000) >> 16);
+
+    // Safety check
+    switch(pm)
+    {
+        case INPUT:
+        case OUTPUT:
+        case INPUT_PULLUP:
+        case INPUT_PULLDOWN:
+        case AF_OUTPUT_PUSHPULL:
+        case AF_OUTPUT_DRAIN:
+        case AN_INPUT:
+        case AN_OUTPUT:
+        break;
+
+        default:
+        pm = PIN_MODE_NONE;
+        break;
+    }
+
+    return pm;
 }
 
 /*
@@ -220,7 +247,7 @@ int32_t HAL_GPIO_Read(uint16_t pin)
     STM32_Pin_Info* PIN_MAP = HAL_Pin_Map();
     if(PIN_MAP[pin].pin_mode == AN_INPUT)
     {
-        PinMode pm = HAL_GPIO_Recall_Pin_Mode();
+        PinMode pm = HAL_GPIO_Recall_Pin_Mode(pin);
         if(pm == PIN_MODE_NONE)
         {
             return 0;
@@ -233,7 +260,7 @@ int32_t HAL_GPIO_Read(uint16_t pin)
     }
     else if (PIN_MAP[pin].pin_mode == AN_OUTPUT)
     {
-        PinMode pm = HAL_GPIO_Recall_Pin_Mode();
+        PinMode pm = HAL_GPIO_Recall_Pin_Mode(pin);
         if(pm == PIN_MODE_NONE)
         {
             return 0;

diff --git a/hal/src/stm32f2xx/pinmap_impl.h b/hal/src/stm32f2xx/pinmap_impl.h
@@ -49,8 +49,8 @@ typedef struct STM32_Pin_Info {
 
 STM32_Pin_Info* HAL_Pin_Map(void);
 
-extern void HAL_GPIO_Save_Pin_Mode(PinMode mode);
-extern PinMode HAL_GPIO_Recall_Pin_Mode();
+extern void HAL_GPIO_Save_Pin_Mode(uint16_t pin);
+extern PinMode HAL_GPIO_Recall_Pin_Mode(uint16_t pin);
 
 #define CHANNEL_NONE ((uint8_t)(0xFF))
 #define ADC_CHANNEL_NONE CHANNEL_NONE

diff --git a/user/tests/wiring/adc_dac/adc_dac.cpp b/user/tests/wiring/adc_dac/adc_dac.cpp
@@ -164,4 +164,31 @@ test(ADC_AnalogReadOnPinWithDACMixedWithDigitalWrite) {
     }
     assertTrue(errorCount == 0);
 }
+
+test(DAC_AnalogWriteWorksMixedWithDigitalRead) {
+    pin_t pin = DAC;
+
+    // when
+    pinMode(pin, INPUT_PULLUP);
+    // then
+    assertEqual(HAL_Get_Pin_Mode(pin), INPUT_PULLUP);
+
+    // 2 analogReads
+    analogWrite(pin, 1000);
+    assertEqual(HAL_Get_Pin_Mode(pin), AN_OUTPUT);
+    analogWrite(pin, 2000);
+    assertEqual(HAL_Get_Pin_Mode(pin), AN_OUTPUT);
+    // 2 digitalReads
+    digitalRead(pin);
+    assertEqual(HAL_Get_Pin_Mode(pin), INPUT_PULLUP);
+    digitalRead(pin);
+    assertEqual(HAL_Get_Pin_Mode(pin), INPUT_PULLUP);
+    // 2 analogReads again
+    analogWrite(pin, 1000);
+    assertEqual(HAL_Get_Pin_Mode(pin), AN_OUTPUT);
+    analogWrite(pin, 500);
+    assertEqual(HAL_Get_Pin_Mode(pin), AN_OUTPUT);
+}
+
+
 #endif
diff --git a/user/tests/wiring/no_fixture/gpio.cpp b/user/tests/wiring/no_fixture/gpio.cpp
@@ -143,3 +143,28 @@ test(GPIO_pulseIn_TimesOutAfter3Seconds) {
     assertMoreOrEqual(millis()-startTime, 2850);
     assertLessOrEqual(millis()-startTime, 3150);
 }
+
+test(GPIO_AnalogReadWorksMixedWithDigitalRead) {
+    pin_t pin = A0;
+
+    // when
+    pinMode(pin, INPUT_PULLUP);
+    // then
+    assertEqual(HAL_Get_Pin_Mode(pin), INPUT_PULLUP);
+
+    // 2 analogReads
+    analogRead(pin);
+    assertEqual(HAL_Get_Pin_Mode(pin), AN_INPUT);
+    analogRead(pin);
+    assertEqual(HAL_Get_Pin_Mode(pin), AN_INPUT);
+    // 2 digitalReads
+    digitalRead(pin);
+    assertEqual(HAL_Get_Pin_Mode(pin), INPUT_PULLUP);
+    digitalRead(pin);
+    assertEqual(HAL_Get_Pin_Mode(pin), INPUT_PULLUP);
+    // 2 analogReads again
+    analogRead(pin);
+    assertEqual(HAL_Get_Pin_Mode(pin), AN_INPUT);
+    analogRead(pin);
+    assertEqual(HAL_Get_Pin_Mode(pin), AN_INPUT);
+}