Skip to content
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

Add description for Stookwijzer get_forecast service #37476

Draft
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

fwestenberg
Copy link
Contributor

@fwestenberg fwestenberg commented Feb 15, 2025

Proposed change

Add a description of the new service get_forecast for the Stookwijzer integration.

Type of change

  • Spelling, grammar or other readability improvements (current branch).
  • Adjusted missing or incorrect information in the current documentation (current branch).
  • Added documentation for a new integration I'm adding to Home Assistant (next branch).
  • Added documentation for a new feature I'm adding to Home Assistant (next branch).
  • Removed stale or deprecated documentation.

Additional information

Checklist

  • This PR uses the correct branch, based on one of the following:
    • I made a change to the existing documentation and used the current branch.
    • I made a change that is related to an upcoming version of Home Assistant and used the next branch.
  • The documentation follows the Home Assistant documentation standards.

Summary by CodeRabbit

  • New Features
    • Introduced a forecast retrieval action for the Stookwijzer integration that delivers advice forecasts.
  • Documentation
    • Enhanced integration guides with detailed field descriptions, usage examples, and sample outputs illustrating how to access forecast data for specified time intervals.
    • Added a new section for template sensor examples utilizing the get_forecast action.

@home-assistant home-assistant bot added current This PR goes into the current branch has-parent This PR has a parent PR in a other repo in-progress This PR/Issue is currently being worked on needs-rebase The PR has been branched of the wrong base branch or targets an incorrect target branch labels Feb 15, 2025
@home-assistant
Copy link

It seems that this PR is targeted against an incorrect branch since it has a parent PR on one of our codebases. Documentation that needs to be updated for an upcoming release should target the next branch. Please change the target branch of this PR to next and rebase if needed.

Copy link

netlify bot commented Feb 15, 2025

Deploy Preview for home-assistant-docs ready!

Name Link
🔨 Latest commit 855a8f3
🔍 Latest deploy log https://app.netlify.com/sites/home-assistant-docs/deploys/67bf7de011106b000854eee7
😎 Deploy Preview https://deploy-preview-37476--home-assistant-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@fwestenberg fwestenberg changed the base branch from current to next February 15, 2025 09:18
@home-assistant home-assistant bot removed current This PR goes into the current branch needs-rebase The PR has been branched of the wrong base branch or targets an incorrect target branch labels Feb 15, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (2)
source/_integrations/stookwijzer.markdown (2)

46-54: Minor Style Suggestion for Response Data Description
The response data table is comprehensive; however, the phrase “at a given moment in time” (line 47) feels a bit redundant. Consider simplifying it to “at a given time” for brevity and clarity.

Below is a suggested diff to implement this change:

-`forecast` is a list of forecast advice entries at a given moment in time:
+`forecast` is a list of forecast advice entries at a given time:
🧰 Tools
🪛 LanguageTool

[style] ~47-~47: This phrase is redundant. Consider writing “moment” or “time”.
Context: ...t of forecast advice entries at a given moment in time: | Response data | Descr...

(MOMENT_IN_TIME)


92-110: Consistency Check on Data Types in Action Response Example
The example action response shows the final field as a string (e.g., "True", "False"). Please verify whether the service actually returns boolean values. If the intended data type is boolean, consider updating the YAML to use unquoted booleans (e.g., true/false) for consistency.

If you decide to change this for clarity, here’s a diff suggestion:

-    final: "True"
+    final: true
...
-    final: "True"
+    final: true
...
-    final: "False"
+    final: false

Adjust the quotes on the final entries accordingly in the example action response.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a27ed17 and aa912d3.

📒 Files selected for processing (1)
  • source/_integrations/stookwijzer.markdown (1 hunks)
🧰 Additional context used
🪛 LanguageTool
source/_integrations/stookwijzer.markdown

[style] ~47-~47: This phrase is redundant. Consider writing “moment” or “time”.
Context: ...t of forecast advice entries at a given moment in time: | Response data | Descr...

(MOMENT_IN_TIME)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Redirect rules - home-assistant-docs
  • GitHub Check: Header rules - home-assistant-docs
  • GitHub Check: Pages changed - home-assistant-docs
🔇 Additional comments (3)
source/_integrations/stookwijzer.markdown (3)

33-38: Clear Documentation for New Action
The new section titled Action stookwijzer.get_forecast is clear and descriptive. It effectively introduces the service action and explains its purpose, linking to the relevant documentation for handling response data.


39-44: YAML Snippet Structure and Clarity
The YAML snippet detailing the action’s structure is well-formatted and easy to follow. It correctly specifies the keys (action, target with config_entry_id, and response_variable).


55-91: Well-Constructed Example for Template Sensor
The examples provided for the template sensors are very clear and demonstrate how to utilize the stookwijzer.get_forecast action effectively. The use of the {% raw %} and detailing blocks ensures that the YAML is displayed exactly as intended, maintaining consistency with Home Assistant’s documentation style.

c0ffeeca7
c0ffeeca7 previously approved these changes Feb 26, 2025
Copy link
Contributor

@c0ffeeca7 c0ffeeca7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @fwestenberg 👍

@c0ffeeca7 c0ffeeca7 added the awaits-parent Awaits the merge of an parent PR label Feb 26, 2025
@abmantis abmantis changed the title Add-Stookwijzer-forecast-service-description Add description for Stookwijzer get_forecast service Feb 26, 2025
@home-assistant home-assistant bot marked this pull request as draft February 26, 2025 15:20
@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@fwestenberg fwestenberg marked this pull request as ready for review February 26, 2025 20:54
@home-assistant home-assistant bot requested a review from abmantis February 26, 2025 20:54
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (3)
source/_integrations/stookwijzer.markdown (3)

33-37: New Forecast Action Documentation Section

The newly added "Action: Get Forecast" section is well-structured and aligns with the existing documentation style. It clearly introduces the service action that populates response data with forecast advice.

Suggestion (Good to Have): If possible, include a brief note about the expected format or constraints for the target field (e.g., whether it accepts only a specific type of entity ID or other identifier).


49-53: Response Data Table Readability

The response data table is informative and clearly outlines each field of the returned forecast data. Consider rephrasing the description for the final field to improve clarity, for example: "Indicates if the advice is final or subject to change."


75-112: Template Sensor Examples Clarity

The "Examples" section, which illustrates how to integrate the get_forecast action into a template sensor, is well documented. The use of a {% raw %} block is appropriate to prevent premature template evaluation.

Suggestion (Good to Have): Since the examples access specific list indexes (e.g., [0] and [1]), it might be helpful to include a brief note reminding users to ensure that the forecast list contains enough items to avoid index errors.

🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

112-112: Multiple consecutive blank lines
Expected: 1; Actual: 2

(MD012, no-multiple-blanks)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26e870a and 855a8f3.

⛔ Files ignored due to path filters (1)
  • Gemfile.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • source/_integrations/stookwijzer.markdown (1 hunks)
🔇 Additional comments (2)
source/_integrations/stookwijzer.markdown (2)

39-44: YAML Example for the get_forecast Action

The YAML snippet clearly demonstrates how to invoke the stookwijzer.get_forecast action with the required keys (action, target, and response_variable). The example is concise and helpful.


55-73: Example Action Response Block

The example action response block neatly shows the expected structure of the forecast result, including multiple forecast entries with their respective timestamps, advice codes, and finality indicators. This concrete example should help users understand the data format being returned.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please undo the changes to this file

@home-assistant home-assistant bot marked this pull request as draft February 26, 2025 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaits-parent Awaits the merge of an parent PR has-parent This PR has a parent PR in a other repo in-progress This PR/Issue is currently being worked on next This PR goes into the next branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants