Add service to set the AC schedule of renault vehicles #125006
Conversation
|
Hey there @epenet, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
Should I create the PR for the documentation update to the 'current' or 'next' branch ? Made it for 'next', hope it is correct |
| mock_call_data: list[ChargeSchedule] = mock_action.mock_calls[0][1][0] | ||
| assert mock_action.mock_calls[0][1] == (mock_call_data,) |
There was a problem hiding this comment.
I think this is wrong (it might be wrong above too) as it's not really testing the values passed.
It's just testing that A == A, when it should be testing that A == expected
Could we use a snapshot assertion here instead?
| mock_call_data: list[ChargeSchedule] = mock_action.mock_calls[0][1][0] | |
| assert mock_action.mock_calls[0][1] == (mock_call_data,) | |
| mock_call_data: list[ChargeSchedule] = mock_action.mock_calls[0][1][0] | |
| assert mock_call_data == snapshot |
There was a problem hiding this comment.
Note: I have opened #125064 for the bad existing tests.
You will probable need to rebase once that is merged.
Yes, it must be |
| @@ -174,7 +174,7 @@ | |||
| }, | |||
| "ac_cancel": { | |||
| "name": "Cancel A/C", | |||
| "description": "Canceles A/C on vehicle.", | |||
| "description": "Cancels A/C on vehicle.", | |||
There was a problem hiding this comment.
This could have been a separate bugfix PR 😉 but I think it's ok to leave it in here.
| schedules: | ||
| example: >- | ||
| [ | ||
| { | ||
| "id": 1, | ||
| "activated": false | ||
| }, | ||
| { | ||
| "id": 2, | ||
| "activated": true, | ||
| "monday": { "readyAtTime": "T20:45Z" }, | ||
| "sunday": { "readyAtTime": "T20:45Z" } | ||
| }, | ||
| { | ||
| "id": 3, | ||
| "activated": false | ||
| }, | ||
| { | ||
| "id": 4, | ||
| "activated": false | ||
| }, | ||
| { | ||
| "id": 5, | ||
| "activated": false | ||
| } | ||
| ] |
There was a problem hiding this comment.
We should not create services that require the user to put in JSON objects. This is too error prone imo
There was a problem hiding this comment.
I changed both examples in the file. Also updated the documentation.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
@joostlek Can you please review ? |
…t#125006) * Add service to set the AC schedule of renault vehicles * Remove executable permission * Applied review comments (use snapshot) * Rewrote examples to not use JSON
Breaking change
Proposed change
This PR adds the functionality to set the AC schedule of renault vehicles.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: