-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
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
Improve Onkyo documentation #35754
Improve Onkyo documentation #35754
Conversation
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 WalkthroughWalkthroughThe changes involve updates to the Onkyo integration documentation for Home Assistant, specifically altering the integration type from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeAssistant
participant OnkyoReceiver
User->>HomeAssistant: Configure Onkyo Receiver
HomeAssistant->>OnkyoReceiver: Send configuration settings
OnkyoReceiver-->>HomeAssistant: Acknowledge configuration
User->>HomeAssistant: Adjust settings later
HomeAssistant->>OnkyoReceiver: Reconfigure settings
OnkyoReceiver-->>HomeAssistant: Acknowledge reconfiguration
HomeAssistant->>User: Display updated status
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
source/_integrations/onkyo.markdown (4)
20-21
: Improve sentence structure for better readabilityConsider restructuring the sentence to make it clearer. Here's a suggested improvement:
-The `onkyo` {% term integration %} allows you to control [Onkyo](https://www.onkyo.com) and [Integra](http://www.integrahometheater.com) (from 2011 onward) and also [Pioneer](https://www.pioneerelectronics.com) (from 2016 onward) receivers using Home Assistant. +The `onkyo` {% term integration %} allows you to control the following receivers using Home Assistant: +- [Onkyo](https://www.onkyo.com) receivers +- [Integra](http://www.integrahometheater.com) receivers (2011 and newer) +- [Pioneer](https://www.pioneerelectronics.com) receivers (2016 and newer)
37-37
: Add more context to the reconfiguration noteConsider providing more information about how to access the reconfigure flow.
-The above can also be adjusted later using the **Reconfigure** flow. +The above settings can be adjusted later through the integration's configuration page by clicking the **Configure** button and selecting the **Reconfigure** option.
Line range hint
41-49
: Remove duplicate 'Input sources' configuration entryThe 'Input sources' configuration appears twice in the documentation. The second instance in the options flow configuration block appears to be redundant with the basic configuration block above.
Consider keeping only one instance and clarifying that it can be configured both during initial setup and later through the options flow.
51-54
: Enhance the zones documentation with more detailsWhile the new zones section is helpful, consider adding more specific information about:
- How zones are named in Home Assistant
- Any limitations or differences between main and additional zones
- Examples of how to control different zones
### Zones -If your receiver has second or third zone available, they are displayed as additional media players with a similar functionality as the main zone. +If your receiver has second or third zone available, they are displayed as additional media players in Home Assistant. These additional zones appear as separate entities with names like `media_player.onkyo_zone2` and `media_player.onkyo_zone3`. While they provide similar functionality to the main zone, please note that: + +- Each zone can be controlled independently +- Some features might only be available in the main zone +- Volume and input source selections are zone-specific
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
source/_integrations/onkyo.markdown
(4 hunks)
🔇 Additional comments (1)
source/_integrations/onkyo.markdown (1)
10-10
: LGTM: Integration type update is correct
The change from integration
to device
type is appropriate as this integration primarily manages a physical device.
source/_integrations/onkyo.markdown
Outdated
@@ -37,6 +34,8 @@ Input sources: | |||
type: list | |||
{% endconfiguration_basic %} | |||
|
|||
The above can also be adjusted later using the **Reconfigure** flow. |
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.
This seems odd to mention like this.
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.
I've already had multiple people ask me how to change it, and even make GitHub issues about it. This is in fact the improvement that I most care about, as it should hopefully decrease my support burden.
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.
The problem is: Reconfigure flow, is a developer thing. Nobody knows what this means.
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.
I'm open to different wording on that, if you prefer something else.
It is called Reconfigure
in the UI, so that should be OK, I think, but you are right that flow
is a developer term.
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.
I adjusted the phrasing, borrowing it from the ring
integration docs.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Co-authored-by: Franck Nijhof <frenck@frenck.nl>
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
source/_integrations/onkyo.markdown (2)
51-53
: Consider using more concise wording for zones descriptionConsider using the previously suggested wording for better conciseness:
-If your receiver has a second or third zone available, they are displayed as additional media players with functionality similar to the main zone. +If your receiver has a second or third zone available, they are displayed as additional media players with functionality similar to the main zone.
Line range hint
28-33
: Consider clarifying the two configuration sectionsThe
Input sources
configuration appears in both the initial setup and reconfigure sections. Consider adding a note to clarify that these are the same setting available in different contexts to avoid confusion.Also applies to: 42-47
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
source/_integrations/onkyo.markdown
(4 hunks)
🔇 Additional comments (3)
source/_integrations/onkyo.markdown (3)
10-10
: LGTM: Integration type change is consistent with core PR
The change from integration
to device
matches the update in core PR #117319.
20-20
: LGTM: Clear compatibility requirements
The addition of specific year requirements (2011 for Integra, 2016 for Pioneer) provides helpful clarity for users.
37-37
: LGTM: Helpful clarification for users
The note about the Reconfigure flow is valuable for reducing user confusion and support burden, as justified in the previous discussion.
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
source/_integrations/onkyo.markdown (1)
20-20
: Consider rephrasing for better readability.Consider restructuring the sentence to make it more readable:
-The `onkyo` {% term integration %} allows you to control [Onkyo](https://www.onkyo.com) and [Integra](http://www.integrahometheater.com) (from 2011 onward) and also [Pioneer](https://www.pioneerelectronics.com) (from 2016 onward) receivers using Home Assistant. +The `onkyo` {% term integration %} allows you to control the following receivers using Home Assistant: +- [Onkyo](https://www.onkyo.com) receivers +- [Integra](http://www.integrahometheater.com) receivers (2011 and newer) +- [Pioneer](https://www.pioneerelectronics.com) receivers (2016 and newer)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
source/_integrations/onkyo.markdown
(4 hunks)
🧰 Additional context used
🪛 LanguageTool
source/_integrations/onkyo.markdown
[style] ~37-~37: Consider a more expressive alternative.
Context: ...settings can also be adjusted later. To do this, click the three-dot menu on the i...
(DO_ACHIEVE)
🔇 Additional comments (3)
source/_integrations/onkyo.markdown (3)
10-10
: LGTM! Integration type correctly updated.
The change from integration
to device
type aligns with the parent PR #117319.
37-38
: LGTM! Clear instructions for reconfiguration.
The note provides clear guidance using UI-consistent terminology.
🧰 Tools
🪛 LanguageTool
[style] ~37-~37: Consider a more expressive alternative.
Context: ...settings can also be adjusted later. To do this, click the three-dot menu on the i...
(DO_ACHIEVE)
51-53
: LGTM! Clear explanation of zones functionality.
The new section provides valuable information about multi-zone functionality with clear, concise wording.
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.
Thanks, @arturpragacz 👍
../Frenck
Proposed change
Improve Onkyo documentation
Type of change
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.Summary by CodeRabbit
Max Volume
setting and adjustments to theReconfigure
flow.