Skip to content

Conversation

@dpgeorge
Copy link
Member

Summary

The num_stas was uninitialised and if it happened to take the value 0 then no results were returned. It now has the correct maximum value.

Testing

Tested with AP on PYBD_SF6 and two STA's connected (PYBD_SF2 and ESP32_GENERIC_S2). With the AP running the following code:

while True:
    print(ap.isconnected(), ap.status('stations'))
    time.sleep(1)

Prior to this fix ap.status('stations') would sometimes be empty. After the fix it always contained the correct MAC's.

@dpgeorge dpgeorge added the extmod Relates to extmod/ directory in source label Nov 19, 2024
@dpgeorge dpgeorge requested a review from projectgus November 19, 2024 00:34
@dpgeorge dpgeorge added this to the release-1.24.1 milestone Nov 19, 2024
@codecov
Copy link

codecov bot commented Nov 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.57%. Comparing base (f562aa1) to head (af743ea).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #16265   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files         164      164           
  Lines       21347    21347           
=======================================
  Hits        21043    21043           
  Misses        304      304           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@github-actions
Copy link

github-actions bot commented Nov 19, 2024

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:   +24 +0.003% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

Copy link
Contributor

@projectgus projectgus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one more to chalk up to automated testing! I'll send another PR to update the wlan test to call status('stations') and check the other ports.

It'd also be nice to #define MAC_LEN 6 at the top of this file and replace some of the other magic number 6s, but can do that in a follow-up as well.

The `num_stas` was uninitialised and if it happened to take the value 0
then no results were returned.  It now has the correct maximum value.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge force-pushed the extmod-network-cyw43-fix-status-stations branch from e816778 to af743ea Compare November 20, 2024 03:24
@dpgeorge
Copy link
Member Author

It'd also be nice to #define MAC_LEN 6 at the top of this file and replace some of the other magic number 6s, but can do that in a follow-up as well.

Yes, I agree. Also worth checking the other network drivers to see if they can have a similar improvement.

@dpgeorge dpgeorge merged commit af743ea into micropython:master Nov 20, 2024
56 of 60 checks passed
@dpgeorge dpgeorge deleted the extmod-network-cyw43-fix-status-stations branch November 20, 2024 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extmod Relates to extmod/ directory in source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants