-
Notifications
You must be signed in to change notification settings - Fork 27
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
#79 Add 'COOLING' state to AirConditioningState #80
#79 Add 'COOLING' state to AirConditioningState #80
Conversation
Thanks for your contribution! 🙏 |
@@ -26,7 +26,7 @@ class AirConditioningState(StrEnum): | |||
OFF = "OFF" |
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.
I also like when items are sorted alphabetically, it is easier to find the value and catch duplicates (may be not valid concern for such short enum but just to be consistent).
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.
Thanks for the suggestion! It seems practical, and I will definitely keep it in mind. I’ve made the change while fixing the other issue.
Would be also nice to have a test case for this value. Please, have a look on |
hey, i've fixed the linting issue. |
I will look into it. I wanted to confirm where the corresponding JSON file should be added. Should it go under enyaq/, superb/, or other/? |
Depends on the car model for which you have your response. E.g. I have superb, so I add my jsons to superb folder. If you do not know the car model, put to other. |
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.
Lgtm
Thanks for your contribution 👍 |
This PR adds the missing 'COOLING' state to the AirConditioningState enum in response to issue #79. The issue reported a ValueError due to the missing 'COOLING' state, and I've updated the enum to include this state.
This is my first open source contribution, and I'm excited to be part of the community. Please feel free to provide any feedback or guidance if there's anything I can improve. Thanks.