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.

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

Add group label properties #228

Merged
merged 3 commits into from
Oct 7, 2014
Merged

Add group label properties #228

merged 3 commits into from
Oct 7, 2014

Conversation

dajobe
Copy link
Contributor

@dajobe dajobe commented Sep 20, 2014

Making the unit tests work here required a bit of thought. The group member zgs responses had to be mocked after the group XML had been parsed, so it's done in the test code.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.33%) when pulling b0b3648 on dajobe:group-labels into 6f318df on SoCo:master.

@stefankoegl
Copy link
Member

Nice work! Should be ready to be merged right after 0.9 is out.

def short_label(self):
""" A short description of the group """
group_names = sorted([m.player_name for m in self.members])
group_label = group_names[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this how the official controllers name groups? Perhaps they use the name of the group coordinator, rather than the first in alphabetical order, i.e. group_label = self.coordinator.player_name ... etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lawrenceakka from what I can tell it's alphabetical; that gives a consistent name even if the coordinator isn't the first by name. It always seems to match for me, I've been using it for a few weeks.

@KennethNielsen
Copy link
Member

Looks good
+1

@stefankoegl stefankoegl added this to the 0.10 milestone Sep 27, 2014
@stefankoegl
Copy link
Member

It seems that there are some conflicts. Can you please rebase onto master?

Conflicts:
	unittest/test_core.py
@dajobe
Copy link
Contributor Author

dajobe commented Sep 27, 2014

@stefankoegl rebased as requested

@coveralls
Copy link

Coverage Status

Coverage increased (+0.32%) when pulling 6392501 on dajobe:group-labels into cb31b09 on SoCo:master.

lawrenceakka added a commit that referenced this pull request Oct 7, 2014
@lawrenceakka lawrenceakka merged commit 020c4a2 into SoCo:master Oct 7, 2014
@lawrenceakka lawrenceakka mentioned this pull request Oct 7, 2014
dajobe added a commit to dajobe/sonos-utils that referenced this pull request Oct 7, 2014
@dajobe dajobe deleted the group-labels branch January 18, 2015 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants