Add and Leverage Linux Connectivity Callback Introspection, Mutation, and Action Delegation Methods#43062
Conversation
bc53c60 to
4e48856
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces new callback setters and delegation methods to improve encapsulation and reduce code duplication within the Linux ConnectivityManagerImpl. The changes are well-structured and successfully centralize callback handling. All identified issues, including a critical one regarding uninitialized member variables and several medium-severity issues related to function parameter passing conventions, remain valid and should be addressed for consistency, performance, and to prevent undefined behavior.
923c954 to
6e18096
Compare
|
@enkiusz or @andy31415, any thoughts on this? |
|
@andy31415, any thoughts on this? |
|
@tersal, @arkq, or @andy31415, any thoughts on this? |
0ea7b3e to
78c8e81
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43062 +/- ##
=======================================
Coverage 54.04% 54.05%
=======================================
Files 1548 1548
Lines 106696 106704 +8
Branches 13314 13313 -1
=======================================
+ Hits 57669 57677 +8
Misses 49027 49027 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@andy31415, I had one open question for you; that aside, I think this incorporates all of your requested feedback. |
78c8e81 to
854e504
Compare
|
@andy31415, all outstanding comments resolved. Should be good to go for your final review. |
854e504 to
60eac26
Compare
60eac26 to
2545f54
Compare
2545f54 to
5c827cd
Compare
|
PR #43062: Size comparison from ba97f02 to 5c827cd Full report (30 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
This more accurately reflects its set-scan-and-clear usage.
This more accurately reflects its set-connect-and-clear usage.
These three methods consolidate the handling and dispatch of scan, connect, and network status change action delegation callback invocation and, for scan and connect, post-invocation nullification.
This not only makes it easier to keep callback members private as implementations fan out from the current, single wpa_supplicant implementation but also reduces duplication among those implementations for common callback-related operations.
Previously, the status change callback was initialized with a default initializer and the connect and scan callbacks were initialized in the explicit initializer. With this change, all three are now initialized in the explicit initializer.
5c827cd to
ebc639e
Compare
|
@enkiusz, any additional thoughts on this one? |
|
Thanks @andy31415 and @enkiusz! |
Summary
This adds two new callback setters as peers to the existing
SetNetworkStatusChangeCallback:SetConnectCallbackSetScanCallbackand adds three new callback-related action delegation methods:
OnConnectResultOnScanFinishedOnStatusChangeand adds two new callback-related introspection methods:
IsWiFiStationConnectingIsWiFiStationScanningThe three action delegation methods consolidate the handling and dispatch of scan, connect, and network status change action delegation callback invocation and, for scan and connect, post-invocation nullification.
Collectively, these seven methods not only make it easier to keep common callback members private as implementations fan out from the current, single
wpa_supplicantimplementation but also reduces duplication among those implementations for common callback-related operations.Related issues
#42516
Testing
An armvl-unknown-linux-gnu device has been successfully paired / commissioned using the iOS 26.2 Apple "Home" app with a Connectivity Manager implementation using this infrastructure.