Documnetation: snapshot FAQ and examples #493
Conversation
doc/faq.rst
Outdated
|
|
||
| SoCo provides a snapshot module that captures the current state of a player and | ||
| then when requested re-instates that state. Examples of it's use are: | ||
| - `basic snap example <https://github.com/SoCo/SoCo/blob/master/examples/snapshot/basic_snap.py>`_ |
There was a problem hiding this comment.
I think the content looks good.
There is a minor syntax error. You should make a blank line after the colon and don't indent the list and I think use * as lists, so it becomes:
SoCo provides a snapshot module that captures the current state of a player and
then when requested re-instates that state. Examples of it's use are:
* `basic snap example <https://github.com/SoCo/SoCo/blob/master/examples/snapshot/basic_snap.py>`_
* `multi zone example <https://github.com/SoCo/SoCo/blob/master/examples/snapshot/multi_zone_snap.py>`_If you have sphinx and napoleon installed you can test the compilation by doing "make html" in the doc directory.
examples/snapshot/basic_snap.py
Outdated
| Note: the snap function is designed to be used once, Having taken a snap to | ||
| take another re-instantiate the class. | ||
| """ | ||
|
|
There was a problem hiding this comment.
A comment about py2 and 3 compatible code. I guess this was developed on Py3 as you use print as a function. However, the print() only work like it is supposed to in Py2, if there is only one argument. If doing print("more", "strings") it will print out ("more", "strings"). The proper way to do it is to start the file with: from __future__ import print_function. Then print will actually be a function also in Py2.
While you are at it, you might as well also add from __future__ import print_function, unicode_literals. Then you will not need to preface your strings with u to make sure they are unicode.
examples/snapshot/basic_snap.py
Outdated
| from soco.snapshot import Snapshot | ||
|
|
||
| # something to play on a Sonos player to start (a radio station) | ||
| start_uri = u'x-sonosapi-stream:s2846?sid=254&flags=32' |
There was a problem hiding this comment.
Here, remove the u after having imported unicode_literals
examples/snapshot/basic_snap.py
Outdated
| - play an alert mp3 for a few seconds (a poem!) | ||
| - re-instate the sonos player to it's previous state (the radio station) | ||
|
|
||
| Note: the snap function is designed to be used once, Having taken a snap to |
There was a problem hiding this comment.
3 things:
Capital after ":"
Calling if function might confuse since it isn't actually a function, so either change to functionality or say that the "Snapshot instance" is only designed to be used once.
"once, Having", change to full stop or make lowercase. Also the last part if that sentence is maybe a little to compactly formulated. What do you think?
examples/snapshot/multi_zone_snap.py
Outdated
| This script does not group or un-group any Sonos players. Group management is | ||
| a separate subject. For alerts grouping causes delays, so is avoided here. | ||
|
|
||
| Note: the snap function is designed to be used once, Having taken a snap to |
examples/snapshot/multi_zone_snap.py
Outdated
|
|
||
| def play_alert(alert_uri, alert_volume=20, alert_duration=0, fade_back=False): | ||
| """ | ||
| Demo function using soco.snapshot accrss multiple Sonos players. |
examples/snapshot/multi_zone_snap.py
Outdated
| """ | ||
| Demo function using soco.snapshot accrss multiple Sonos players. | ||
|
|
||
| :param alert_uri: file that Sonos can play as an alert |
There was a problem hiding this comment.
Consider using the Napoleon style for sphinc docstrings, which we are trying to switch to in general. Let me know if you need help.
examples/snapshot/multi_zone_snap.py
Outdated
| for zone in zones: | ||
| if zone.is_coordinator: | ||
| zone.play_uri(uri=alert_uri, title='Sonos Alert') | ||
| a_coordinator = zone # remember last coordinator for use next |
There was a problem hiding this comment.
I can't seem to see this being used.
examples/snapshot/multi_zone_snap.py
Outdated
| from soco.snapshot import Snapshot | ||
|
|
||
|
|
||
| def play_alert(alert_uri, alert_volume=20, alert_duration=0, fade_back=False): |
There was a problem hiding this comment.
I'd maybe pass zones in as a parameter. What do you think.
|
Have updated as suggested. Not sure if I have the Napoleon structure right in the multi_zone_snap.py example. Welcome any further comments Cheers Daivd |
|
Looks good. There are still a lot of pylint warnings, but they seem to be caused by a new version of pylint, which included new checks. I'll see about fixing those tonight. |
|
Merged. Can you add a comment about the changes to the release notes #440? |
A documentation pull request as discussed - an area with frequent question.
Please check I have changed the faq.rst correctly - not used that before.
Added 2 examples of using snapshot - tested on python 2.7 and 3.5 - feel free to recommend changes.
Cheers David