Matter server: primary_interface config option#4070
Matter server: primary_interface config option#4070elestril wants to merge 2 commits intohome-assistant:masterfrom
Conversation
Allow primary_interface to be manually set in config
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
home-assistant
bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
📝 WalkthroughWalkthroughThe changes introduce a new optional configuration option, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Config
participant StartupScript
participant SupervisorAPI
User->>Config: Sets primary_interface (optional)
StartupScript->>Config: Reads primary_interface
alt primary_interface is set
StartupScript->>StartupScript: Use configured value
else primary_interface not set
StartupScript->>SupervisorAPI: Query for primary interface
SupervisorAPI-->>StartupScript: Return detected interface
end
StartupScript->>MatterServer: Start with selected primary interface
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
matter_server/CHANGELOG.md(1 hunks)matter_server/config.yaml(3 hunks)matter_server/rootfs/etc/s6-overlay/s6-rc.d/matter-server/run(1 hunks)matter_server/translations/en.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
`*/**(html|markdown|md)`: - For instructional content in documentation, use a di...
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
matter_server/CHANGELOG.mdmatter_server/config.yaml
`*/**(html|markdown|md)`: - Use bold to mark UI strings. - If "" are used to mark UI strings, replace them by bold.
*/**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
matter_server/CHANGELOG.mdmatter_server/config.yaml
`*/**(html|markdown|md)`: - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
*/**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
matter_server/CHANGELOG.mdmatter_server/config.yaml
`*/**(html|markdown|md)`: - Use sentence-style capitalization also in headings.
*/**(html|markdown|md): - Use sentence-style capitalization also in headings.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
matter_server/CHANGELOG.mdmatter_server/config.yaml
`*/**(html|markdown|md)`: do not comment on HTML used for icons
*/**(html|markdown|md): do not comment on HTML used for icons
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
matter_server/CHANGELOG.mdmatter_server/config.yaml
`*/**(html|markdown|md)`: Avoid flagging inline HTML for embedding videos in future reviews for this repository.
*/**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
matter_server/CHANGELOG.mdmatter_server/config.yaml
🔇 Additional comments (4)
matter_server/config.yaml (2)
2-2: LGTM - Version bump is appropriate.Version bump to 8.0.1 is correctly applied for the new feature addition.
42-42: LGTM - Schema definition is correct.The schema definition for
primary_interfaceas an optional string is appropriate.matter_server/rootfs/etc/s6-overlay/s6-rc.d/matter-server/run (1)
56-60: LGTM - Conditional logic correctly implements the new feature.The implementation properly checks for user-configured
primary_interfaceand falls back to the supervisor API when not set. This addresses the original issue where the run script would override the--primary_interfaceargument even if specified inmatter_server_args.matter_server/translations/en.yaml (1)
45-49: LGTM - Translation entry is clear and informative.The translation entry provides appropriate name and description for the
primary_interfaceconfiguration option, clearly explaining its purpose for overriding the default system primary interface used for provisioning Matter devices.
|
Hm, if you specify Did you use separate fields for the argument ( |
|
Regardless of whether that works or not, having the option spelled out like this makes it much easier to discover rather than a wormhole of googling and trying to figure out how to isolate matter/thread devices from the internet. |
The problem is that the primary interface argument is really somewhat misleading: It has very limited significance currently, as it only scopes link-local addresses which we receive from the Android App (see #3468 (comment), or more specifically matter-js/python-matter-server#501). Ultimately this argument likely will be removed again from the Matter Server, once we have a way to rediscover the Matter device commissioned through Android. If discovering the configuration is a problem, we can promote the Tip If you are using Android to commission Matter devices, the Matter device needs to be on reachable through the network interface considered as primary interface by Home Assistant (check That said, I just double checked again, specifying the argument in the extra argument list seems to work. I am closing this PR as adding a dedicated option is not the right approach here. |
Can you elaborate on what you did to make it work? We have multiple reports from people (including myself) that adding the argument has no impact and that is verified by the log messages which are using the default interface rather than the specified interface. |
I've simply added it to the Extra Matter Server arguments. The follow log message is a red herring, this is a SDK detected primary interface which has no significance: See: #4092 (comment) |
|
I see, you're saying that the log message is coming from the script which is just indicating what it autodetected, but the service itself is properly using the second primary interface argument which is the one specified by the user? |
Exactly, the Python Matter Server code itself is using the last I've opened a PR to print the primary interface name as known by the Matter Server (see matter-js/python-matter-server#1212). With that it should be easier to verify that the argument indeed is processed correctly. |
|
Does changing primary interface do anything to prevent Thread devices from accessing the internet? |
No. Currently, the primary interface only has influence during commissioning with the Android Companion app. |
I think that's actually what I'm after. When I try to add a Matter device on another vlan it connects to wifi just fine but then fails to be added to Home Assistant. I tried adding another network interface to Home Assistant on that vlan but that didn't work which is how I ended up on this issue. |
... using Android? Yes, then that is what primary interface is helping to solve. You should be able to fix it by adding |
|
Sorry for letting this go stale. To wrap this up: does achieve the same thing, and using the Log from mainline version, using this config (relevant lines): So the I would still argue that |
|
This problem continues to manifest in Incus OCI host-networking deployments. |
5 participants
Allow to configure primary_interface in addon json
That was probably the intent of #3468, but the run script overrides
--primary_interfaceeven when provided inmatter_server_argsCloses: #4028
See also: https://community.home-assistant.io/t/home-assistant-cant-add-matter-devices-that-are-in-a-different-vlan/750324/54
Summary by CodeRabbit
New Features
Documentation
Chores