[mac] simplify scan implementation (#2420)

This commit contains the following changes: It simplifies the MAC
scan implementation by adding common helper methods such as
`UpdateScanChannel()`. It also fixes an issue where invoking back-to-
back scan could cause the handler to be overwritten with NULL. It adds
support for special case of Energy Scan with zero scan duration to
collect a single RSSI sample per channel.

src/core/mac/mac.cpp

@@ -133,63 +133,45 @@ Mac::Mac(Instance &aInstance):
 
 otError Mac::ActiveScan(uint32_t aScanChannels, uint16_t aScanDuration, ActiveScanHandler aHandler, void *aContext)
 {
-    otError error;
+    otError error = OT_ERROR_NONE;
+
+    VerifyOrExit(!IsActiveScanInProgress() && !IsEnergyScanInProgress(), error = OT_ERROR_BUSY);
 
     mActiveScanHandler = aHandler;
-    SuccessOrExit(error = Scan(kOperationActiveScan, aScanChannels, aScanDuration, aContext));
 
-exit:
+    if (aScanDuration == 0)
-
-    if (OT_ERROR_NONE != error)
     {
-        mActiveScanHandler = NULL;
+        aScanDuration = kScanDurationDefault;
     }
 
+    Scan(kOperationActiveScan, aScanChannels, aScanDuration, aContext);
+
+exit:
     return error;
 }
 
 otError Mac::EnergyScan(uint32_t aScanChannels, uint16_t aScanDuration, EnergyScanHandler aHandler, void *aContext)
-{
-    otError error;
-
-    mEnergyScanHandler = aHandler;
-    SuccessOrExit(error = Scan(kOperationEnergyScan, aScanChannels, aScanDuration, aContext));
-
-exit:
-
-    if (OT_ERROR_NONE != error)
-    {
-        mEnergyScanHandler = NULL;
-    }
-
-    return error;
-}
-
-otError Mac::Scan(Operation aScanOperation, uint32_t aScanChannels, uint16_t aScanDuration, void *aContext)
 {
     otError error = OT_ERROR_NONE;
 
     VerifyOrExit(!IsActiveScanInProgress() && !IsEnergyScanInProgress(), error = OT_ERROR_BUSY);
 
-    mScanContext = aContext;
+    mEnergyScanHandler = aHandler;
-    mScanChannels = (aScanChannels == 0) ? static_cast<uint32_t>(kScanChannelsAll) : aScanChannels;
+    Scan(kOperationEnergyScan, aScanChannels, aScanDuration, aContext);
-    mScanDuration = (aScanDuration == 0) ? static_cast<uint16_t>(kScanDurationDefault) : aScanDuration;
-
-    mScanChannel = OT_RADIO_CHANNEL_MIN;
-    mScanChannels >>= OT_RADIO_CHANNEL_MIN;
-
-    while ((mScanChannels & 1) == 0)
-    {
-        mScanChannels >>= 1;
-        mScanChannel++;
-    }
-
-    StartOperation(aScanOperation);
 
 exit:
     return error;
 }
 
+void Mac::Scan(Operation aScanOperation, uint32_t aScanChannels, uint16_t aScanDuration, void *aContext)
+{
+    mScanContext = aContext;
+    mScanDuration = aScanDuration;
+    mScanChannels = (aScanChannels == 0) ? static_cast<uint32_t>(kScanChannelsAll) : aScanChannels;
+    mScanChannel = OT_RADIO_CHANNEL_MIN - 1;
+    StartOperation(aScanOperation);
+}
+
 bool Mac::IsActiveScanInProgress(void)
 {
     return (mOperation == kOperationActiveScan) || (mPendingActiveScan);
@@ -250,26 +232,66 @@ exit:
     return error;
 }
 
-void Mac::StartEnergyScan(void)
+otError Mac::UpdateScanChannel(void)
 {
-    if (!(otPlatRadioGetCaps(&GetInstance()) & OT_RADIO_CAPS_ENERGY_SCAN))
+    otError error = OT_ERROR_NONE;
+
+    do
     {
-        mEnergyScanCurrentMaxRssi = kInvalidRssiValue;
+        mScanChannel++;
-        mMacTimer.Start(mScanDuration);
+        VerifyOrExit(mScanChannel <= OT_RADIO_CHANNEL_MAX, error = OT_ERROR_NOT_FOUND);
-        mEnergyScanSampleRssiTask.Post();
+
-        RadioReceive(mScanChannel);
+    }
+    while ((mScanChannels & (1U << mScanChannel)) == 0);
+
+exit:
+    return error;
+}
+
+void Mac::PerformActiveScan(void)
+{
+    if (UpdateScanChannel() == OT_ERROR_NONE)
+    {
+        // If there are more channels to scan, start CSMA backoff to send the beacon request.
+        StartCsmaBackoff();
     }
     else
     {
-        otError error = otPlatRadioEnergyScan(&GetInstance(), mScanChannel, mScanDuration);
+        otPlatRadioSetPanId(&GetInstance(), mPanId);
+        mActiveScanHandler(mScanContext, NULL);
+        FinishOperation();
+    }
+}
 
-        if (error != OT_ERROR_NONE)
+void Mac::PerformEnergyScan(void)
+{
+    otError error = OT_ERROR_NONE;
+
+    SuccessOrExit(error = UpdateScanChannel());
+
+    if (!(otPlatRadioGetCaps(&GetInstance()) & OT_RADIO_CAPS_ENERGY_SCAN) || (mScanDuration == 0))
+    {
+        RadioReceive(mScanChannel);
+        mEnergyScanCurrentMaxRssi = kInvalidRssiValue;
+        mEnergyScanSampleRssiTask.Post();
+
+        if (mScanDuration != 0)
         {
-            // Cancel scan
+            mMacTimer.Start(mScanDuration);
-            mEnergyScanHandler(mScanContext, NULL);
-            FinishOperation();
         }
     }
+    else
+    {
+        SuccessOrExit(error = otPlatRadioEnergyScan(&GetInstance(), mScanChannel, mScanDuration));
+    }
+
+exit:
+
+    if (error != OT_ERROR_NONE)
+    {
+        mEnergyScanHandler(mScanContext, NULL);
+        FinishOperation();
+    }
 }
 
 extern "C" void otPlatRadioEnergyScanDone(otInstance *aInstance, int8_t aEnergyScanMaxRssi)
@@ -294,40 +316,23 @@ exit:
     return;
 }
 
-void Mac::EnergyScanDone(int8_t aEnergyScanMaxRssi)
+void Mac::ReportEnergyScanResult(int8_t aRssi)
 {
-    // Trigger a energy scan handler callback if necessary
+    if (aRssi != kInvalidRssiValue)
-    if (aEnergyScanMaxRssi != kInvalidRssiValue)
     {
         otEnergyScanResult result;
 
         result.mChannel = mScanChannel;
-        result.mMaxRssi = aEnergyScanMaxRssi;
+        result.mMaxRssi = aRssi;
+
         mEnergyScanHandler(mScanContext, &result);
     }
+}
 
-    // Update to the next scan channel
+void Mac::EnergyScanDone(int8_t aRssi)
-    do
+{
-    {
+    ReportEnergyScanResult(aRssi);
-        mScanChannels >>= 1;
+    PerformEnergyScan();
-        mScanChannel++;
-
-        // If we have scanned all the channels, then fire the final callback
-        // and finish scan operation.
-        if (mScanChannels == 0 || mScanChannel > OT_RADIO_CHANNEL_MAX)
-        {
-            mEnergyScanHandler(mScanContext, NULL);
-            FinishOperation();
-            ExitNow();
-        }
-    }
-    while ((mScanChannels & 1) == 0);
-
-    // Start scanning the next channel
-    StartEnergyScan();
-
-exit:
-    return;
 }
 
 void Mac::HandleEnergyScanSampleRssi(Tasklet &aTasklet)
@@ -351,7 +356,14 @@ void Mac::HandleEnergyScanSampleRssi(void)
         }
     }
 
-    mEnergyScanSampleRssiTask.Post();
+    if (mScanDuration == 0)
+    {
+        EnergyScanDone(mEnergyScanCurrentMaxRssi);
+    }
+    else
+    {
+        mEnergyScanSampleRssiTask.Post();
+    }
 
 exit:
     return;
@@ -581,13 +593,13 @@ void Mac::StartOperation(Operation aOperation)
     {
         mPendingActiveScan = false;
         mOperation = kOperationActiveScan;
-        StartCsmaBackoff();
+        PerformActiveScan();
     }
     else if (mPendingEnergyScan)
     {
         mPendingEnergyScan = false;
         mOperation = kOperationEnergyScan;
-        StartEnergyScan();
+        PerformEnergyScan();
     }
     else if (mPendingTransmitBeacon)
     {
@@ -1262,22 +1274,7 @@ void Mac::HandleMacTimer(void)
     switch (mOperation)
     {
     case kOperationActiveScan:
-        do
+        PerformActiveScan();
-        {
-            mScanChannels >>= 1;
-            mScanChannel++;
-
-            if (mScanChannels == 0 || mScanChannel > OT_RADIO_CHANNEL_MAX)
-            {
-                otPlatRadioSetPanId(&GetInstance(), mPanId);
-                mActiveScanHandler(mScanContext, NULL);
-                FinishOperation();
-                ExitNow();
-            }
-        }
-        while ((mScanChannels & 1) == 0);
-
-        StartCsmaBackoff();
         break;
 
     case kOperationEnergyScan:
@@ -1293,9 +1290,6 @@ void Mac::HandleMacTimer(void)
         assert(false);
         break;
     }
-
-exit:
-    return;
 }
 
 void Mac::HandleReceiveTimer(Timer &aTimer)

src/core/mac/mac.hpp

@@ -226,7 +226,7 @@ public:
      * This function pointer is called on receiving an IEEE 802.15.4 Beacon during an Active Scan.
      *
      * @param[in]  aContext       A pointer to arbitrary context information.
-     * @param[in]  aBeaconFrame   A pointer to the Beacon frame.
+     * @param[in]  aBeaconFrame   A pointer to the Beacon frame or NULL to indicate end of Active Scan operation.
      *
      */
     typedef void (*ActiveScanHandler)(void *aContext, Frame *aBeaconFrame);
@@ -234,11 +234,15 @@ public:
     /**
      * This method starts an IEEE 802.15.4 Active Scan.
      *
-     * @param[in]  aScanChannels  A bit vector indicating which channels to scan.
+     * @param[in]  aScanChannels  A bit vector indicating which channels to scan. Zero is mapped to all channels.
-     * @param[in]  aScanDuration  The time in milliseconds to spend scanning each channel.
+     * @param[in]  aScanDuration  The time in milliseconds to spend scanning each channel. Zero duration maps to
+     *                            default value `kScanDurationDefault` = 300 ms.
      * @param[in]  aHandler       A pointer to a function that is called on receiving an IEEE 802.15.4 Beacon.
      * @param[in]  aContext       A pointer to arbitrary context information.
      *
+     * @retval OT_ERROR_NONE  Successfully scheduled the Active Scan request.
+     * @retval OT_ERROR_BUSY  Could not schedule the scan (a scan is ongoing or scheduled).
+     *
      */
     otError ActiveScan(uint32_t aScanChannels, uint16_t aScanDuration, ActiveScanHandler aHandler, void *aContext);
 
@@ -256,7 +260,7 @@ public:
     otError ConvertBeaconToActiveScanResult(Frame *aBeaconFrame, otActiveScanResult &aResult);
 
     /**
-     * This function pointer is called during an "Energy Scan" when the result for a channel is ready or the scan
+     * This function pointer is called during an Energy Scan when the result for a channel is ready or the scan
      * completes.
      *
      * @param[in]  aContext  A pointer to arbitrary context information.
@@ -269,8 +273,9 @@ public:
     /**
      * This method starts an IEEE 802.15.4 Energy Scan.
      *
-     * @param[in]  aScanChannels     A bit vector indicating on which channels to perform energy scan.
+     * @param[in]  aScanChannels     A bit vector indicating on which channels to scan. Zero is mapped to all channels.
-     * @param[in]  aScanDuration     The time in milliseconds to spend scanning each channel.
+     * @param[in]  aScanDuration     The time in milliseconds to spend scanning each channel. If the duration is set to
+     *                               zero, a single RSSI sample will be taken per channel.
      * @param[in]  aHandler          A pointer to a function called to pass on scan result or indicate scan completion.
      * @param[in]  aContext          A pointer to arbitrary context information.
      *
@@ -631,7 +636,6 @@ private:
     void SendBeaconRequest(Frame &aFrame);
     void SendBeacon(Frame &aFrame);
     void StartBackoff(void);
-    void StartEnergyScan(void);
     otError HandleMacCommand(Frame &aFrame);
 
     static void HandleMacTimer(Timer &aTimer);
@@ -644,7 +648,12 @@ private:
     void HandleEnergyScanSampleRssi(void);
 
     void StartCsmaBackoff(void);
-    otError Scan(Operation aScanOperation, uint32_t aScanChannels, uint16_t aScanDuration, void *aContext);
+
+    void Scan(Operation aScanOperation, uint32_t aScanChannels, uint16_t aScanDuration, void *aContext);
+    otError UpdateScanChannel(void);
+    void PerformActiveScan(void);
+    void PerformEnergyScan(void);
+    void ReportEnergyScanResult(int8_t aRssi);
 
     otError RadioTransmit(Frame *aSendFrame);
     otError RadioReceive(uint8_t aChannel);