-
Notifications
You must be signed in to change notification settings - Fork 223
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
[host] add network properties class #2387
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2387 +/- ##
===========================================
- Coverage 55.77% 45.37% -10.41%
===========================================
Files 87 94 +7
Lines 6890 10590 +3700
Branches 0 769 +769
===========================================
+ Hits 3843 4805 +962
- Misses 3047 5546 +2499
- Partials 0 239 +239 ☔ View full report in Codecov by Sentry. |
361c148
to
79ef2d4
Compare
The idea is to maintain a cache of the properties at the host (by receiving updates from the callbacks). Right? I think 'synchronized' may not be accurate here. |
79ef2d4
to
94e8f87
Compare
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.
LGTM 👍
ffb4ed1
to
24edda5
Compare
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.
LGTM
24edda5
to
238219c
Compare
238219c
to
40c5a6a
Compare
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.
Looks great. Thanks @Irving-cl.
One (optional) suggestion below (above) 😄
93a5832
to
c19aa5f
Compare
c19aa5f
to
b26915c
Compare
b26915c
to
51e700e
Compare
This PR adds a new interface
NetworkProperties
to provide a unified, synchronized way to access network properties for both NCP and RCP.This is important for adding NCP support because in some cases the network properties are required by some functionality and we don't want to make an asynchronous call simply to get the property. As one example, in AdvertisingProxy, we need to the MeshLocal Prefix to check if an address is mesh local address. Currently this is done by OT API, which is not available under NCP mode. However, the information is contained in the active dataset and NCP will always notify the host about the change. With
NetworkProperties
, we can always get these properties in a synchronized way.This PR also replaces a few usage of
otThreadGetDeviceRole
byNetworkProperties
. In this way we will gradually remove the direct usage of OT APIs and mInstance in ot-br-posix modules. In this way we can develop for both NCP&RCP more easily.