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

Alarm volume #250

Merged
merged 25 commits into from
Jun 9, 2021
Merged

Alarm volume #250

merged 25 commits into from
Jun 9, 2021

Conversation

ArnyminerZ
Copy link
Collaborator

@ArnyminerZ ArnyminerZ commented Jun 6, 2021 •
edited
Loading

Added an alarm volume entity.
I've set up it as an entity instead of an attribute, as suggested in #193 so the value can be modified easily through a number entity.

@ArnyminerZ ArnyminerZ added the feature New feature or request label Jun 6, 2021
@ArnyminerZ ArnyminerZ requested a review from DurgNomis-drol June 6, 2021 12:21
@ArnyminerZ ArnyminerZ self-assigned this Jun 6, 2021
@ArnyminerZ
Copy link
Collaborator Author

Still doing some tests to check if everything is set up correctly

@ArnyminerZ ArnyminerZ mentioned this pull request Jun 6, 2021
Closed
@ArnyminerZ ArnyminerZ mentioned this pull request Jun 6, 2021
Open
KapJI
KapJI reviewed Jun 7, 2021
View reviewed changes
custom_components/google_home/api.py Outdated Show resolved Hide resolved
custom_components/google_home/const.py Outdated Show resolved Hide resolved
custom_components/google_home/models.py Outdated Show resolved Hide resolved
custom_components/google_home/number.py Outdated Show resolved Hide resolved
custom_components/google_home/number.py Outdated Show resolved Hide resolved
custom_components/google_home/number.py Outdated Show resolved Hide resolved
custom_components/google_home/number.py Outdated Show resolved Hide resolved
leikoilja
leikoilja reviewed Jun 7, 2021
View reviewed changes
Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

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

Awesome stuff, thank you @ArnyminerZ 🚀

custom_components/google_home/api.py Outdated Show resolved Hide resolved
@leikoilja leikoilja linked an issue Jun 7, 2021 that may be closed by this pull request
Change alarm volume #192
Closed
@KapJI
Copy link
Collaborator

KapJI commented Jun 8, 2021 •
edited
Loading

There is some pylint error. Probably just disable duplicate-code check in pyproject.toml.

ArnyminerZ and others added 2 commits June 8, 2021 21:01
@ArnyminerZ
Disabled duplicate-code
357422b
@ArnyminerZ
Update api.py
a9ba6dd 
Not sure if this is necessary, but added just in case
@KapJI
Copy link
Collaborator

KapJI commented Jun 9, 2021

This needs to be documented in README.

@ArnyminerZ
Update README.md
bcf2734 
Added the Numbers section, not sure if it's a good name, but it's the one that Home Assistant gives so... 🤷🏼
KapJI
KapJI approved these changes Jun 9, 2021
View reviewed changes
Copy link
Collaborator

@KapJI KapJI left a comment

Choose a reason for hiding this comment

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

Awesome, thanks! 🎉

@KapJI
Copy link
Collaborator

KapJI commented Jun 9, 2021

There is a merge conflict which needs to be resolved.

@KapJI
Copy link
Collaborator

KapJI commented Jun 9, 2021

The indices are shifted a bit 🙂

@ArnyminerZ
Copy link
Collaborator Author

OMG this check is driving me crazy, so picky 😂

@ArnyminerZ
Update README.md
36a1f78 
Fixed indices
@KapJI KapJI merged commit 2427916 into leikoilja:master Jun 9, 2021
@ArnyminerZ ArnyminerZ deleted the alarm_volume branch June 9, 2021 17:43
@leikoilja
Copy link
Owner

@ArnyminerZ, are you using pre-commit hooks locally (they run the same checks, but locally on your computer)? They can really help to quickly scan the code even before committing it :)

@ArnyminerZ
Copy link
Collaborator Author

@ArnyminerZ, are you using pre-commit hooks locally (they run the same checks, but locally on your computer)?

Yes I do, but my WSL machine is giving some problems, so some of them had to be manually fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leikoilja leikoilja leikoilja left review comments

@KapJI KapJI KapJI approved these changes

@DurgNomis-drol DurgNomis-drol Awaiting requested review from DurgNomis-drol

Assignees

@ArnyminerZ ArnyminerZ

Labels
feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change alarm volume
3 participants
@ArnyminerZ @KapJI @leikoilja