[all-devices-app] Remove dependency on global LinuxDeviceOptions and add --KVS, --discriminator, --vendor-id and --product-id support#71735
Open
soares-sergio wants to merge 16 commits intoproject-chip:masterfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the application options into a centralized AppConfig structure and introduces new command-line arguments for the KVS path and setup discriminator. The review feedback highlights the need for robust input validation on the discriminator value to prevent unexpected behavior, suggests returning a const reference from GetConfig to prevent accidental modifications, and points out that the refactoring removed default device configuration logic which could cause issues when no arguments are provided.
|
PR #71735: Size comparison from e2dc005 to 723d1e8 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #71735 +/- ##
==========================================
- Coverage 54.81% 54.81% -0.01%
==========================================
Files 1603 1603
Lines 109834 109834
Branches 13452 13452
==========================================
- Hits 60203 60202 -1
- Misses 49631 49632 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
soares-sergio
force-pushed
the
all_devices_kvs
branch
from
794cfd4 to
c2c7728
Compare
soares-sergio
changed the title
soares-sergio
changed the title
|
PR #71735: Size comparison from c383110 to 39d9976 Full report (3 builds for realtek, stm32)
|
soares-sergio
force-pushed
the
all_devices_kvs
branch
from
df13d97 to
9c02903
Compare
soares-sergio
force-pushed
the
all_devices_kvs
branch
from
63ab458 to
1399267
Compare
soares-sergio
force-pushed
the
all_devices_kvs
branch
from
cc100df to
4a10b0b
Compare
soares-sergio
force-pushed
the
all_devices_kvs
branch
from
45fadcb to
f3d0dd9
Compare
|
PR #71735: Size comparison from f76eca9 to f3d0dd9 Full report (6 builds for cc32xx, nrfconnect, realtek, stm32)
|
…ovider and add safe fallbacks
soares-sergio
force-pushed
the
all_devices_kvs
branch
from
e5cdf71 to
9eba06f
Compare
for more information, see https://pre-commit.ci
|
PR #71735: Size comparison from f76eca9 to a1b4f94 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refactors
all-devices-appto remove its reliance on the globalLinuxDeviceOptionssingleton and centralizes its specific providers in theall-devices-commondirectory.Key Changes
Configuration and Options
ParseArgumentsfunction.--KVSoption, allowing specification of a custom Key-Value Store file path.--discriminatorby parsing it locally with validation (ensuring 12-bit value).--vendor-idand--product-idoptions to override default values dynamically.--portoption to override the default Matter port (5540), allowing multiple instances to run without port conflicts.AppOptionsto use a localAppConfigstruct instead of global state.GetConfig()return a const reference and restoring the default device fallback ("contact-sensor" on endpoint 1).deviceConfigstodeviceTypeEntriesfor consistency withDeviceTypeParser.Providers
DeviceInstanceInfoProviderImplinall-devices-common(moved and renamed fromAppDeviceInstanceInfoProvider) to handle VID/PID overrides via dependency injection.DeviceInfoProviderImplinall-devices-commonto provide supported locales, fixed labels, etc., removing the dependency onexamples/providers/AllClustersExampleDeviceInfoProviderImplandAllDevicesExampleDeviceInfoProviderImpl.AllDevicesExampleDeviceInfoProviderImplfromexamples/providers/as it is no longer needed.Build System
BUILD.gninexamples/all-devices-app/all-devices-common/to manage the new providers.examples/all-devices-app/posix/BUILD.gnandexamples/providers/BUILD.gnto reflect the provider changes.Testing
./all-devices-app --KVS out/test_kvs/kvs3.ini --discriminator 1234 --vendor-id 0x6006 --product-id 0xFE00 --port 5555sscommand.chip-toolwithpairing onnetwork-long.--bypass-attestation-verifier truewas used withchip-tool.Basic Informationcluster correctly reported the overridden values.