[device_info_plus] Add duid getter - #1106
Conversation
Add DeviceInfoPluginTizen.duid to read the device's DUID via vconf.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a569b092b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Fixes a PR review comment: caching the resolved value meant a null DUID never got cached, so every call re-triggered the native lookup.
| auto vconf_get_str = | ||
| reinterpret_cast<FuncVconfGetStr>(dlsym(handle, "vconf_get_str")); | ||
| if (vconf_get_str) { | ||
| if (char *value = vconf_get_str("db/comss/duid")) { |
There was a problem hiding this comment.
We need to check whether this path is maintained and managed identically in the common profile as well.
In the case of vconf_get_str used in in_app_purchase, it is used only in the TV profile, but this package is also used in the common profile.
| void *handle = dlopen("libvconf.so.0.3.1", RTLD_LAZY); | ||
| if (!handle) { | ||
| LOG_ERROR("Failed to open libvconf.so.0.3.1."); | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
I am well aware that it is used to the same way in in_app_purchase, but since libvconf.so.3.1 includes the detailed version, it may be vulnerable to future changes. What about libvconf.so.0?
Would it actually be more vulnerable to potential issues regarding management or changes?
| test('duid does not throw and is a nullable string', () async { | ||
| final deviceInfoPlugin = DeviceInfoPluginTizen(); | ||
| final duid = await deviceInfoPlugin.duid; | ||
| expect(duid, anyOf(isNull, isA<String>())); |
There was a problem hiding this comment.
expect(duid, anyOf(isNull, isA<String>())) holds for every possible value of a Future<String?>, so this assertion can never failed.
test('duid does not throw and is a non-empty string when available', () async {
final deviceInfoPlugin = DeviceInfoPluginTizen();
final duid = await deviceInfoPlugin.duid;
if (duid != null) {
expect(duid, isNotEmpty);
}
}, skip: !Platform.isLinux);
| /// Returns `null` if the DUID is unavailable, for example because the | ||
| /// underlying `db/comss/duid` vconf key could not be read. | ||
| Future<String?> get duid => _cachedDuid ??= _platform.getDuid(); |
There was a problem hiding this comment.
Just curios: If conditions are favorable for success after the first failure, the failure case is always cached. Is this intended behavior? Would that scenario never occur?
|
|
||
| ## Supported properties | ||
|
|
||
| | Property | Feature or system key | |
There was a problem hiding this comment.
Please add the API you want to add in the table. If it is TV only, please add a new column.
Add DeviceInfoPluginTizen.duid to read the device's DUID via vconf. This is a Tizen-specific API with no upstream counterpart in device_info_plus.