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.

Sign up for GitHub

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

Jump to bottom

Add start/stop auxiliary-heating and verify_spin support #175

Merged
merged 4 commits into from
Nov 10, 2024
Merged

Add start/stop auxiliary-heating and verify_spin support #175

merged 4 commits into from
Nov 10, 2024

Conversation

prvakt
Copy link
Collaborator

@prvakt prvakt commented Nov 8, 2024

  • Added auxiliary heater support (start/stop). For some reason progress is not published via MQTT so request is just send and not awaited for COMPLETED_SUCCESS status
  • Added verify_spin method which returns CORRECT_SPIN or INCORRECT_SPIN and S-Pin status

dvx76
dvx76 reviewed Nov 8, 2024
View reviewed changes
myskoda/models/spin.py Outdated Show resolved Hide resolved
WebSpider
WebSpider requested changes Nov 8, 2024
View reviewed changes
Copy link
Contributor

@WebSpider WebSpider left a comment

Choose a reason for hiding this comment

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

Thanks for your PR, much appreciated!

Looks good overall, some minor remarks.

myskoda/models/spin.py Outdated Show resolved Hide resolved
myskoda/rest_api.py Show resolved Hide resolved
myskoda/rest_api.py Outdated Show resolved Hide resolved
myskoda/models/spin.py Outdated Show resolved Hide resolved
@prvakt prvakt requested a review from WebSpider November 8, 2024 16:22
dvx76
dvx76 previously approved these changes Nov 8, 2024
View reviewed changes
Copy link
Member

@dvx76 dvx76 left a comment

Choose a reason for hiding this comment

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

LGTM

@dvx76 dvx76 mentioned this pull request Nov 8, 2024
Closed
@prvakt
Copy link
Collaborator Author

prvakt commented Nov 8, 2024

please wait with approval, planning to test something tomorrow because maybe we are using wrong endpoint for the auxiliary-heating MQTT subscription (#179)

@WebSpider WebSpider marked this pull request as draft November 8, 2024 22:48
@WebSpider
Copy link
Contributor

Converted to draft PR, so it won't be merged by accident.

@prvakt prvakt marked this pull request as ready for review November 9, 2024 08:14
@prvakt
Copy link
Collaborator Author

prvakt commented Nov 9, 2024

Good news, after changing mqtt endpoint for auxiliary-heater as suggested in (#179), I can receive state updates.
This PR is finished and can be merged ;)

WebSpider
WebSpider approved these changes Nov 9, 2024
View reviewed changes
Copy link
Contributor

@WebSpider WebSpider left a comment

Choose a reason for hiding this comment

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

Lgtm

@dvx76 dvx76 merged commit 1bdf988 into skodaconnect:main Nov 10, 2024
2 checks passed
@WebSpider WebSpider added the enhancement New feature or request label Nov 11, 2024
zaptm pushed a commit to zaptm/myskoda that referenced this pull request Dec 8, 2024
@prvakt @zaptm
Add start/stop auxiliary-heating and verify_spin support (skodaconnec…
cc47d81 
…t#175)

* Add start/stop auxiliary-heating and verify_spin

* fixed remarks + implemented temperature rounding to 0,5 step

* fixed MQTT endpoint for auxiliary-heating

* added missing comma

---------

Co-authored-by: Tomas Prvak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dvx76 dvx76 dvx76 approved these changes

@WebSpider WebSpider WebSpider approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@prvakt @WebSpider @dvx76