Only on master

[KennethNielsen]

This is my attempt to solve the problem of calling master-only methods on a slave. It fixes this along the lines of option 2 as discussed here #138 (comment) by @lawrenceakka (Raises SoCoSlaveException when calling a master command on a slave). It is implemented as a decorator should be used to decorate all the master only methods in SoCo (of course it might it might turn out to be better to reverse the logic if there are many more master-only than master-and-slave method).

Right now it is only implemented on the play and pause method, because I wanted to get some input about whether you like this approach or not, before I start testing all the methods in SoCo for whether they can be called on a slave.

Please also have a look at the test modifications. This is my first run-in with mock (I spent about twice the time on the test modifications as I did on the implementation), so I'm very curious if there is a better way to adapt the tests for this change.

EDIT: I improved the test modifications, so that they actually are still unit tests, so it should be better now, but still could use a review.

[KennethNielsen]

I improved the test changes, so that they actually are still unit tests.

I'd like some feedback on this, if anyone has a little time (I know it can be tight).

[stefankoegl]

I like the approach! From my perspective we can continue like this.

[KennethNielsen]

Ok, I have now applied the decorator to all the methods that I think it should be used on. Please test it and let me know what you think.

A few specifics of the application of the decorator below:

For cross_fade the getter works and returns the value for the slave, but the setter fails. Since it would be required to apply the decorator to the setter I decided to also apply it to the getter to not break the symmetry.

play_uri will add the uri to the slave and then break the group and ultimately fail because we cannot call play, therefore mark the entire method as only_on_master

stop, stops playback on the slave, but without breaking the group and the master still plays?!?, also mark that as only_on_master

I think the above cases of somewhat inconsistent or unexpected behavior seems to me to support our idea of trying to catch this incorrect use before it is sent to the player.

When methods calls other methods for the actual interaction, I have chosen to mark both (e.g. add_uri_to_queue and add_to_queue) because the implementation is hidden from normal use and the error message will then mention another method, which will be confusing (and if the implementation is changed later, the information from my testing will not be lost).

add_item_to_queueable_item seems to have been forgotten in the data structures restructuring, at least it tries to form metadata in the old way. Since it is broken I cannot test it. I will open a separate issue for that.

[stefankoegl]

I think the above cases of somewhat inconsistent or unexpected behavior seems to me
to support our idea of trying to catch this incorrect use before it is sent to the player.

Agreed

Good catch regarding add_item_to_queueable_item (#308)!

+1 to the whole PR -- nice work :)

[KennethNielsen]

I seem to have messed up the rebase, please hold

[stefankoegl]

Seems to be fixed now, right?

[KennethNielsen]

Yes. I'll merge.