-
Notifications
You must be signed in to change notification settings - Fork 235
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
[meshcop] support non-standard TXT entries at runtime #2308
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2308 +/- ##
===========================================
- Coverage 55.77% 39.81% -15.97%
===========================================
Files 87 89 +2
Lines 6890 9873 +2983
Branches 0 727 +727
===========================================
+ Hits 3843 3931 +88
- Misses 3047 5744 +2697
- Partials 0 198 +198 ☔ View full report in Codecov by Sentry. |
8653d98
to
fc1cdb0
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 overall 👍
In the future we may want to unify the two ways (D-BUS, Android overlay) for customizing meshcop TXT for better readability.
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.
can we use the existing HandleUpdateVendorMeshCoPTxtEntries?
That one doesn't support overriding the vendor or product name |
I think we should allow overriding these fields by dbus api as well. |
I am not sure that's a good idea. The It would require the dbus client side change if you want to ensure that advertised vendor name never changes for dbus-controlled device. But this is out of scope I think. |
See also https://android-review.git.corp.google.com/c/platform/external/ot-br-posix/+/3111060
for how this change is used and tested.