-
Notifications
You must be signed in to change notification settings - Fork 7.2k
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 documentation for renault.ac set schedules #34636
base: next
Are you sure you want to change the base?
Add documentation for renault.ac set schedules #34636
Conversation
WalkthroughWalkthroughThe pull request introduces a new action, Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional context usedLanguageTool
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 (
|
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Did a fuck-up in my previous PR: #34521 |
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, codebase verification and nitpick comments (1)
source/_integrations/renault.markdown (1)
Line range hint
91-120
: Review of Modified Actionrenault.charge_set_schedules
The modifications to the
renault.charge_set_schedules
action align it with the newrenault.ac_set_schedules
action, which is a good practice for consistency. The updated documentation clarifies the structure of theschedules
attribute and the requirements for themonday
tosunday
elements.Suggestions:
- Clarify the
startTime
andduration
keys: The documentation should specify that bothstartTime
andduration
are required if any day is specified. This ensures that users do not omit these crucial pieces of information.- Example Consistency: The example provided shows how to set schedules but consider adding more varied examples to cover different scenarios, such as setting schedules for multiple days or using the
None
value to clear settings.Code Example Validation:
- The YAML example should be tested to ensure it accurately reflects the described functionality and works as expected in the Home Assistant environment.
### Action `renault.ac_set_schedules` | ||
|
||
Update AC schedule on vehicle. | ||
|
||
| Data attribute | Required | Description | Example | | ||
| ---------------------- | -------- | ----------- | ------- | | ||
| `vehicle`| yes | device_id of the vehicle | | | ||
| `schedules` | yes | Schedule details. Can be a single schedule or a list of schedules | see [example below](#ac_schedule_example) | | ||
|
||
Notes: | ||
|
||
- `schedules` can contain one or more schedules which are set within the same call | ||
- The `id` is compulsory on each `schedule` (should be 1 to 5 depending on the vehicle). | ||
- The `activated` flag is an optional boolean. If it is not provided, then the existing flag will be kept as is. | ||
- The `monday` to `sunday` elements are optional. If they are not provided, then the existing settings will be kept for each day. If they are provided as None, then the existing setting will be cleared. If a value is provided, it must contain the key `readyAtTime` (in UTC format). | ||
|
||
<a name="ac_schedule_example">Example</a>: | ||
|
||
```yaml | ||
- id: 1 | ||
activated: true | ||
monday: | ||
readyAtTime: 'T12:00Z' | ||
- id: 2 | ||
activated: false | ||
monday: | ||
readyAtTime: 'T12:00Z' | ||
tuesday: | ||
readyAtTime: 'T12:00Z' | ||
``` |
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.
Review of Action renault.ac_set_schedules
The documentation for the new action renault.ac_set_schedules
is comprehensive and well-structured. The table format is consistent with other actions in the document, and the example provided is clear and illustrative of the functionality.
Suggestions:
- Clarify the
readyAtTime
format: While it's mentioned thatreadyAtTime
should be in UTC format, it might be helpful to specify that this should be in ISO 8601 format to avoid any confusion. - Expand on the
activated
flag: The description mentions that if theactivated
flag is not provided, the existing flag will be kept as is. It might be beneficial to clarify what the default state is if this is the first time the schedule is being set.
Grammar Correction:
- Line 68 has a minor grammatical issue as pointed out by LanguageTool. Consider rephrasing to: "Schedule details can include either a single schedule or a list of schedules."
Code Example Validation:
- The YAML example is correctly formatted and matches the described functionality. Ensure that the example is tested to confirm that it works as expected in the Home Assistant environment.
Tools
LanguageTool
[style] ~68-~68: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ...|schedules
| yes | Schedule details. Can be a single schedule or a list of sched...(MISSING_IT_THERE)
@epenet Could you review this PR ? |
Proposed change
This PR adds documentation for the new service that is implemented by home-assistant/core#125006
Type of change
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.Summary by CodeRabbit