Merge pull request #562 from adafruit/followup-496

clean up pwn and analog

Author: hathach

.travis.yml.bck

@@ -54,10 +54,10 @@ HardwarePWM* HwPWMx[] =
 };
 
 #if CFG_DEBUG
-bool can_stringify_token(uintptr_t token)
+bool can_stringify_token(uint32_t token)
 {
   uint8_t * t = (uint8_t *)&token;
-  for (size_t i = 0; i < sizeof(uintptr_t); ++i, ++t)
+  for (size_t i = 0; i < sizeof(uint32_t); ++i, ++t)
   {
     uint8_t x = *t;
     if ((x < 0x20) || (x > 0x7E)) return false;
@@ -71,14 +71,12 @@ void HardwarePWM::DebugOutput(Stream& logger)
   logger.printf("HwPWM Debug:");
   for (size_t i = 0; i < count; i++) {
     HardwarePWM const * pwm = HwPWMx[i];
-    uintptr_t token = pwm->_owner_token;
+    uint32_t token = pwm->_owner_token;
     logger.printf(" || %d:", i);
     if (can_stringify_token(token)) {
       uint8_t * t = (uint8_t*)(&token);
-      static_assert(sizeof(uintptr_t) == 4);
       logger.printf("   \"%c%c%c%c\"", t[0], t[1], t[2], t[3] );
     } else {
-      static_assert(sizeof(uintptr_t) == 4);
       logger.printf(" %08x", token);
     }
     for (size_t j = 0; j < MAX_CHANNELS; j++) {
@@ -96,76 +94,6 @@ void HardwarePWM::DebugOutput(Stream& logger)
 void HardwarePWM::DebugOutput(Stream& logger) {}
 #endif // CFG_DEBUG
 
-// returns true ONLY when (1) no PWM channel has a pin, and (2) the owner token is nullptr
-bool HardwarePWM::takeOwnership(uintptr_t token)
-{
-  bool notInIsr = !isInISR();
-  if (token == 0) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "zero / nullptr is not a valid ownership token (attempted use in takeOwnership)");
-    }
-    return false; // cannot take ownership with nullptr
-  }
-  if (token == this->_owner_token) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "failing to acquire ownership because already owned by requesting token (cannot take ownership twice)");
-    }
-  }
-  if (this->_owner_token != 0) {
-    return false;
-  }
-  if (this->usedChannelCount() != 0) {
-    return false;
-  }
-  if (this->enabled()) {
-    return false;
-  }
-  // TODO: warn, but do not fail, if taking ownership with IRQs already enabled
-  // NVIC_GetActive
-
-  // Use C++11 atomic CAS operation
-  uintptr_t newValue = 0U;
-  return this->_owner_token.compare_exchange_strong(newValue, token);
-}
-// returns true ONLY when (1) no PWM channel has a pin attached, and (2) the owner token matches
-bool HardwarePWM::releaseOwnership(uintptr_t token)
-{
-  bool notInIsr = !isInISR();
-  if (token == 0) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "zero / nullptr is not a valid ownership token (attempted use in releaseOwnership)");
-    }
-    return false;
-  }
-  if (!this->isOwner(token)) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "attempt to release ownership when not the current owner");
-    }
-    return false;
-  }
-  if (this->usedChannelCount() != 0) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "attempt to release ownership when at least on channel is still connected");
-    }
-    return false;
-  }
-  if (this->enabled()) {
-    if (notInIsr) {
-      LOG_LV1("HwPWM", "attempt to release ownership when PWM peripheral is still enabled");
-    }
-    return false; // if it's enabled, do not allow ownership to be released, even with no pins in use
-  }
-  // TODO: warn, but do not fail, if releasing ownership with IRQs enabled
-  // NVIC_GetActive
-
-  // Use C++11 atomic CAS operation
-  bool result = this->_owner_token.compare_exchange_strong(token, 0U);
-  if (!result) {
-    LOG_LV1("HwPWM", "race condition resulted in failure to acquire ownership");
-  }
-  return result;
-}
-
 HardwarePWM::HardwarePWM(NRF_PWM_Type* pwm) :
   _pwm(pwm)
 {
@@ -179,6 +107,38 @@ HardwarePWM::HardwarePWM(NRF_PWM_Type* pwm) :
   _pwm->PSEL.OUT[1] = 0xFFFFFFFFUL;
 }
 
+void HardwarePWM::begin(void)
+{
+  // Initialize Registers
+  _pwm->MODE            = PWM_MODE_UPDOWN_Up;
+  _pwm->COUNTERTOP      = _max_value; // default is 255 (8 bit), can be configured before begin()
+  _pwm->PRESCALER       = _clock_div;
+  _pwm->DECODER         = PWM_DECODER_LOAD_Individual;
+  _pwm->LOOP            = 0;
+
+  _pwm->SEQ[0].PTR      = (uint32_t) _seq0;
+  _pwm->SEQ[0].CNT      = MAX_CHANNELS; // default mode is Individual --> count must be 4
+  _pwm->SEQ[0].REFRESH  = 0;
+  _pwm->SEQ[0].ENDDELAY = 0;
+
+  _pwm->SEQ[1].PTR      = 0;
+  _pwm->SEQ[1].CNT      = 0;
+  _pwm->SEQ[1].REFRESH  = 0;
+  _pwm->SEQ[1].ENDDELAY = 0;
+
+  _pwm->ENABLE = 1;
+}
+
+void HardwarePWM::stop(void)
+{
+  _pwm->ENABLE = 0;
+}
+
+bool HardwarePWM::enabled (void)
+{
+  return _pwm->ENABLE;
+}
+
 void HardwarePWM::setResolution(uint8_t bitnum)
 {
   setMaxValue( bit(min8(bitnum, 15)) -1 );
@@ -196,6 +156,25 @@ void HardwarePWM::setClockDiv(uint8_t div)
   _pwm->PRESCALER = _clock_div;
 }
 
+void HardwarePWM::_set_psel(int ch, uint32_t value)
+{
+  // Must disable before changing PSEL
+  if ( enabled() )
+  {
+    _pwm->ENABLE = 0;
+    _pwm->PSEL.OUT[ch] = value;
+    _seq0[ch] = 0;
+    _pwm->ENABLE = 1;
+
+    // re-start sequence
+    if ( usedChannelCount() ) _pwm->TASKS_SEQSTART[0] = 1;
+  }else
+  {
+    _pwm->PSEL.OUT[ch] = value;
+    _seq0[ch] = 0;
+  }
+}
+
 /**
  * Add pin to this group.
  * @param pin Pin to add
@@ -222,17 +201,7 @@ bool HardwarePWM::addPin(uint8_t pin)
   pinMode(pin, OUTPUT);
   digitalWrite(pin, LOW);
 
-  // Must disable before changing PSEL
-  if ( enabled() )
-  {
-    _pwm->ENABLE = 0;
-    _pwm->PSEL.OUT[ch] = g_ADigitalPinMap[pin];
-    _pwm->ENABLE = 1;
-    _start();
-  }else
-  {
-    _pwm->PSEL.OUT[ch] = g_ADigitalPinMap[pin];
-  }
+  _set_psel(ch, g_ADigitalPinMap[pin]);
 
   return true;
 }
@@ -242,70 +211,21 @@ bool HardwarePWM::removePin(uint8_t pin)
   int ch = pin2channel(pin);
   VERIFY( ch >= 0 );
 
-  bool const en = enabled();
-
-  // Must disable before changing PSEL
-  if ( en ) _pwm->ENABLE = 0;
-
-  _pwm->PSEL.OUT[ch] = 0xFFFFFFFFUL;
-  _seq0[ch] = 0;
-
-  if ( en ) _pwm->ENABLE = 1;
-
+  _set_psel(ch, 0xFFFFFFFFUL);
   return true;
 }
 
-bool HardwarePWM::enabled (void)
-{
-  return _pwm->ENABLE;
-}
-
-void HardwarePWM::begin(void)
-{
-  // Initialize Registers
-  _pwm->MODE            = PWM_MODE_UPDOWN_Up;
-  _pwm->COUNTERTOP      = _max_value; // default is 255 (8 bit), can be configured before begin()
-  _pwm->PRESCALER       = _clock_div;
-  _pwm->DECODER         = PWM_DECODER_LOAD_Individual;
-  _pwm->LOOP            = 0;
-
-  _pwm->SEQ[0].PTR      = (uint32_t) _seq0;
-  _pwm->SEQ[0].CNT      = MAX_CHANNELS; // default mode is Individual --> count must be 4
-  _pwm->SEQ[0].REFRESH  = 0;
-  _pwm->SEQ[0].ENDDELAY = 0;
-
-  _pwm->SEQ[1].PTR      = 0;
-  _pwm->SEQ[1].CNT      = 0;
-  _pwm->SEQ[1].REFRESH  = 0;
-  _pwm->SEQ[1].ENDDELAY = 0;
-
-  _pwm->ENABLE = 1;
-}
-
-void HardwarePWM::_start(void)
-{
-  // update sequence count (depending on mode)
-  //  _pwm->SEQ[0].CNT = MAX_CHANNELS;
-
-  // start sequence
-  _pwm->TASKS_SEQSTART[0] = 1;
-}
-
-void HardwarePWM::stop(void)
-{
-  _pwm->ENABLE = 0;
-}
-
-bool HardwarePWM::writeChannel(uint8_t ch, uint16_t value, bool inverted )
+bool HardwarePWM::writeChannel(uint8_t ch, uint16_t value, bool inverted)
 {
   VERIFY( ch < MAX_CHANNELS );
 
   _seq0[ch] = value | (inverted ? 0 : bit(15));
 
-  // Start PWM if not already
+  // Initialize PWM if not already
   if ( !enabled() ) begin();
 
-  _start();
+  // start sequence
+  _pwm->TASKS_SEQSTART[0] = 1;
 
   return true;
 }
@@ -350,3 +270,55 @@ uint8_t HardwarePWM::freeChannelCount(void) const
   return MAX_CHANNELS - usedChannelCount();
 }
 
+// returns true ONLY when (1) no PWM channel has a pin, and (2) the owner token is nullptr
+bool HardwarePWM::takeOwnership(uint32_t token)
+{
+  bool const thread_mode = !isInISR();
+
+  if (token == 0) {
+    if (thread_mode) LOG_LV1("HwPWM", "zero is not a valid ownership token (attempted use in takeOwnership)");
+    return false;
+  }
+
+  if (token == this->_owner_token) {
+    if (thread_mode) LOG_LV1("HwPWM", "failing to acquire ownership because already owned by requesting token (cannot take ownership twice)");
+    return false;
+  }
+
+  if ( this->_owner_token != 0 ) return false;
+  if ( this->usedChannelCount() != 0 ) return false;
+  if ( this->enabled() ) return false;
+
+  // Use C++11 atomic CAS operation
+  uint32_t expectedValue = 0U;
+  return this->_owner_token.compare_exchange_strong(expectedValue, token);
+}
+
+// returns true ONLY when (1) no PWM channel has a pin attached, and (2) the owner token matches
+bool HardwarePWM::releaseOwnership(uint32_t token)
+{
+  bool const thread_mode = !isInISR();
+
+  if (token == 0) {
+    if (thread_mode) LOG_LV1("HwPWM", "zero is not a valid ownership token (attempted use in releaseOwnership)");
+    return false;
+  }
+
+  if (!this->isOwner(token)) {
+    if (thread_mode) LOG_LV1("HwPWM", "attempt to release ownership when not the current owner");
+    return false;
+  }
+
+  if (this->usedChannelCount() != 0) {
+    if (thread_mode) LOG_LV1("HwPWM", "attempt to release ownership when at least on channel is still connected");
+    return false;
+  }
+
+  if (this->enabled()) {
+    if (thread_mode) LOG_LV1("HwPWM", "attempt to release ownership when PWM peripheral is still enabled");
+    return false; // if it's enabled, do not allow ownership to be released, even with no pins in use
+  }
+
+  // Use C++11 atomic CAS operation
+  return this->_owner_token.compare_exchange_strong(token, 0U);
+}