You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/06/17 17:33:34 UTC

[GitHub] [incubator-nuttx] anchao opened a new pull request, #6466: wireless/bcm43xxx: sort scan result by rssi

anchao opened a new pull request, #6466:
URL: https://github.com/apache/incubator-nuttx/pull/6466

   ## Summary
   
   wireless/bcm43xxx: sort scan result by rssi
     
   1. Replace SCAN_RESULT_SIZE to SCAN_RESULT_ENTRIES
   2. filter scan result with better rssi
   3. Sort scan result by rssi
   
   ## Impact
   
   N/A
   
   ## Testing
   
   bcm43013 scan


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6466: wireless/bcm43xxx: sort scan result by rssi

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6466:
URL: https://github.com/apache/incubator-nuttx/pull/6466#discussion_r900699013


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,235 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
-
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
+          curr = &priv->scan_result[j];
 
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          /* Find worst rssi and mark the entries */
 
-      /* Check if current bss AP is not already detected */
+          if (curr->RSSI < worst_rssi)
+            {
+              worst_entries = j;
+              worst_rssi = curr->RSSI;
+            }
 
-      check_offset = 0;
+          /* Check if current bss AP is not already detected */
 
-      while (priv->scan_result_size - check_offset
-                                     >= offsetof(struct iw_event, u))
-        {
-          iwe = (struct iw_event *)&priv->scan_result[check_offset];
-
-          if (iwe->cmd == SIOCGIWAP)
+          if (memcmp(&curr->BSSID, &bss[i].BSSID,
+                     sizeof(curr->BSSID)) == 0)
             {
-              if (memcmp(&iwe->u.ap_addr.sa_data,
-                         bss->BSSID.ether_addr_octet,
-                         sizeof(bss->BSSID.ether_addr_octet)) == 0)
+              /* Replace the duplicate entries if rssi is
+               * better than before
+               */
+
+              if (curr->RSSI < bss[i].RSSI)
                 {
-                  goto process_next_bss;
+                  memcpy(curr, bss, sizeof(*curr));
                 }
-            }
 
-          check_offset += iwe->len;
+              goto process_next_bss;
+            }
         }
 
-      wlinfo("Scan result: <%.32s> %02x:%02x:%02x:%02x:%02x:%02x\n",
-               bss->SSID,
-               bss->BSSID.ether_addr_octet[0],
-               bss->BSSID.ether_addr_octet[1],
-               bss->BSSID.ether_addr_octet[2],
-               bss->BSSID.ether_addr_octet[3],
-               bss->BSSID.ether_addr_octet[4],
-               bss->BSSID.ether_addr_octet[5]);
-
-      /* Copy BSSID */
-
-      if (result_size < BCMF_IW_EVENT_SIZE(ap_addr))
+      if (priv->scan_result_entries == BCMF_SCAN_RESULT_ENTRIES)
         {
-          goto scan_result_full;
-        }
-
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(ap_addr);
-      iwe->cmd = SIOCGIWAP;
-      memcpy(&iwe->u.ap_addr.sa_data, bss->BSSID.ether_addr_octet,
-             sizeof(bss->BSSID.ether_addr_octet));
-      iwe->u.ap_addr.sa_family = ARPHRD_ETHER;
-
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(ap_addr);
-      result_size -= BCMF_IW_EVENT_SIZE(ap_addr);
+          /* Entries full and replace the worst entries */
 
-      /* Copy ESSID */
-
-      essid_len = min(strlen((const char *)bss->SSID), 32);
-      essid_len_aligned = (essid_len + 3) & -4;
+          if (worst_entries >= 0)
+            {
+              curr = &priv->scan_result[worst_entries];
+              if (curr->RSSI < bss->RSSI)
+                {
+                  memcpy(curr, bss, sizeof(*curr));
+                }
+            }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned)
-        {
-          goto scan_result_full;
+process_next_bss:
+          continue;
         }
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      iwe->cmd = SIOCGIWESSID;
-      iwe->u.essid.flags = 0;
-      iwe->u.essid.length = essid_len;
-
-      /* Special processing for iw_point, set offset in pointer field */
-
-      iwe->u.essid.pointer = (FAR void *)sizeof(iwe->u.essid);
-      memcpy(&iwe->u.essid + 1, bss->SSID, essid_len);
-
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      result_size -= BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
+      curr = &priv->scan_result[priv->scan_result_entries];
+      memcpy(curr, bss, sizeof(wl_bss_info_t));

Review Comment:
   ```suggestion
         memcpy(curr, bss, sizeof(*curr));
   ```
   Just to follow the same style as at line 728 and 712



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -56,7 +56,7 @@
 #define DOT11_BSSTYPE_ANY      2
 #define BCMF_SCAN_TIMEOUT_TICK (5*CLOCKS_PER_SEC)
 #define BCMF_AUTH_TIMEOUT_MS   20000  /* was 10000 */

Review Comment:
   ```suggestion
   #define DOT11_BSSTYPE_ANY         2
   #define BCMF_SCAN_TIMEOUT_TICK    (5*CLOCKS_PER_SEC)
   #define BCMF_AUTH_TIMEOUT_MS      20000  /* was 10000 */
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6466: wireless/bcm43xxx: sort scan result by rssi

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6466:
URL: https://github.com/apache/incubator-nuttx/pull/6466


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #6466: wireless/bcm43xxx: sort scan result by rssi

Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6466:
URL: https://github.com/apache/incubator-nuttx/pull/6466#discussion_r900784447


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
+      wlinfo("Scan result: <%.32s> "
+             "%02x:%02x:%02x:%02x:%02x:%02x "
+             "signal %d %d %d\n",
+             bss->SSID,
+             bss->BSSID.ether_addr_octet[0],
+             bss->BSSID.ether_addr_octet[1],
+             bss->BSSID.ether_addr_octet[2],
+             bss->BSSID.ether_addr_octet[3],
+             bss->BSSID.ether_addr_octet[4],
+             bss->BSSID.ether_addr_octet[5],
+             bss->RSSI, bss->phy_noise, bss->SNR);
 
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
-
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          curr = &priv->scan_result[j];
 
-      /* Check if current bss AP is not already detected */
+          /* Find worst rssi and mark the entries */
 
-      check_offset = 0;
+          if (curr->RSSI < worst_rssi)
+            {
+              worst_entries = j;
+              worst_rssi = curr->RSSI;
+            }
 
-      while (priv->scan_result_size - check_offset
-                                     >= offsetof(struct iw_event, u))
-        {
-          iwe = (struct iw_event *)&priv->scan_result[check_offset];
+          /* Check if current bss AP is not already detected */
 
-          if (iwe->cmd == SIOCGIWAP)
+          if (memcmp(&curr->BSSID, &bss[i].BSSID,
+                     sizeof(curr->BSSID)) == 0)
             {
-              if (memcmp(&iwe->u.ap_addr.sa_data,
-                         bss->BSSID.ether_addr_octet,
-                         sizeof(bss->BSSID.ether_addr_octet)) == 0)
+              /* Replace the duplicate entries if rssi is
+               * better than before
+               */
+
+              if (curr->RSSI < bss[i].RSSI)
                 {
-                  goto process_next_bss;
+                  memcpy(curr, bss, sizeof(*curr));
                 }
-            }
 
-          check_offset += iwe->len;
+              goto process_next_bss;
+            }
         }
 
-      wlinfo("Scan result: <%.32s> %02x:%02x:%02x:%02x:%02x:%02x\n",
-               bss->SSID,
-               bss->BSSID.ether_addr_octet[0],
-               bss->BSSID.ether_addr_octet[1],
-               bss->BSSID.ether_addr_octet[2],
-               bss->BSSID.ether_addr_octet[3],
-               bss->BSSID.ether_addr_octet[4],
-               bss->BSSID.ether_addr_octet[5]);
-
-      /* Copy BSSID */
-
-      if (result_size < BCMF_IW_EVENT_SIZE(ap_addr))
+      if (priv->scan_result_entries == BCMF_SCAN_RESULT_ENTRIES)
         {
-          goto scan_result_full;
-        }
+          /* Entries full and replace the worst entries */
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(ap_addr);
-      iwe->cmd = SIOCGIWAP;
-      memcpy(&iwe->u.ap_addr.sa_data, bss->BSSID.ether_addr_octet,
-             sizeof(bss->BSSID.ether_addr_octet));
-      iwe->u.ap_addr.sa_family = ARPHRD_ETHER;
-
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(ap_addr);
-      result_size -= BCMF_IW_EVENT_SIZE(ap_addr);
-
-      /* Copy ESSID */
-
-      essid_len = min(strlen((const char *)bss->SSID), 32);
-      essid_len_aligned = (essid_len + 3) & -4;
+          if (worst_entries >= 0)
+            {
+              curr = &priv->scan_result[worst_entries];
+              if (curr->RSSI < bss->RSSI)
+                {
+                  memcpy(curr, bss, sizeof(*curr));
+                }
+            }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned)
-        {
-          goto scan_result_full;
+process_next_bss:
+          continue;
         }
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      iwe->cmd = SIOCGIWESSID;
-      iwe->u.essid.flags = 0;
-      iwe->u.essid.length = essid_len;
-
-      /* Special processing for iw_point, set offset in pointer field */
-
-      iwe->u.essid.pointer = (FAR void *)sizeof(iwe->u.essid);
-      memcpy(&iwe->u.essid + 1, bss->SSID, essid_len);
+      curr = &priv->scan_result[priv->scan_result_entries];
+      memcpy(curr, bss, sizeof(*curr));
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      result_size -= BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-
-      /* Copy link quality info */
+      priv->scan_result_entries++;
+    }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(qual))
-        {
-          goto scan_result_full;
-        }
+wl_escan_result_processed:
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(qual);
-      iwe->cmd = IWEVQUAL;
-      iwe->u.qual.qual = bss->SNR;
-      wlinfo("signal %d %d %d\n", bss->RSSI, bss->phy_noise, bss->SNR);
-      iwe->u.qual.level = bss->RSSI;
-      iwe->u.qual.noise = bss->phy_noise;
-      iwe->u.qual.updated = IW_QUAL_DBM | IW_QUAL_ALL_UPDATED;
+  if (status == WLC_E_STATUS_PARTIAL)
+    {
+      /* More frames to come */
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(qual);
-      result_size -= BCMF_IW_EVENT_SIZE(qual);
+      return;
+    }
 
-      /* Copy AP mode */
+  if (status != WLC_E_STATUS_SUCCESS)
+    {
+      wlerr("Invalid event status %" PRId32 "\n", status);
+      return;
+    }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(mode))
-        {
-          goto scan_result_full;
-        }
+  /* Scan done */
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(mode);
-      iwe->cmd = SIOCGIWMODE;
-      if (bss->capability & DOT11_CAP_ESS)
-        {
-          iwe->u.mode = IW_MODE_INFRA;
-        }
-      else if (bss->capability & DOT11_CAP_IBSS)
-        {
-          iwe->u.mode = IW_MODE_ADHOC;
-        }
-      else
-        {
-          iwe->u.mode = IW_MODE_AUTO;
-        }
+  wlinfo("escan done event %" PRId32 " %" PRId32 "\n",
+         status, bcmf_getle32(&event->reason));
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(mode);
-      result_size -= BCMF_IW_EVENT_SIZE(mode);
+  wd_cancel(&priv->scan_timeout);
 
-      /* Copy AP encryption mode */
+  priv->scan_status = BCMF_SCAN_DONE;
+  nxsem_post(&priv->control_mutex);
 
-      if (result_size < BCMF_IW_EVENT_SIZE(data))
-        {
-          goto scan_result_full;
-        }
+  return;
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(data);
-      iwe->cmd = SIOCGIWENCODE;
-      iwe->u.data.flags = bss->capability & DOT11_CAP_PRIVACY ?
-                          IW_ENCODE_ENABLED | IW_ENCODE_NOKEY :
-                          IW_ENCODE_DISABLED;
-      iwe->u.data.length = 0;
-      iwe->u.essid.pointer = NULL;
+exit_invalid_frame:
+  wlerr("Invalid scan result event\n");
+  bcmf_hexdump((FAR uint8_t *)event, event_len, (unsigned long)event);
+}
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(data);
-      result_size -= BCMF_IW_EVENT_SIZE(data);
+static FAR char *
+bcmf_wl_iwe_add_event(FAR char *stream, FAR char *stop,
+                      FAR struct iw_event *iwe, int event_len)
+{
+  if (stream + event_len > stop)
+    {
+      return stream;
+    }
 
-      /* Copy relevant raw IE frame */
+  iwe->len = event_len;
+  return stream + event_len;
+}
 
-      if (bss->ie_offset >= bss_info_len ||
-          bss->ie_length > bss_info_len - bss->ie_offset)
-        {
-          goto process_next_bss;
-        }
+static int bcmf_wl_scan_format_results(FAR struct bcmf_dev_s *priv,
+                                       FAR struct iwreq *iwr)
+{
+  FAR wl_bss_info_t *scan_result[BCMF_SCAN_RESULT_ENTRIES];
+  FAR wl_bss_info_t *info;
+  FAR struct iw_event *iwe;
+  FAR char *start;
+  FAR char *stop;
+  int len;
+  int i;
+  int j;
 
-      ie_offset = 0;
-      ie_buffer = (uint8_t *)bss + bss->ie_offset;
+  if (priv->scan_result_entries == 0)
+    {
+      iwr->u.data.length = 0;
+      return OK;
+    }
 
-      while (1)
-        {
-          size_t ie_frame_size;
+  len = IW_EV_LEN(ap_addr) + IW_EV_LEN(qual) +
+        IW_EV_LEN(freq)    + IW_EV_LEN(data) +
+        IW_EV_LEN(essid);
 
-          if (bss->ie_length - ie_offset < 2)
-            {
-              /* Minimum Information element size is 2 bytes */
+  len *= priv->scan_result_entries;
 
-              break;
-            }
+  for (i = 0; i < priv->scan_result_entries; i++)
+    {
+      scan_result[i] = &priv->scan_result[i];
+      len += (min(strlen((FAR const char *)scan_result[i]->SSID),
+                         32) + 3) & ~3;
+    }
 
-          ie_frame_size = ie_buffer[ie_offset + 1] + 2;
+  if (iwr->u.data.pointer == NULL || iwr->u.data.length < len)
+    {
+      iwr->u.data.length = len;
+      return -E2BIG;
+    }
 
-          if (ie_frame_size > bss->ie_length - ie_offset)
-            {
-              /* Entry too big */
+  start = iwr->u.data.pointer;
+  stop = (FAR char *)iwr->u.data.pointer + iwr->u.data.length;
 
-              break;
-            }
+  /* Sort list by RSSI */
 
-          switch (ie_buffer[ie_offset])
+  for (i = 0; i < priv->scan_result_entries; i++)
+    {
+      for (j = 0; j + 1 < priv->scan_result_entries - i; j++)
+        {
+          if (scan_result[j]->RSSI < scan_result[j + 1]->RSSI)
             {
-              case IEEE80211_ELEMID_RSN:
-                {
-                  size_t ie_frame_size_aligned;
-                  ie_frame_size_aligned = (ie_frame_size + 3) & -4;
-
-                  wlinfo("found RSN\n");
-
-                  if (result_size < BCMF_IW_EVENT_SIZE(data) +
-                                    ie_frame_size_aligned)
-                    {
-                      break;
-                    }
-
-                  iwe = (struct iw_event *)
-                    &priv->scan_result[priv->scan_result_size];
-
-                  iwe->len = BCMF_IW_EVENT_SIZE(data)+ie_frame_size_aligned;
-                  iwe->cmd = IWEVGENIE;
-                  iwe->u.data.flags = 0;
-                  iwe->u.data.length = ie_frame_size;
-                  iwe->u.data.pointer = (FAR void *)sizeof(iwe->u.data);
-                  memcpy(&iwe->u.data + 1, &ie_buffer[ie_offset],
-                         ie_frame_size);
-
-                  priv->scan_result_size += BCMF_IW_EVENT_SIZE(essid) +
-                                            ie_frame_size_aligned;
-                  result_size -= BCMF_IW_EVENT_SIZE(essid) +
-                                 ie_frame_size_aligned;
-                  break;
-                }
-
-              default:
-                break;
+              info = scan_result[j];
+              scan_result[j] = scan_result[j + 1];
+              scan_result[j + 1] = info;
             }
-
-          ie_offset += ie_buffer[ie_offset + 1] + 2;
         }
+    }
 
-      goto process_next_bss;
-
-    scan_result_full:
-
-      /* Continue instead of break to log dropped AP results */
-
-      wlerr("No more space in scan_result buffer\n");
-
-    process_next_bss:
+  /* Copy scan result */
 
-      /* Process next bss_info */
+  for (i = 0; i < priv->scan_result_entries; i++)
+    {
+      info = scan_result[i];
 
-      len -= bss_info_len;
-      bss = (struct wl_bss_info *)((uint8_t *)bss + bss_info_len);
-      bss_count += 1;
-    }
+      /* Copy BSSID */
 
-wl_escan_result_processed:
+      iwe = (FAR struct iw_event *)start;
+      iwe->cmd = SIOCGIWAP;
+      iwe->u.ap_addr.sa_family = ARPHRD_ETHER;
+      memcpy(&iwe->u.ap_addr.sa_data,
+             info->BSSID.ether_addr_octet, IFHWADDRLEN);
+      start = bcmf_wl_iwe_add_event(start, stop, iwe, IW_EV_LEN(ap_addr));

Review Comment:
   Done



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
+      wlinfo("Scan result: <%.32s> "
+             "%02x:%02x:%02x:%02x:%02x:%02x "
+             "signal %d %d %d\n",
+             bss->SSID,
+             bss->BSSID.ether_addr_octet[0],
+             bss->BSSID.ether_addr_octet[1],
+             bss->BSSID.ether_addr_octet[2],
+             bss->BSSID.ether_addr_octet[3],
+             bss->BSSID.ether_addr_octet[4],
+             bss->BSSID.ether_addr_octet[5],
+             bss->RSSI, bss->phy_noise, bss->SNR);
 
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
-
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          curr = &priv->scan_result[j];
 
-      /* Check if current bss AP is not already detected */
+          /* Find worst rssi and mark the entries */
 
-      check_offset = 0;
+          if (curr->RSSI < worst_rssi)
+            {
+              worst_entries = j;
+              worst_rssi = curr->RSSI;
+            }
 
-      while (priv->scan_result_size - check_offset
-                                     >= offsetof(struct iw_event, u))
-        {
-          iwe = (struct iw_event *)&priv->scan_result[check_offset];
+          /* Check if current bss AP is not already detected */
 
-          if (iwe->cmd == SIOCGIWAP)
+          if (memcmp(&curr->BSSID, &bss[i].BSSID,
+                     sizeof(curr->BSSID)) == 0)
             {
-              if (memcmp(&iwe->u.ap_addr.sa_data,
-                         bss->BSSID.ether_addr_octet,
-                         sizeof(bss->BSSID.ether_addr_octet)) == 0)
+              /* Replace the duplicate entries if rssi is
+               * better than before
+               */
+
+              if (curr->RSSI < bss[i].RSSI)
                 {
-                  goto process_next_bss;
+                  memcpy(curr, bss, sizeof(*curr));
                 }
-            }
 
-          check_offset += iwe->len;
+              goto process_next_bss;
+            }
         }
 
-      wlinfo("Scan result: <%.32s> %02x:%02x:%02x:%02x:%02x:%02x\n",
-               bss->SSID,
-               bss->BSSID.ether_addr_octet[0],
-               bss->BSSID.ether_addr_octet[1],
-               bss->BSSID.ether_addr_octet[2],
-               bss->BSSID.ether_addr_octet[3],
-               bss->BSSID.ether_addr_octet[4],
-               bss->BSSID.ether_addr_octet[5]);
-
-      /* Copy BSSID */
-
-      if (result_size < BCMF_IW_EVENT_SIZE(ap_addr))
+      if (priv->scan_result_entries == BCMF_SCAN_RESULT_ENTRIES)
         {
-          goto scan_result_full;
-        }
+          /* Entries full and replace the worst entries */
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(ap_addr);
-      iwe->cmd = SIOCGIWAP;
-      memcpy(&iwe->u.ap_addr.sa_data, bss->BSSID.ether_addr_octet,
-             sizeof(bss->BSSID.ether_addr_octet));
-      iwe->u.ap_addr.sa_family = ARPHRD_ETHER;
-
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(ap_addr);
-      result_size -= BCMF_IW_EVENT_SIZE(ap_addr);
-
-      /* Copy ESSID */
-
-      essid_len = min(strlen((const char *)bss->SSID), 32);
-      essid_len_aligned = (essid_len + 3) & -4;
+          if (worst_entries >= 0)
+            {
+              curr = &priv->scan_result[worst_entries];
+              if (curr->RSSI < bss->RSSI)
+                {
+                  memcpy(curr, bss, sizeof(*curr));
+                }
+            }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned)
-        {
-          goto scan_result_full;
+process_next_bss:
+          continue;
         }
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      iwe->cmd = SIOCGIWESSID;
-      iwe->u.essid.flags = 0;
-      iwe->u.essid.length = essid_len;
-
-      /* Special processing for iw_point, set offset in pointer field */
-
-      iwe->u.essid.pointer = (FAR void *)sizeof(iwe->u.essid);
-      memcpy(&iwe->u.essid + 1, bss->SSID, essid_len);
+      curr = &priv->scan_result[priv->scan_result_entries];
+      memcpy(curr, bss, sizeof(*curr));
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      result_size -= BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-
-      /* Copy link quality info */
+      priv->scan_result_entries++;
+    }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(qual))
-        {
-          goto scan_result_full;
-        }
+wl_escan_result_processed:
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(qual);
-      iwe->cmd = IWEVQUAL;
-      iwe->u.qual.qual = bss->SNR;
-      wlinfo("signal %d %d %d\n", bss->RSSI, bss->phy_noise, bss->SNR);
-      iwe->u.qual.level = bss->RSSI;
-      iwe->u.qual.noise = bss->phy_noise;
-      iwe->u.qual.updated = IW_QUAL_DBM | IW_QUAL_ALL_UPDATED;
+  if (status == WLC_E_STATUS_PARTIAL)
+    {
+      /* More frames to come */
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(qual);
-      result_size -= BCMF_IW_EVENT_SIZE(qual);
+      return;
+    }
 
-      /* Copy AP mode */
+  if (status != WLC_E_STATUS_SUCCESS)
+    {
+      wlerr("Invalid event status %" PRId32 "\n", status);
+      return;
+    }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(mode))
-        {
-          goto scan_result_full;
-        }
+  /* Scan done */
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(mode);
-      iwe->cmd = SIOCGIWMODE;
-      if (bss->capability & DOT11_CAP_ESS)
-        {
-          iwe->u.mode = IW_MODE_INFRA;
-        }
-      else if (bss->capability & DOT11_CAP_IBSS)
-        {
-          iwe->u.mode = IW_MODE_ADHOC;
-        }
-      else
-        {
-          iwe->u.mode = IW_MODE_AUTO;
-        }
+  wlinfo("escan done event %" PRId32 " %" PRId32 "\n",
+         status, bcmf_getle32(&event->reason));
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(mode);
-      result_size -= BCMF_IW_EVENT_SIZE(mode);
+  wd_cancel(&priv->scan_timeout);
 
-      /* Copy AP encryption mode */
+  priv->scan_status = BCMF_SCAN_DONE;
+  nxsem_post(&priv->control_mutex);
 
-      if (result_size < BCMF_IW_EVENT_SIZE(data))
-        {
-          goto scan_result_full;
-        }
+  return;
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(data);
-      iwe->cmd = SIOCGIWENCODE;
-      iwe->u.data.flags = bss->capability & DOT11_CAP_PRIVACY ?
-                          IW_ENCODE_ENABLED | IW_ENCODE_NOKEY :
-                          IW_ENCODE_DISABLED;
-      iwe->u.data.length = 0;
-      iwe->u.essid.pointer = NULL;
+exit_invalid_frame:
+  wlerr("Invalid scan result event\n");
+  bcmf_hexdump((FAR uint8_t *)event, event_len, (unsigned long)event);
+}
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(data);
-      result_size -= BCMF_IW_EVENT_SIZE(data);
+static FAR char *
+bcmf_wl_iwe_add_event(FAR char *stream, FAR char *stop,
+                      FAR struct iw_event *iwe, int event_len)
+{
+  if (stream + event_len > stop)

Review Comment:
   Done



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
+      wlinfo("Scan result: <%.32s> "
+             "%02x:%02x:%02x:%02x:%02x:%02x "
+             "signal %d %d %d\n",
+             bss->SSID,
+             bss->BSSID.ether_addr_octet[0],
+             bss->BSSID.ether_addr_octet[1],
+             bss->BSSID.ether_addr_octet[2],
+             bss->BSSID.ether_addr_octet[3],
+             bss->BSSID.ether_addr_octet[4],
+             bss->BSSID.ether_addr_octet[5],
+             bss->RSSI, bss->phy_noise, bss->SNR);
 
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
-
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          curr = &priv->scan_result[j];
 
-      /* Check if current bss AP is not already detected */
+          /* Find worst rssi and mark the entries */
 
-      check_offset = 0;
+          if (curr->RSSI < worst_rssi)
+            {
+              worst_entries = j;
+              worst_rssi = curr->RSSI;
+            }
 
-      while (priv->scan_result_size - check_offset
-                                     >= offsetof(struct iw_event, u))
-        {
-          iwe = (struct iw_event *)&priv->scan_result[check_offset];
+          /* Check if current bss AP is not already detected */
 
-          if (iwe->cmd == SIOCGIWAP)
+          if (memcmp(&curr->BSSID, &bss[i].BSSID,

Review Comment:
   Done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6466: wireless/bcm43xxx: sort scan result by rssi

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6466:
URL: https://github.com/apache/incubator-nuttx/pull/6466#discussion_r900751609


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
+      wlinfo("Scan result: <%.32s> "
+             "%02x:%02x:%02x:%02x:%02x:%02x "
+             "signal %d %d %d\n",
+             bss->SSID,
+             bss->BSSID.ether_addr_octet[0],
+             bss->BSSID.ether_addr_octet[1],
+             bss->BSSID.ether_addr_octet[2],
+             bss->BSSID.ether_addr_octet[3],
+             bss->BSSID.ether_addr_octet[4],
+             bss->BSSID.ether_addr_octet[5],
+             bss->RSSI, bss->phy_noise, bss->SNR);
 
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
-
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          curr = &priv->scan_result[j];
 
-      /* Check if current bss AP is not already detected */
+          /* Find worst rssi and mark the entries */

Review Comment:
   entries->entry



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;

Review Comment:
   keep in the origin location



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
+      wlinfo("Scan result: <%.32s> "
+             "%02x:%02x:%02x:%02x:%02x:%02x "
+             "signal %d %d %d\n",
+             bss->SSID,
+             bss->BSSID.ether_addr_octet[0],
+             bss->BSSID.ether_addr_octet[1],
+             bss->BSSID.ether_addr_octet[2],
+             bss->BSSID.ether_addr_octet[3],
+             bss->BSSID.ether_addr_octet[4],
+             bss->BSSID.ether_addr_octet[5],
+             bss->RSSI, bss->phy_noise, bss->SNR);
 
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
-
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          curr = &priv->scan_result[j];
 
-      /* Check if current bss AP is not already detected */
+          /* Find worst rssi and mark the entries */
 
-      check_offset = 0;
+          if (curr->RSSI < worst_rssi)
+            {
+              worst_entries = j;
+              worst_rssi = curr->RSSI;
+            }
 
-      while (priv->scan_result_size - check_offset
-                                     >= offsetof(struct iw_event, u))
-        {
-          iwe = (struct iw_event *)&priv->scan_result[check_offset];
+          /* Check if current bss AP is not already detected */
 
-          if (iwe->cmd == SIOCGIWAP)
+          if (memcmp(&curr->BSSID, &bss[i].BSSID,
+                     sizeof(curr->BSSID)) == 0)
             {
-              if (memcmp(&iwe->u.ap_addr.sa_data,
-                         bss->BSSID.ether_addr_octet,
-                         sizeof(bss->BSSID.ether_addr_octet)) == 0)
+              /* Replace the duplicate entries if rssi is
+               * better than before
+               */
+
+              if (curr->RSSI < bss[i].RSSI)
                 {
-                  goto process_next_bss;
+                  memcpy(curr, bss, sizeof(*curr));
                 }
-            }
 
-          check_offset += iwe->len;
+              goto process_next_bss;
+            }
         }
 
-      wlinfo("Scan result: <%.32s> %02x:%02x:%02x:%02x:%02x:%02x\n",
-               bss->SSID,
-               bss->BSSID.ether_addr_octet[0],
-               bss->BSSID.ether_addr_octet[1],
-               bss->BSSID.ether_addr_octet[2],
-               bss->BSSID.ether_addr_octet[3],
-               bss->BSSID.ether_addr_octet[4],
-               bss->BSSID.ether_addr_octet[5]);
-
-      /* Copy BSSID */
-
-      if (result_size < BCMF_IW_EVENT_SIZE(ap_addr))
+      if (priv->scan_result_entries == BCMF_SCAN_RESULT_ENTRIES)
         {
-          goto scan_result_full;
-        }
+          /* Entries full and replace the worst entries */
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(ap_addr);
-      iwe->cmd = SIOCGIWAP;
-      memcpy(&iwe->u.ap_addr.sa_data, bss->BSSID.ether_addr_octet,
-             sizeof(bss->BSSID.ether_addr_octet));
-      iwe->u.ap_addr.sa_family = ARPHRD_ETHER;
-
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(ap_addr);
-      result_size -= BCMF_IW_EVENT_SIZE(ap_addr);
-
-      /* Copy ESSID */
-
-      essid_len = min(strlen((const char *)bss->SSID), 32);
-      essid_len_aligned = (essid_len + 3) & -4;
+          if (worst_entries >= 0)
+            {
+              curr = &priv->scan_result[worst_entries];
+              if (curr->RSSI < bss->RSSI)
+                {
+                  memcpy(curr, bss, sizeof(*curr));
+                }
+            }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned)
-        {
-          goto scan_result_full;
+process_next_bss:
+          continue;
         }
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      iwe->cmd = SIOCGIWESSID;
-      iwe->u.essid.flags = 0;
-      iwe->u.essid.length = essid_len;
-
-      /* Special processing for iw_point, set offset in pointer field */
-
-      iwe->u.essid.pointer = (FAR void *)sizeof(iwe->u.essid);
-      memcpy(&iwe->u.essid + 1, bss->SSID, essid_len);
+      curr = &priv->scan_result[priv->scan_result_entries];
+      memcpy(curr, bss, sizeof(*curr));
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      result_size -= BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-
-      /* Copy link quality info */
+      priv->scan_result_entries++;
+    }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(qual))
-        {
-          goto scan_result_full;
-        }
+wl_escan_result_processed:
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(qual);
-      iwe->cmd = IWEVQUAL;
-      iwe->u.qual.qual = bss->SNR;
-      wlinfo("signal %d %d %d\n", bss->RSSI, bss->phy_noise, bss->SNR);
-      iwe->u.qual.level = bss->RSSI;
-      iwe->u.qual.noise = bss->phy_noise;
-      iwe->u.qual.updated = IW_QUAL_DBM | IW_QUAL_ALL_UPDATED;
+  if (status == WLC_E_STATUS_PARTIAL)
+    {
+      /* More frames to come */
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(qual);
-      result_size -= BCMF_IW_EVENT_SIZE(qual);
+      return;
+    }
 
-      /* Copy AP mode */
+  if (status != WLC_E_STATUS_SUCCESS)
+    {
+      wlerr("Invalid event status %" PRId32 "\n", status);
+      return;
+    }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(mode))
-        {
-          goto scan_result_full;
-        }
+  /* Scan done */
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(mode);
-      iwe->cmd = SIOCGIWMODE;
-      if (bss->capability & DOT11_CAP_ESS)
-        {
-          iwe->u.mode = IW_MODE_INFRA;
-        }
-      else if (bss->capability & DOT11_CAP_IBSS)
-        {
-          iwe->u.mode = IW_MODE_ADHOC;
-        }
-      else
-        {
-          iwe->u.mode = IW_MODE_AUTO;
-        }
+  wlinfo("escan done event %" PRId32 " %" PRId32 "\n",
+         status, bcmf_getle32(&event->reason));
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(mode);
-      result_size -= BCMF_IW_EVENT_SIZE(mode);
+  wd_cancel(&priv->scan_timeout);
 
-      /* Copy AP encryption mode */
+  priv->scan_status = BCMF_SCAN_DONE;
+  nxsem_post(&priv->control_mutex);
 
-      if (result_size < BCMF_IW_EVENT_SIZE(data))
-        {
-          goto scan_result_full;
-        }
+  return;
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(data);
-      iwe->cmd = SIOCGIWENCODE;
-      iwe->u.data.flags = bss->capability & DOT11_CAP_PRIVACY ?
-                          IW_ENCODE_ENABLED | IW_ENCODE_NOKEY :
-                          IW_ENCODE_DISABLED;
-      iwe->u.data.length = 0;
-      iwe->u.essid.pointer = NULL;
+exit_invalid_frame:
+  wlerr("Invalid scan result event\n");
+  bcmf_hexdump((FAR uint8_t *)event, event_len, (unsigned long)event);
+}
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(data);
-      result_size -= BCMF_IW_EVENT_SIZE(data);
+static FAR char *
+bcmf_wl_iwe_add_event(FAR char *stream, FAR char *stop,
+                      FAR struct iw_event *iwe, int event_len)
+{
+  if (stream + event_len > stop)

Review Comment:
   why need check again? Line 833 already handle this case.



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -623,16 +623,19 @@ void bcmf_wl_auth_event_handler(FAR struct bcmf_dev_s *priv,
  */
 
 void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
-                                struct bcmf_event_s *event,
+                                FAR struct bcmf_event_s *event,
                                 unsigned int len)
 {
-  uint32_t status;
-  uint32_t event_len;
-  struct wl_escan_result *result;
-  struct wl_bss_info *bss;
-  unsigned int bss_info_len;
+  FAR struct wl_escan_result *result;
   unsigned int escan_result_len;
-  unsigned int bss_count = 0;
+  FAR struct wl_bss_info *curr;
+  FAR struct wl_bss_info *bss;
+  uint32_t event_len;
+  int16_t worst_rssi;
+  int worst_entries;

Review Comment:
   worst_entry



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
+      wlinfo("Scan result: <%.32s> "
+             "%02x:%02x:%02x:%02x:%02x:%02x "
+             "signal %d %d %d\n",
+             bss->SSID,
+             bss->BSSID.ether_addr_octet[0],
+             bss->BSSID.ether_addr_octet[1],
+             bss->BSSID.ether_addr_octet[2],
+             bss->BSSID.ether_addr_octet[3],
+             bss->BSSID.ether_addr_octet[4],
+             bss->BSSID.ether_addr_octet[5],
+             bss->RSSI, bss->phy_noise, bss->SNR);
 
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
-
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          curr = &priv->scan_result[j];
 
-      /* Check if current bss AP is not already detected */
+          /* Find worst rssi and mark the entries */
 
-      check_offset = 0;
+          if (curr->RSSI < worst_rssi)
+            {
+              worst_entries = j;
+              worst_rssi = curr->RSSI;
+            }
 
-      while (priv->scan_result_size - check_offset
-                                     >= offsetof(struct iw_event, u))
-        {
-          iwe = (struct iw_event *)&priv->scan_result[check_offset];
+          /* Check if current bss AP is not already detected */
 
-          if (iwe->cmd == SIOCGIWAP)
+          if (memcmp(&curr->BSSID, &bss[i].BSSID,
+                     sizeof(curr->BSSID)) == 0)
             {
-              if (memcmp(&iwe->u.ap_addr.sa_data,
-                         bss->BSSID.ether_addr_octet,
-                         sizeof(bss->BSSID.ether_addr_octet)) == 0)
+              /* Replace the duplicate entries if rssi is

Review Comment:
   entries->entry



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
+      wlinfo("Scan result: <%.32s> "
+             "%02x:%02x:%02x:%02x:%02x:%02x "
+             "signal %d %d %d\n",
+             bss->SSID,
+             bss->BSSID.ether_addr_octet[0],
+             bss->BSSID.ether_addr_octet[1],
+             bss->BSSID.ether_addr_octet[2],
+             bss->BSSID.ether_addr_octet[3],
+             bss->BSSID.ether_addr_octet[4],
+             bss->BSSID.ether_addr_octet[5],
+             bss->RSSI, bss->phy_noise, bss->SNR);
 
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
-
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          curr = &priv->scan_result[j];
 
-      /* Check if current bss AP is not already detected */
+          /* Find worst rssi and mark the entries */
 
-      check_offset = 0;
+          if (curr->RSSI < worst_rssi)
+            {
+              worst_entries = j;
+              worst_rssi = curr->RSSI;
+            }
 
-      while (priv->scan_result_size - check_offset
-                                     >= offsetof(struct iw_event, u))
-        {
-          iwe = (struct iw_event *)&priv->scan_result[check_offset];
+          /* Check if current bss AP is not already detected */
 
-          if (iwe->cmd == SIOCGIWAP)
+          if (memcmp(&curr->BSSID, &bss[i].BSSID,

Review Comment:
   move the check to the first one



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
+      wlinfo("Scan result: <%.32s> "
+             "%02x:%02x:%02x:%02x:%02x:%02x "
+             "signal %d %d %d\n",
+             bss->SSID,
+             bss->BSSID.ether_addr_octet[0],
+             bss->BSSID.ether_addr_octet[1],
+             bss->BSSID.ether_addr_octet[2],
+             bss->BSSID.ether_addr_octet[3],
+             bss->BSSID.ether_addr_octet[4],
+             bss->BSSID.ether_addr_octet[5],
+             bss->RSSI, bss->phy_noise, bss->SNR);
 
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
-
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          curr = &priv->scan_result[j];
 
-      /* Check if current bss AP is not already detected */
+          /* Find worst rssi and mark the entries */
 
-      check_offset = 0;
+          if (curr->RSSI < worst_rssi)
+            {
+              worst_entries = j;
+              worst_rssi = curr->RSSI;
+            }
 
-      while (priv->scan_result_size - check_offset
-                                     >= offsetof(struct iw_event, u))
-        {
-          iwe = (struct iw_event *)&priv->scan_result[check_offset];
+          /* Check if current bss AP is not already detected */
 
-          if (iwe->cmd == SIOCGIWAP)
+          if (memcmp(&curr->BSSID, &bss[i].BSSID,
+                     sizeof(curr->BSSID)) == 0)
             {
-              if (memcmp(&iwe->u.ap_addr.sa_data,
-                         bss->BSSID.ether_addr_octet,
-                         sizeof(bss->BSSID.ether_addr_octet)) == 0)
+              /* Replace the duplicate entries if rssi is
+               * better than before
+               */
+
+              if (curr->RSSI < bss[i].RSSI)
                 {
-                  goto process_next_bss;
+                  memcpy(curr, bss, sizeof(*curr));
                 }
-            }
 
-          check_offset += iwe->len;
+              goto process_next_bss;
+            }
         }
 
-      wlinfo("Scan result: <%.32s> %02x:%02x:%02x:%02x:%02x:%02x\n",
-               bss->SSID,
-               bss->BSSID.ether_addr_octet[0],
-               bss->BSSID.ether_addr_octet[1],
-               bss->BSSID.ether_addr_octet[2],
-               bss->BSSID.ether_addr_octet[3],
-               bss->BSSID.ether_addr_octet[4],
-               bss->BSSID.ether_addr_octet[5]);
-
-      /* Copy BSSID */
-
-      if (result_size < BCMF_IW_EVENT_SIZE(ap_addr))
+      if (priv->scan_result_entries == BCMF_SCAN_RESULT_ENTRIES)
         {
-          goto scan_result_full;
-        }
+          /* Entries full and replace the worst entries */
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(ap_addr);
-      iwe->cmd = SIOCGIWAP;
-      memcpy(&iwe->u.ap_addr.sa_data, bss->BSSID.ether_addr_octet,
-             sizeof(bss->BSSID.ether_addr_octet));
-      iwe->u.ap_addr.sa_family = ARPHRD_ETHER;
-
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(ap_addr);
-      result_size -= BCMF_IW_EVENT_SIZE(ap_addr);
-
-      /* Copy ESSID */
-
-      essid_len = min(strlen((const char *)bss->SSID), 32);
-      essid_len_aligned = (essid_len + 3) & -4;
+          if (worst_entries >= 0)
+            {
+              curr = &priv->scan_result[worst_entries];
+              if (curr->RSSI < bss->RSSI)
+                {
+                  memcpy(curr, bss, sizeof(*curr));
+                }
+            }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned)
-        {
-          goto scan_result_full;
+process_next_bss:
+          continue;
         }
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      iwe->cmd = SIOCGIWESSID;
-      iwe->u.essid.flags = 0;
-      iwe->u.essid.length = essid_len;
-
-      /* Special processing for iw_point, set offset in pointer field */
-
-      iwe->u.essid.pointer = (FAR void *)sizeof(iwe->u.essid);
-      memcpy(&iwe->u.essid + 1, bss->SSID, essid_len);
+      curr = &priv->scan_result[priv->scan_result_entries];
+      memcpy(curr, bss, sizeof(*curr));
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      result_size -= BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-
-      /* Copy link quality info */
+      priv->scan_result_entries++;
+    }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(qual))
-        {
-          goto scan_result_full;
-        }
+wl_escan_result_processed:
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(qual);
-      iwe->cmd = IWEVQUAL;
-      iwe->u.qual.qual = bss->SNR;
-      wlinfo("signal %d %d %d\n", bss->RSSI, bss->phy_noise, bss->SNR);
-      iwe->u.qual.level = bss->RSSI;
-      iwe->u.qual.noise = bss->phy_noise;
-      iwe->u.qual.updated = IW_QUAL_DBM | IW_QUAL_ALL_UPDATED;
+  if (status == WLC_E_STATUS_PARTIAL)
+    {
+      /* More frames to come */
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(qual);
-      result_size -= BCMF_IW_EVENT_SIZE(qual);
+      return;
+    }
 
-      /* Copy AP mode */
+  if (status != WLC_E_STATUS_SUCCESS)
+    {
+      wlerr("Invalid event status %" PRId32 "\n", status);
+      return;
+    }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(mode))
-        {
-          goto scan_result_full;
-        }
+  /* Scan done */
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(mode);
-      iwe->cmd = SIOCGIWMODE;
-      if (bss->capability & DOT11_CAP_ESS)
-        {
-          iwe->u.mode = IW_MODE_INFRA;
-        }
-      else if (bss->capability & DOT11_CAP_IBSS)
-        {
-          iwe->u.mode = IW_MODE_ADHOC;
-        }
-      else
-        {
-          iwe->u.mode = IW_MODE_AUTO;
-        }
+  wlinfo("escan done event %" PRId32 " %" PRId32 "\n",
+         status, bcmf_getle32(&event->reason));
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(mode);
-      result_size -= BCMF_IW_EVENT_SIZE(mode);
+  wd_cancel(&priv->scan_timeout);
 
-      /* Copy AP encryption mode */
+  priv->scan_status = BCMF_SCAN_DONE;
+  nxsem_post(&priv->control_mutex);
 
-      if (result_size < BCMF_IW_EVENT_SIZE(data))
-        {
-          goto scan_result_full;
-        }
+  return;
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(data);
-      iwe->cmd = SIOCGIWENCODE;
-      iwe->u.data.flags = bss->capability & DOT11_CAP_PRIVACY ?
-                          IW_ENCODE_ENABLED | IW_ENCODE_NOKEY :
-                          IW_ENCODE_DISABLED;
-      iwe->u.data.length = 0;
-      iwe->u.essid.pointer = NULL;
+exit_invalid_frame:
+  wlerr("Invalid scan result event\n");
+  bcmf_hexdump((FAR uint8_t *)event, event_len, (unsigned long)event);
+}
 
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(data);
-      result_size -= BCMF_IW_EVENT_SIZE(data);
+static FAR char *
+bcmf_wl_iwe_add_event(FAR char *stream, FAR char *stop,
+                      FAR struct iw_event *iwe, int event_len)
+{
+  if (stream + event_len > stop)
+    {
+      return stream;
+    }
 
-      /* Copy relevant raw IE frame */
+  iwe->len = event_len;
+  return stream + event_len;
+}
 
-      if (bss->ie_offset >= bss_info_len ||
-          bss->ie_length > bss_info_len - bss->ie_offset)
-        {
-          goto process_next_bss;
-        }
+static int bcmf_wl_scan_format_results(FAR struct bcmf_dev_s *priv,
+                                       FAR struct iwreq *iwr)
+{
+  FAR wl_bss_info_t *scan_result[BCMF_SCAN_RESULT_ENTRIES];
+  FAR wl_bss_info_t *info;
+  FAR struct iw_event *iwe;
+  FAR char *start;
+  FAR char *stop;
+  int len;
+  int i;
+  int j;
 
-      ie_offset = 0;
-      ie_buffer = (uint8_t *)bss + bss->ie_offset;
+  if (priv->scan_result_entries == 0)
+    {
+      iwr->u.data.length = 0;
+      return OK;
+    }
 
-      while (1)
-        {
-          size_t ie_frame_size;
+  len = IW_EV_LEN(ap_addr) + IW_EV_LEN(qual) +
+        IW_EV_LEN(freq)    + IW_EV_LEN(data) +
+        IW_EV_LEN(essid);
 
-          if (bss->ie_length - ie_offset < 2)
-            {
-              /* Minimum Information element size is 2 bytes */
+  len *= priv->scan_result_entries;
 
-              break;
-            }
+  for (i = 0; i < priv->scan_result_entries; i++)
+    {
+      scan_result[i] = &priv->scan_result[i];
+      len += (min(strlen((FAR const char *)scan_result[i]->SSID),
+                         32) + 3) & ~3;
+    }
 
-          ie_frame_size = ie_buffer[ie_offset + 1] + 2;
+  if (iwr->u.data.pointer == NULL || iwr->u.data.length < len)
+    {
+      iwr->u.data.length = len;
+      return -E2BIG;
+    }
 
-          if (ie_frame_size > bss->ie_length - ie_offset)
-            {
-              /* Entry too big */
+  start = iwr->u.data.pointer;
+  stop = (FAR char *)iwr->u.data.pointer + iwr->u.data.length;
 
-              break;
-            }
+  /* Sort list by RSSI */
 
-          switch (ie_buffer[ie_offset])
+  for (i = 0; i < priv->scan_result_entries; i++)
+    {
+      for (j = 0; j + 1 < priv->scan_result_entries - i; j++)
+        {
+          if (scan_result[j]->RSSI < scan_result[j + 1]->RSSI)
             {
-              case IEEE80211_ELEMID_RSN:
-                {
-                  size_t ie_frame_size_aligned;
-                  ie_frame_size_aligned = (ie_frame_size + 3) & -4;
-
-                  wlinfo("found RSN\n");
-
-                  if (result_size < BCMF_IW_EVENT_SIZE(data) +
-                                    ie_frame_size_aligned)
-                    {
-                      break;
-                    }
-
-                  iwe = (struct iw_event *)
-                    &priv->scan_result[priv->scan_result_size];
-
-                  iwe->len = BCMF_IW_EVENT_SIZE(data)+ie_frame_size_aligned;
-                  iwe->cmd = IWEVGENIE;
-                  iwe->u.data.flags = 0;
-                  iwe->u.data.length = ie_frame_size;
-                  iwe->u.data.pointer = (FAR void *)sizeof(iwe->u.data);
-                  memcpy(&iwe->u.data + 1, &ie_buffer[ie_offset],
-                         ie_frame_size);
-
-                  priv->scan_result_size += BCMF_IW_EVENT_SIZE(essid) +
-                                            ie_frame_size_aligned;
-                  result_size -= BCMF_IW_EVENT_SIZE(essid) +
-                                 ie_frame_size_aligned;
-                  break;
-                }
-
-              default:
-                break;
+              info = scan_result[j];
+              scan_result[j] = scan_result[j + 1];
+              scan_result[j + 1] = info;
             }
-
-          ie_offset += ie_buffer[ie_offset + 1] + 2;
         }
+    }
 
-      goto process_next_bss;
-
-    scan_result_full:
-
-      /* Continue instead of break to log dropped AP results */
-
-      wlerr("No more space in scan_result buffer\n");
-
-    process_next_bss:
+  /* Copy scan result */
 
-      /* Process next bss_info */
+  for (i = 0; i < priv->scan_result_entries; i++)
+    {
+      info = scan_result[i];
 
-      len -= bss_info_len;
-      bss = (struct wl_bss_info *)((uint8_t *)bss + bss_info_len);
-      bss_count += 1;
-    }
+      /* Copy BSSID */
 
-wl_escan_result_processed:
+      iwe = (FAR struct iw_event *)start;
+      iwe->cmd = SIOCGIWAP;
+      iwe->u.ap_addr.sa_family = ARPHRD_ETHER;
+      memcpy(&iwe->u.ap_addr.sa_data,
+             info->BSSID.ether_addr_octet, IFHWADDRLEN);
+      start = bcmf_wl_iwe_add_event(start, stop, iwe, IW_EV_LEN(ap_addr));

Review Comment:
   let's use iwe directly and remove start/stop



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #6466: wireless/bcm43xxx: sort scan result by rssi

Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6466:
URL: https://github.com/apache/incubator-nuttx/pull/6466#discussion_r900776750


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -623,16 +623,19 @@ void bcmf_wl_auth_event_handler(FAR struct bcmf_dev_s *priv,
  */
 
 void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
-                                struct bcmf_event_s *event,
+                                FAR struct bcmf_event_s *event,
                                 unsigned int len)
 {
-  uint32_t status;
-  uint32_t event_len;
-  struct wl_escan_result *result;
-  struct wl_bss_info *bss;
-  unsigned int bss_info_len;
+  FAR struct wl_escan_result *result;
   unsigned int escan_result_len;
-  unsigned int bss_count = 0;
+  FAR struct wl_bss_info *curr;
+  FAR struct wl_bss_info *bss;
+  uint32_t event_len;
+  int16_t worst_rssi;
+  int worst_entries;

Review Comment:
   done



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
+      wlinfo("Scan result: <%.32s> "
+             "%02x:%02x:%02x:%02x:%02x:%02x "
+             "signal %d %d %d\n",
+             bss->SSID,
+             bss->BSSID.ether_addr_octet[0],
+             bss->BSSID.ether_addr_octet[1],
+             bss->BSSID.ether_addr_octet[2],
+             bss->BSSID.ether_addr_octet[3],
+             bss->BSSID.ether_addr_octet[4],
+             bss->BSSID.ether_addr_octet[5],
+             bss->RSSI, bss->phy_noise, bss->SNR);
 
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
-
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          curr = &priv->scan_result[j];
 
-      /* Check if current bss AP is not already detected */
+          /* Find worst rssi and mark the entries */
 
-      check_offset = 0;
+          if (curr->RSSI < worst_rssi)
+            {
+              worst_entries = j;
+              worst_rssi = curr->RSSI;
+            }
 
-      while (priv->scan_result_size - check_offset
-                                     >= offsetof(struct iw_event, u))
-        {
-          iwe = (struct iw_event *)&priv->scan_result[check_offset];
+          /* Check if current bss AP is not already detected */
 
-          if (iwe->cmd == SIOCGIWAP)
+          if (memcmp(&curr->BSSID, &bss[i].BSSID,
+                     sizeof(curr->BSSID)) == 0)
             {
-              if (memcmp(&iwe->u.ap_addr.sa_data,
-                         bss->BSSID.ether_addr_octet,
-                         sizeof(bss->BSSID.ether_addr_octet)) == 0)
+              /* Replace the duplicate entries if rssi is

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #6466: wireless/bcm43xxx: sort scan result by rssi

Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6466:
URL: https://github.com/apache/incubator-nuttx/pull/6466#discussion_r900707373


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,235 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
-
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
+          curr = &priv->scan_result[j];
 
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          /* Find worst rssi and mark the entries */
 
-      /* Check if current bss AP is not already detected */
+          if (curr->RSSI < worst_rssi)
+            {
+              worst_entries = j;
+              worst_rssi = curr->RSSI;
+            }
 
-      check_offset = 0;
+          /* Check if current bss AP is not already detected */
 
-      while (priv->scan_result_size - check_offset
-                                     >= offsetof(struct iw_event, u))
-        {
-          iwe = (struct iw_event *)&priv->scan_result[check_offset];
-
-          if (iwe->cmd == SIOCGIWAP)
+          if (memcmp(&curr->BSSID, &bss[i].BSSID,
+                     sizeof(curr->BSSID)) == 0)
             {
-              if (memcmp(&iwe->u.ap_addr.sa_data,
-                         bss->BSSID.ether_addr_octet,
-                         sizeof(bss->BSSID.ether_addr_octet)) == 0)
+              /* Replace the duplicate entries if rssi is
+               * better than before
+               */
+
+              if (curr->RSSI < bss[i].RSSI)
                 {
-                  goto process_next_bss;
+                  memcpy(curr, bss, sizeof(*curr));
                 }
-            }
 
-          check_offset += iwe->len;
+              goto process_next_bss;
+            }
         }
 
-      wlinfo("Scan result: <%.32s> %02x:%02x:%02x:%02x:%02x:%02x\n",
-               bss->SSID,
-               bss->BSSID.ether_addr_octet[0],
-               bss->BSSID.ether_addr_octet[1],
-               bss->BSSID.ether_addr_octet[2],
-               bss->BSSID.ether_addr_octet[3],
-               bss->BSSID.ether_addr_octet[4],
-               bss->BSSID.ether_addr_octet[5]);
-
-      /* Copy BSSID */
-
-      if (result_size < BCMF_IW_EVENT_SIZE(ap_addr))
+      if (priv->scan_result_entries == BCMF_SCAN_RESULT_ENTRIES)
         {
-          goto scan_result_full;
-        }
-
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(ap_addr);
-      iwe->cmd = SIOCGIWAP;
-      memcpy(&iwe->u.ap_addr.sa_data, bss->BSSID.ether_addr_octet,
-             sizeof(bss->BSSID.ether_addr_octet));
-      iwe->u.ap_addr.sa_family = ARPHRD_ETHER;
-
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(ap_addr);
-      result_size -= BCMF_IW_EVENT_SIZE(ap_addr);
+          /* Entries full and replace the worst entries */
 
-      /* Copy ESSID */
-
-      essid_len = min(strlen((const char *)bss->SSID), 32);
-      essid_len_aligned = (essid_len + 3) & -4;
+          if (worst_entries >= 0)
+            {
+              curr = &priv->scan_result[worst_entries];
+              if (curr->RSSI < bss->RSSI)
+                {
+                  memcpy(curr, bss, sizeof(*curr));
+                }
+            }
 
-      if (result_size < BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned)
-        {
-          goto scan_result_full;
+process_next_bss:
+          continue;
         }
 
-      iwe = (struct iw_event *)&priv->scan_result[priv->scan_result_size];
-      iwe->len = BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      iwe->cmd = SIOCGIWESSID;
-      iwe->u.essid.flags = 0;
-      iwe->u.essid.length = essid_len;
-
-      /* Special processing for iw_point, set offset in pointer field */
-
-      iwe->u.essid.pointer = (FAR void *)sizeof(iwe->u.essid);
-      memcpy(&iwe->u.essid + 1, bss->SSID, essid_len);
-
-      priv->scan_result_size += BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
-      result_size -= BCMF_IW_EVENT_SIZE(essid)+essid_len_aligned;
+      curr = &priv->scan_result[priv->scan_result_entries];
+      memcpy(curr, bss, sizeof(wl_bss_info_t));

Review Comment:
   done



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -56,7 +56,7 @@
 #define DOT11_BSSTYPE_ANY      2
 #define BCMF_SCAN_TIMEOUT_TICK (5*CLOCKS_PER_SEC)
 #define BCMF_AUTH_TIMEOUT_MS   20000  /* was 10000 */

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #6466: wireless/bcm43xxx: sort scan result by rssi

Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6466:
URL: https://github.com/apache/incubator-nuttx/pull/6466#discussion_r900784582


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;
 
   /* Process bss_infos */
 
-  bss = result->bss_info;
-
-  while (len > 0 && bss_count < result->bss_count)
+  for (i = 0; i < result->bss_count; i++)
     {
-      struct iw_event *iwe;
-      unsigned int result_size;
-      size_t essid_len;
-      size_t essid_len_aligned;
-      uint8_t *ie_buffer;
-      unsigned int ie_offset;
-      unsigned int check_offset;
+      worst_entries = -1;
+      worst_rssi = 0;
 
-      result_size = BCMF_SCAN_RESULT_SIZE - priv->scan_result_size;
-      bss_info_len = bss->length;
+      wlinfo("Scan result: <%.32s> "
+             "%02x:%02x:%02x:%02x:%02x:%02x "
+             "signal %d %d %d\n",
+             bss->SSID,
+             bss->BSSID.ether_addr_octet[0],
+             bss->BSSID.ether_addr_octet[1],
+             bss->BSSID.ether_addr_octet[2],
+             bss->BSSID.ether_addr_octet[3],
+             bss->BSSID.ether_addr_octet[4],
+             bss->BSSID.ether_addr_octet[5],
+             bss->RSSI, bss->phy_noise, bss->SNR);
 
-      if (len < bss_info_len)
+      for (j = 0; j < priv->scan_result_entries; j++)
         {
-          wlerr("bss_len error %d %d\n", len, bss_info_len);
-          goto exit_invalid_frame;
-        }
-
-      /* Append current bss_info to priv->scan_results
-       * FIXME protect this against race conditions
-       */
+          curr = &priv->scan_result[j];
 
-      /* Check if current bss AP is not already detected */
+          /* Find worst rssi and mark the entries */

Review Comment:
   Done



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_driver.c:
##########
@@ -666,300 +669,247 @@ void bcmf_wl_scan_event_handler(FAR struct bcmf_dev_s *priv,
 
   /* Process escan result payload */
 
-  result = (struct wl_escan_result *)&event[1];
+  result = (FAR struct wl_escan_result *)&event[1];
 
   if (len < result->buflen ||
       result->buflen < sizeof(struct wl_escan_result))
     {
       goto exit_invalid_frame;
     }
 
-  /* wl_escan_result structure contains a wl_bss_info field */
-
-  len = result->buflen - sizeof(struct wl_escan_result)
-                       + sizeof(struct wl_bss_info);
+  bss = result->bss_info;

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org