-
-
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
Rewrite myuplink docs for new quality scale #36017
Conversation
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 Walkthrough📝 WalkthroughWalkthroughThe documentation for the myUplink integration has been revised to clarify its functionality, emphasizing both information retrieval and control of heat-pump devices. The introduction now explicitly states that the integration allows users to control heat-pump devices, in addition to retrieving information. The process of connecting to the user's account has been detailed, indicating that the integration will download all available data from the API, which can result in the creation of a variable number of entities based on the equipment type. A note has been added to remind users that a valid subscription with myUplink is necessary for controlling equipment through switch, select, and number entities. The prerequisites section has been updated to change terminology from "Application ID" to "Application Name" and "Redirect URI" to "Callback URL," and a new description field for the application has been introduced. New sections have been added, including "Data updates," which specifies that the integration will poll the API every 60 seconds, and "Known limitations," which outlines potential mapping issues with API data to Home Assistant entities and constraints regarding entity name translations. Additionally, a "Troubleshooting" section has been included, providing guidance on common login issues related to application credentials. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant myUplink
participant HomeAssistant
User->>myUplink: Connect to account
myUplink-->>User: Provide access
User->>HomeAssistant: Control heat-pump devices
HomeAssistant->>myUplink: Retrieve device data
myUplink-->>HomeAssistant: Send device information
HomeAssistant->>User: Display device status
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
🧹 Outside diff range and nitpick comments (4)
source/_integrations/myuplink.markdown (4)
33-35
: Consider specifying required subscription typesThe note about subscription requirements is important, but it could be more specific about which subscription types/levels are needed for controlling the equipment.
Consider expanding the note to clarify the subscription requirements, for example:
-You may need a valid subscription with myUplink to control your equipment with switch, select, and number entities. +You may need a valid myUplink subscription (Basic/Premium/etc.) to control your equipment with switch, select, and number entities. The free tier only allows monitoring capabilities.
69-69
: Consider adding rate limit detailsThe polling interval explanation could benefit from mentioning the actual API rate limits to help users understand the constraints.
Consider adding rate limit information:
-The integration will poll the API for data every 5 minutes. This polling interval is designed to work within the rate limits of myUplink APIs while providing timely updates. +The integration will poll the API for data every 5 minutes. This polling interval is designed to work within the myUplink API rate limits (X requests per minute/hour) while providing timely updates.
73-75
: Expand troubleshooting sectionConsider adding more common issues and their solutions to the troubleshooting section.
Consider adding sections for:
- Connection timeout issues
- API errors and their meaning
- Steps to refresh OAuth tokens
- Common entity state issues
79-80
: Enhance issue reporting guidanceThe known limitations section could provide more specific guidance for reporting issues.
Consider adding:
-Please create an issue on GitHub and include a diagnostic download file from your installation if you believe that the mapping can be improved. +Please create an issue on GitHub following these steps: +1. Enable debug logging for the integration +2. Download diagnostic data from your installation +3. Create a new issue using the bug report template +4. Attach the diagnostic file and debug logs +5. Include your heat pump model and firmware version
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Converting back to draft, nothing has been done with the feedback yet. ./klaas |
Sorry, forgot to push the changes. |
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.
Another small adjustment for the removal instructions
Co-authored-by: Klaas Schoute <[email protected]>
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/myuplink.markdown (2)
31-31
: Consider adding supported device informationThe phrase "best effort to map the data-points" could be more specific. Consider listing the officially supported heat pump brands/models, if known, to set clear expectations for users.
- The integration will connect to your account and download all available data from the API. The downloaded information will be used to create devices and entities in Home Assistant. There can be from a few entities up to many hundreds depending on the type of equipment. The integration will make the best effort to map the data-points in the API to sensors, switches, number, and select entities. + The integration will connect to your account and download all available data from the API. The downloaded information will be used to create devices and entities in Home Assistant. There can be from a few entities up to many hundreds depending on the type of equipment. The integration supports the following brands/models: + + - Brand A (Models X, Y, Z) + - Brand B (all models) + + The integration will map the available data-points in the API to Home Assistant entities (sensors, switches, numbers, and selects) based on the device capabilities.
65-65
: Add API rate limit detailsConsider adding specific information about the myUplink API rate limits to help users understand why the 60-second polling interval was chosen.
- The integration will poll the API for data every 60 seconds. This polling interval is designed to work within the rate limits of myUplink APIs while providing timely updates. + The integration will poll the API for data every 60 seconds. This polling interval is designed to work within the myUplink API rate limits (X requests per minute per endpoint) while providing timely updates. This interval ensures stable operation without hitting rate limits that could cause temporary service disruptions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Gemfile.lock
is excluded by!**/*.lock
📒 Files selected for processing (1)
source/_integrations/myuplink.markdown
(2 hunks)
🔇 Additional comments (3)
source/_integrations/myuplink.markdown (3)
42-44
: Document required API permissions/scopes
The prerequisites section should specify which API permissions/scopes users need to select when registering their application.
69-70
: LGTM! Clear and informative limitations section
The known limitations section effectively communicates potential issues and their reasons, particularly regarding entity mapping and translation constraints.
74-82
: LGTM! Well-structured troubleshooting and removal sections
The troubleshooting and removal sections are well-organized using the details block and include clear instructions for credential cleanup.
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.
Thank you, @astrandb 👍
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.
Thnx for the PR! 🥇
./Klaas
Proposed change
Update documentation to take a step towards the ultimate goal - Platinum grade
Type of change
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.Summary by CodeRabbit
Summary by CodeRabbit