chime: Support playing motion test sound#46
Conversation
Changes the test_sound property to be a function. Fixes Issue python-ring-doorbell#28.
|
@vickyg3, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tchellomello to be a potential reviewer. |
2 similar comments
| # chime test sound kinds | ||
| KIND_DING = 'ding' | ||
| KIND_MOTION = 'motion' | ||
| CHIME_TEST_SOUND_KINDS = (KIND_DING, KIND_MOTION) |
There was a problem hiding this comment.
Let's make a list instead a map
There was a problem hiding this comment.
I'm sorry, but can you please explain what the key and value of the map would be?
There was a problem hiding this comment.
CHIME_TEST_SOUND_KINDS = [KIND_DING, KIND_MOTION]There was a problem hiding this comment.
Why does it have to be a list? What i've used is a tuple which is basically an immutable list (not a map). Which is exactly what we need here.
| return False | ||
| url = API_URI + TESTSOUND_CHIME_ENDPOINT.format(self.account_id) | ||
| self._ring.query(url, method='POST') | ||
| self._ring.query(url, method='POST', extra_params={"kind": kind}) |
There was a problem hiding this comment.
Shouldn't this be expanded to the "ringtone" the user selected to play?
There was a problem hiding this comment.
It probably should be. But i'll leave that to a follow-up. Don't have my phone set-up to do the proxy again right now. :)
There was a problem hiding this comment.
Yes make sense.. You got a good point!!
Changes the test_sound property to be a function.
Fixes Issue #28.
Please let me know if this change needs to be backward compatible. I can make it another property in that case. (Also thanks for the pointer on mitmproxy, worked like a charm!)