-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[Zephyr] Update Zephyr api to match Zephyr 4.2 #40948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[Zephyr] Update Zephyr api to match Zephyr 4.2 #40948
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the Zephyr platform integration to support Zephyr 4.2, specifically addressing API changes and deprecations in the new version. The changes make WPA_SUPPLICANT functionality modular and adapt the codebase to handle version-specific API differences.
- Updates event handler signatures to use uint64_t for Zephyr 4.2+ (previously uint32_t)
- Makes WPA_SUPPLICANT related code conditional via configuration flags
- Replaces deprecated BT_GATT_CCC_INITIALIZER with BT_GATT_CCC_MANAGED_USER_DATA_INIT for Bluetooth
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
src/platform/Zephyr/wifi/WiFiManager.h | Adds version checks and conditional compilation for WPA_SUPPLICANT, updates event handler signatures |
src/platform/Zephyr/wifi/WiFiManager.cpp | Removes event handler map, adds inline switch statement for event handling, makes disconnect handling conditional |
src/platform/Zephyr/BLEManagerImpl.cpp | Updates Bluetooth GATT CCC initialization for Zephyr 4.2 compatibility |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request updates the Zephyr API usage to be compatible with version 4.2 and makes the WPA_SUPPLICANT functionality modular. The changes are well-structured and use conditional compilation to support multiple Zephyr versions and configurations.
I've found one critical issue related to a potential memory leak in the DisconnectHandler
which should be addressed. Otherwise, the changes look good and align with the goals of the pull request.
uint16_t reason = 0; | ||
#ifdef CONFIG_WIFI_NM_WPA_SUPPLICANT | ||
Platform::UniquePtr<uint8_t> safePtr(capturedData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a potential memory leak here. The safePtr
which should take ownership of capturedData
is created inside the #ifdef CONFIG_WIFI_NM_WPA_SUPPLICANT
block. However, data.release()
is called unconditionally later, transferring ownership to the lambda. If CONFIG_WIFI_NM_WPA_SUPPLICANT
is not defined, no UniquePtr
takes ownership of the raw pointer inside the lambda, leading to a memory leak.
To fix this, Platform::UniquePtr<uint8_t> safePtr(capturedData);
should be moved outside and before the #ifdef
block to ensure the memory is always managed correctly, regardless of the compilation flag.
Platform::UniquePtr<uint8_t> safePtr(capturedData);
uint16_t reason = 0;
#ifdef CONFIG_WIFI_NM_WPA_SUPPLICANT
Converting to draft to figure out a way to handle disconnection even in the absence of WPA_SUPPLICANT feature of Zephyr. |
02a4c10
to
647da7a
Compare
647da7a
to
2e5063e
Compare
PR #40948: Size comparison from a82e43e to 2e5063e Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40948 +/- ##
==========================================
+ Coverage 50.77% 50.92% +0.15%
==========================================
Files 1361 1361
Lines 99850 99861 +11
Branches 12933 12917 -16
==========================================
+ Hits 50695 50851 +156
+ Misses 49155 49010 -145 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
With Zephyr 4.2 some API were deprecated/updated. Update the necessary files so that everything works with the latest and greatest
Also make WPA_SUPPLICANT related functionality modular.
Related issues
None
Testing
We were able to successfully compile a Zephyr based sample app with theses changes without triggering any build failure using zephyr 4.2 upstream