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

Cleaned up version of #558 - Add avahi publishing of MPD and HTTP server endpoints. #573

Merged
merged 15 commits into from
Nov 11, 2013

Conversation

adamcik
Copy link
Member

@adamcik adamcik commented Nov 11, 2013

No description provided.

eisnerd and others added 13 commits November 1, 2013 10:17
A rudimentary implementation to resolve mopidy#39, ignoring dbus errors (just restart), name collisions (choose a fresh name), etc.
- Prepare for handling missing dbus directly in module.
- Constants should always be all caps.
- Extracted helpers from class to convey intent via their naming.
- Moved imports out of class, imports should always be on the top.
- Made sure calling publish mutiple times does not re-convert text.
- Made sure calling unpublish without publish does not break.
- Handle dbus not being installed.
- Handle not finding the system bus.
- Handle not finding the avahi service.
- Return if publish succeeded.
- Move imports to top as they should be.
- Report state based on publish return value.
- Remove overly broad except clauses.
- Unpublish before stopping servers.
def on_start(self):
if self.config_section['zeroconf']:
self.zeroconf_service = zeroconf.Zeroconf(
stype="_mpd._tcp", name=self.config_section['zeroconf'],
Copy link
Member

Choose a reason for hiding this comment

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

Use single quotes.

@jodal
Copy link
Member

jodal commented Nov 11, 2013

+1 with comments

- Switched to newstyle class
- Switched to safe values in kwargs
@adamcik
Copy link
Member Author

adamcik commented Nov 11, 2013

Found some more things that needed fixing, but should be good now.

@@ -17,6 +17,10 @@ def __init__(self, config, core):
super(MpdFrontend, self).__init__()
hostname = network.format_hostname(config['mpd']['hostname'])
port = config['mpd']['port']
self.config_section = config['mpd']
Copy link
Member

Choose a reason for hiding this comment

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

Might it be neater to do this first and use self.config_section to get the host and port above (like the http frontend does)? In fact, why not just assign straight to self.hostname and self.port while you're at it?

jodal added a commit that referenced this pull request Nov 11, 2013
Cleaned up version of #558 - Add avahi publishing of MPD and HTTP server endpoints.
@jodal jodal merged commit d723db8 into mopidy:develop Nov 11, 2013
@adamcik adamcik deleted the feature/avahi branch November 16, 2013 13:07
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.

4 participants