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

Zeroconf and filesystem browser #256

Closed
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@eisnerd

eisnerd commented Aug 10, 2013

The main changes in my mergable branch relate to #249 and touches on #30. I'm sure this wasn't quite where you were going with this, but I've added, primarily for my own use, some quick access to the zeroconf/bonjour/avahi activity. I find this a fairly quick way of operating multiple mpd servers from MPDroid. Even without storing or simultaneously using multiple connection settings, switching is quick and easy enough to provide a nice stopgap.

I use a script that I currently call mpd-enslaver to synchronise playlists and play states between mpd servers, and have another branch with functions particular to that setup. Those probably aren't suitable for general use, however, as the server setup is quite convoluted.

@abarisain

This comment has been minimized.

Show comment
Hide comment
@abarisain

abarisain Sep 1, 2013

Owner

Hmm, I'm going to have to take a better look at it before I merge it. It does make some UX changes I'm not sure about or at least make them optional (since most do not care about Zeroconf).
The included various fixes (Travis, performance, strverscmp) are really welcomed :)

Thanks !

Owner

abarisain commented Sep 1, 2013

Hmm, I'm going to have to take a better look at it before I merge it. It does make some UX changes I'm not sure about or at least make them optional (since most do not care about Zeroconf).
The included various fixes (Travis, performance, strverscmp) are really welcomed :)

Thanks !

@eisnerd

This comment has been minimized.

Show comment
Hide comment
@eisnerd

eisnerd Sep 2, 2013

Understood. I can separate out the misc fixes if necessary.

The server switch action bar item could do with being optional. However, wrt. zeroconf in general I would point out that most MPD users currently have to be fairly comfortable with long text config files for MPD itself, nfs/samba, etc. and be at ease typing IP addresses into all their clients. Whilst I fit into that category really, I would like to see open source network music systems giving sonos, airplay, etc. some serious competition. Projects like raspyfi and having zeroconf support in all major clients could open MPD up to a much wider audience.

Most distributions of MPD have zeroconf, albeit with some naff name, enabled by default, so a first use of MPDroid would now involve 1) starting the app, 2) noticing your MPD server already in the list presented to you and 3) being connected in one touch. Even if the UI could do with further attention, this is a massively better UX than being thrown into the settings to type long numbers on an awkward keyboard, numbers which many potential MPD users (the sort who might be impressed by someone's MPD system enough to go out and by something, but end up with Sonos because they'd be out of their depth) would have to look up or ask their networking literate friend to find, not knowing (the fools) such things of the top of their heads.

The second purpose to zeroconf support in an MPD client would be easy multi-room support, a feature that there has definitely been some interest in. The MPD server doesn't really help with this, but a simple setup of separate MPD servers sharing a music collection can be controlled with minimal fuss using the zeroconf activity, whereas manual configuration for this would not only be tedious but require settings management UI that we do not currently have.

Kind regards.

eisnerd commented Sep 2, 2013

Understood. I can separate out the misc fixes if necessary.

The server switch action bar item could do with being optional. However, wrt. zeroconf in general I would point out that most MPD users currently have to be fairly comfortable with long text config files for MPD itself, nfs/samba, etc. and be at ease typing IP addresses into all their clients. Whilst I fit into that category really, I would like to see open source network music systems giving sonos, airplay, etc. some serious competition. Projects like raspyfi and having zeroconf support in all major clients could open MPD up to a much wider audience.

Most distributions of MPD have zeroconf, albeit with some naff name, enabled by default, so a first use of MPDroid would now involve 1) starting the app, 2) noticing your MPD server already in the list presented to you and 3) being connected in one touch. Even if the UI could do with further attention, this is a massively better UX than being thrown into the settings to type long numbers on an awkward keyboard, numbers which many potential MPD users (the sort who might be impressed by someone's MPD system enough to go out and by something, but end up with Sonos because they'd be out of their depth) would have to look up or ask their networking literate friend to find, not knowing (the fools) such things of the top of their heads.

The second purpose to zeroconf support in an MPD client would be easy multi-room support, a feature that there has definitely been some interest in. The MPD server doesn't really help with this, but a simple setup of separate MPD servers sharing a music collection can be controlled with minimal fuss using the zeroconf activity, whereas manual configuration for this would not only be tedious but require settings management UI that we do not currently have.

Kind regards.

@abarisain

This comment has been minimized.

Show comment
Hide comment
@abarisain

abarisain Sep 2, 2013

Owner

I get why zeroconf is needed, and this is also why (a long time ago) I started working on it. It's simple and amazing technology, and checking out what your ip is manually gets old fast.

I believe there is a middle ground. I've got a huge legacy MPDroid userbase. They are used to these settings and I don't want to change that. Or at least not that way.
The goal is to merge your autodetection list with the old config settings in one view.

One thing I also have to think about is how to deal with the per-wifi settings ? People should be able to use the zeroconf for filling up their per-network configuration.

Ideally, MPDroid needs a new server wizard. Asking questions like "Do you want this server configuration to only apply on X wireless network ?" "Does it require a password ?" (currently, the autodetection doesn't even ask for a password, you get thrown into the main activity and connection fails.)

For the actionbar button, well ... I need to rework this window anyway, since I want to always have access to Now Playing without loosing where I am in the library.

The only thing that keeps me from merging this is that, and I'm not sure why even after reviewing every single changed line, MPDroid is less stable and freezes a lot more. Maybe it's my crappy home wireless, maybe not. But when I reverted to my versions, the problems went away :/

Owner

abarisain commented Sep 2, 2013

I get why zeroconf is needed, and this is also why (a long time ago) I started working on it. It's simple and amazing technology, and checking out what your ip is manually gets old fast.

I believe there is a middle ground. I've got a huge legacy MPDroid userbase. They are used to these settings and I don't want to change that. Or at least not that way.
The goal is to merge your autodetection list with the old config settings in one view.

One thing I also have to think about is how to deal with the per-wifi settings ? People should be able to use the zeroconf for filling up their per-network configuration.

Ideally, MPDroid needs a new server wizard. Asking questions like "Do you want this server configuration to only apply on X wireless network ?" "Does it require a password ?" (currently, the autodetection doesn't even ask for a password, you get thrown into the main activity and connection fails.)

For the actionbar button, well ... I need to rework this window anyway, since I want to always have access to Now Playing without loosing where I am in the library.

The only thing that keeps me from merging this is that, and I'm not sure why even after reviewing every single changed line, MPDroid is less stable and freezes a lot more. Maybe it's my crappy home wireless, maybe not. But when I reverted to my versions, the problems went away :/

@eisnerd

This comment has been minimized.

Show comment
Hide comment
@eisnerd

eisnerd Sep 2, 2013

I agree entirely with the points you've raised about working with both the manual settings and zeroconf. This patch is definitely not where I'd leave it if I had more time to put into it now or soon. A combined view could definitely make a better UI, especially if it avoided seeming clutter or intimidating to newcomers.

The patch as it stands does not replace the manual settings screens or the per-wifi/global split. The settings can be accessed in the same way as before, or via the zeroconf activity. The zeroconf activity sets the settings for the current wifi connection, so, were you to simply visit each wifi network and use zeroconf, you would end up with a set of per-wifi settings that would switch just as if set up manually. A global setting, it seems to me, would most likely be remote access over the internet, whether someone else's wifi or cellular, in which scenario zeroconf is unlikely to be any use. Usually, zeroconf will be advertising a local IP address, so even having the option to store it as global rather than per-wifi would not get you closer to remote access.

The question about passwords is a more realistic one. Still, they are used more for remote access, where the manual settings dialog, accessed in the same way as before, will likely have to be used for all fields. It would be simple for the zeroconf activity to attempt a connect and prompt for a password on failure, for the case where a password was being used even for the local connections.

I've had a similar gripe with the Library <-> Now Playing navigation. I used to use rockbox with a custom keybinding, and that gave quite a fluid way of navigating. It seems like achieving the same thing here would mean restructuring quite a bit.

I hadn't noticed a change in stability. I have had it crash, but I was putting that down to my phone's dire shortage of RAM. Perhaps the change of project/build structure has had an effect, i.e. building JmDNS as an android library project. Without doing this, I was struggling to get past class not found errors at runtime.

eisnerd commented Sep 2, 2013

I agree entirely with the points you've raised about working with both the manual settings and zeroconf. This patch is definitely not where I'd leave it if I had more time to put into it now or soon. A combined view could definitely make a better UI, especially if it avoided seeming clutter or intimidating to newcomers.

The patch as it stands does not replace the manual settings screens or the per-wifi/global split. The settings can be accessed in the same way as before, or via the zeroconf activity. The zeroconf activity sets the settings for the current wifi connection, so, were you to simply visit each wifi network and use zeroconf, you would end up with a set of per-wifi settings that would switch just as if set up manually. A global setting, it seems to me, would most likely be remote access over the internet, whether someone else's wifi or cellular, in which scenario zeroconf is unlikely to be any use. Usually, zeroconf will be advertising a local IP address, so even having the option to store it as global rather than per-wifi would not get you closer to remote access.

The question about passwords is a more realistic one. Still, they are used more for remote access, where the manual settings dialog, accessed in the same way as before, will likely have to be used for all fields. It would be simple for the zeroconf activity to attempt a connect and prompt for a password on failure, for the case where a password was being used even for the local connections.

I've had a similar gripe with the Library <-> Now Playing navigation. I used to use rockbox with a custom keybinding, and that gave quite a fluid way of navigating. It seems like achieving the same thing here would mean restructuring quite a bit.

I hadn't noticed a change in stability. I have had it crash, but I was putting that down to my phone's dire shortage of RAM. Perhaps the change of project/build structure has had an effect, i.e. building JmDNS as an android library project. Without doing this, I was struggling to get past class not found errors at runtime.

@abarisain

This comment has been minimized.

Show comment
Hide comment
@abarisain

abarisain Sep 2, 2013

Owner

My bad, I thought it was writing on the global settings. I did notice that the old settings are still available.

Sounds good to me. I still don't know if I am gonna merge this in my master or in another branch. Sadly, I don't have a lot of time to spend on MPDroid these days (I think that you noticed that)

The Now Playing restructuration will have to be done, but frankly, I have no idea of how to achieve this while keeping the feature set intact. This is another discussion though.

Concerning the stability, I'm still trying hard to get rid of the crashes. In my experience none have been added by your patches, but I did notice some freezes after going in the Zeroconf activity.

Thanks for the patches !

Owner

abarisain commented Sep 2, 2013

My bad, I thought it was writing on the global settings. I did notice that the old settings are still available.

Sounds good to me. I still don't know if I am gonna merge this in my master or in another branch. Sadly, I don't have a lot of time to spend on MPDroid these days (I think that you noticed that)

The Now Playing restructuration will have to be done, but frankly, I have no idea of how to achieve this while keeping the feature set intact. This is another discussion though.

Concerning the stability, I'm still trying hard to get rid of the crashes. In my experience none have been added by your patches, but I did notice some freezes after going in the Zeroconf activity.

Thanks for the patches !

@abarisain

This comment has been minimized.

Show comment
Hide comment
@abarisain

abarisain Oct 24, 2013

Owner

https://play.google.com/store/apps/details?id=org.musicpd.android

Well this solves the pull request problem. I guess I had it coming, since I didn't merge it.
I'm fine with forks, but releasing it with the same icon (and it appears when you search for MPDroid) and half assedly replacing every MPDroid string with Mupeace (at least remove the donation button, it seems to point to a non existing page on my server ...) without even telling me is not a very nice thing to do.
The fork is really outdated, but thumbs up for keeping 2.x compatibility.

Don't worry, I don't want you to take any action, and I will not do anything either. This is how open source works, but I am just pretty pissed.

Owner

abarisain commented Oct 24, 2013

https://play.google.com/store/apps/details?id=org.musicpd.android

Well this solves the pull request problem. I guess I had it coming, since I didn't merge it.
I'm fine with forks, but releasing it with the same icon (and it appears when you search for MPDroid) and half assedly replacing every MPDroid string with Mupeace (at least remove the donation button, it seems to point to a non existing page on my server ...) without even telling me is not a very nice thing to do.
The fork is really outdated, but thumbs up for keeping 2.x compatibility.

Don't worry, I don't want you to take any action, and I will not do anything either. This is how open source works, but I am just pretty pissed.

@abarisain abarisain closed this Oct 24, 2013

@ZenithDK

This comment has been minimized.

Show comment
Hide comment
@ZenithDK

ZenithDK Oct 24, 2013

I noticed the fork as well and checkout the app, it does seem to have some nice changes.
In all fairness, you have been pretty quiet until a few days ago, so I wouldn't blame him for forking it.
I don't know the license on the logo, but yeah, a minor change to it or changing it completely would have been a nice gesture.

But why kill what is a really nice feature over this? I use Mopidy as my backend so I can't use it, but I can definitely see other people having good use of it.

ZenithDK commented Oct 24, 2013

I noticed the fork as well and checkout the app, it does seem to have some nice changes.
In all fairness, you have been pretty quiet until a few days ago, so I wouldn't blame him for forking it.
I don't know the license on the logo, but yeah, a minor change to it or changing it completely would have been a nice gesture.

But why kill what is a really nice feature over this? I use Mopidy as my backend so I can't use it, but I can definitely see other people having good use of it.

@abarisain

This comment has been minimized.

Show comment
Hide comment
@abarisain

abarisain Oct 24, 2013

Owner

I don't want to implement it that way (I find the UX shitty, and MPDroid's UX is already bad, I don't want to make it worse). I tried multiple times to integrate a profile support while not removing features for the people using the "per-WiFi" settings, and it's really hard, so I just left it that way.

I also don't want to have anything to do with this fork. Why ?

Well there is the play store release with the same icon and screenshots for one. There is no license on the logo, and I'm probably even violating a creative commons one since I lost the origin of the files I mixed (if the icon authors find me, I will be pleased to fix this).
It's not about the icon license, it's about etiquette.

If I look past that : it's broken on my tablet (it just says "Please restart the application" when I press back on a library panel), outdated and when I digged the code I noticed it was probably sending reports to a 3rd party server without even telling me.

https://github.com/eisnerd/dmix/blob/classical-gingerbread/main/src/org/musicpd/android/MPDApplication.java#L42

I have been pretty quiet about this project for some years, only popping up for some updates here and there. As I'm said, I'm not opposed to a fork (after all, this is a fork of PMix). This comes with open source, and I know it. If you like the fork better, just use it. I do this project for fun on my spare time, and it's not like I'm trying to push any paid app or service on you.
Wanna fork our apps and mix them into a super-great mpdroid ? Go ahead !

Anyway, I'm sure some of you will find that I am overreacting, but that's it.

Owner

abarisain commented Oct 24, 2013

I don't want to implement it that way (I find the UX shitty, and MPDroid's UX is already bad, I don't want to make it worse). I tried multiple times to integrate a profile support while not removing features for the people using the "per-WiFi" settings, and it's really hard, so I just left it that way.

I also don't want to have anything to do with this fork. Why ?

Well there is the play store release with the same icon and screenshots for one. There is no license on the logo, and I'm probably even violating a creative commons one since I lost the origin of the files I mixed (if the icon authors find me, I will be pleased to fix this).
It's not about the icon license, it's about etiquette.

If I look past that : it's broken on my tablet (it just says "Please restart the application" when I press back on a library panel), outdated and when I digged the code I noticed it was probably sending reports to a 3rd party server without even telling me.

https://github.com/eisnerd/dmix/blob/classical-gingerbread/main/src/org/musicpd/android/MPDApplication.java#L42

I have been pretty quiet about this project for some years, only popping up for some updates here and there. As I'm said, I'm not opposed to a fork (after all, this is a fork of PMix). This comes with open source, and I know it. If you like the fork better, just use it. I do this project for fun on my spare time, and it's not like I'm trying to push any paid app or service on you.
Wanna fork our apps and mix them into a super-great mpdroid ? Go ahead !

Anyway, I'm sure some of you will find that I am overreacting, but that's it.

@eisnerd

This comment has been minimized.

Show comment
Hide comment
@eisnerd

eisnerd Oct 24, 2013

Arnaud and all, I'm sure you understand already that the play store release is neither to spite you nor to detract from MPDroid development. You are welcome to incorporate any changes from Mupeace and use any icon, even one not released under the Apache License. You could easily make Mupeace obsolete, should you care. I left all donation and contributor information unchanged so as to encourage any support there might be to be directed to your development effort rather than mine.

As you have observed, I have not tried particularly hard to remove all bugs on all targets, though, funnily enough, I fixed the one you mention a while ago and was thinking of releasing this afternoon. It also lacks your later UI improvements related to dropping Gingerbread compatibility. I think the rationale here is simple and more or less set out in the store listing.

The crux of Mupeace' existence is that I wanted something to point visitors to should they want to use my multi-room system while in my house. Doing this with MPDroid is just too tedious. I would have continued using my fork as a private build were it not for this factor. This particular pull request would have made a difference, though my other weird hacks to the Library make my collection much easier to browse.

I wish you and other open source contributors success in your endeavours. Peace.

eisnerd commented Oct 24, 2013

Arnaud and all, I'm sure you understand already that the play store release is neither to spite you nor to detract from MPDroid development. You are welcome to incorporate any changes from Mupeace and use any icon, even one not released under the Apache License. You could easily make Mupeace obsolete, should you care. I left all donation and contributor information unchanged so as to encourage any support there might be to be directed to your development effort rather than mine.

As you have observed, I have not tried particularly hard to remove all bugs on all targets, though, funnily enough, I fixed the one you mention a while ago and was thinking of releasing this afternoon. It also lacks your later UI improvements related to dropping Gingerbread compatibility. I think the rationale here is simple and more or less set out in the store listing.

The crux of Mupeace' existence is that I wanted something to point visitors to should they want to use my multi-room system while in my house. Doing this with MPDroid is just too tedious. I would have continued using my fork as a private build were it not for this factor. This particular pull request would have made a difference, though my other weird hacks to the Library make my collection much easier to browse.

I wish you and other open source contributors success in your endeavours. Peace.

@eisnerd

This comment has been minimized.

Show comment
Hide comment
@eisnerd

eisnerd Oct 24, 2013

Whoops, yes, I did screw up the in-app donate url. I could have sworn I saw that and left it out. Anyway, I will revert it for the next release.

If anyone were interested in using Mopidy on Linux with a Zeroconf-enabled client, he could, in lieu of mopidy/mopidy#39, which would solve this properly, use a static avahi config like http://mpd.wikia.com/wiki/Hack:avahi_mpd.service. This is what MPD users did before it was integrated into the server. I would imagine there's a way to do this with Bonjour on OS X.

eisnerd commented Oct 24, 2013

Whoops, yes, I did screw up the in-app donate url. I could have sworn I saw that and left it out. Anyway, I will revert it for the next release.

If anyone were interested in using Mopidy on Linux with a Zeroconf-enabled client, he could, in lieu of mopidy/mopidy#39, which would solve this properly, use a static avahi config like http://mpd.wikia.com/wiki/Hack:avahi_mpd.service. This is what MPD users did before it was integrated into the server. I would imagine there's a way to do this with Bonjour on OS X.

@abarisain

This comment has been minimized.

Show comment
Hide comment
@abarisain

abarisain Oct 24, 2013

Owner

Hi ! Sorry about my initial post, I think I should haven't gone public while still mad.

I know that the release is not a personnal attack or anything. It's just so that other users can use your features (the demand for Zeroconf is really high, and I've failed at providing it for the past two years, even rejected your pull request).
I also understand the frustration caused by my slow development. I don't want to make Mupeace obsolete, and I would not even have any interest in doing so, your library browsing is going at another direction than mine.

It's great that you provide an up to date version to Gingerbread users and another way to browse the library.

I really don't mind the forking, it is justified ! What I didn't like is the execution. I think that it is confusing.
Users will just pick the app that fits them best. After all Android (and open source) is all about choice.
The Apache license even allows anybody to fork it, charge for it and not even release the source ... So yeah, anything open source is great.

Again, sorry about that.

About the donations : It's just that it is a dead link. I added that button because a lot of people suggested me to do so, but I'm really embarassed about them. MPDroid is made mainly by me, but also benefit from many, many improvements made by other devs. You are one of them, and a nice (still not complete) list is in the README.
I don't really feel well about taking donations when they should just be distributed between us.

I think I will remove the button once and for all, but this has nothing to do with your fork. They make me feel uneasy.

TL;DR : Sorry I got mad, go on and enjoy the freedom a fork brings you :) That's how I was able to start MPDroid after all.
Of course you are still welcome to pull from my trunk, and I will be sure to credit you if I use your code at any point.

Owner

abarisain commented Oct 24, 2013

Hi ! Sorry about my initial post, I think I should haven't gone public while still mad.

I know that the release is not a personnal attack or anything. It's just so that other users can use your features (the demand for Zeroconf is really high, and I've failed at providing it for the past two years, even rejected your pull request).
I also understand the frustration caused by my slow development. I don't want to make Mupeace obsolete, and I would not even have any interest in doing so, your library browsing is going at another direction than mine.

It's great that you provide an up to date version to Gingerbread users and another way to browse the library.

I really don't mind the forking, it is justified ! What I didn't like is the execution. I think that it is confusing.
Users will just pick the app that fits them best. After all Android (and open source) is all about choice.
The Apache license even allows anybody to fork it, charge for it and not even release the source ... So yeah, anything open source is great.

Again, sorry about that.

About the donations : It's just that it is a dead link. I added that button because a lot of people suggested me to do so, but I'm really embarassed about them. MPDroid is made mainly by me, but also benefit from many, many improvements made by other devs. You are one of them, and a nice (still not complete) list is in the README.
I don't really feel well about taking donations when they should just be distributed between us.

I think I will remove the button once and for all, but this has nothing to do with your fork. They make me feel uneasy.

TL;DR : Sorry I got mad, go on and enjoy the freedom a fork brings you :) That's how I was able to start MPDroid after all.
Of course you are still welcome to pull from my trunk, and I will be sure to credit you if I use your code at any point.

eisnerd added a commit to eisnerd/mupeace that referenced this pull request Feb 15, 2014

Always store Zeroconf choice as per-wifi
if connected by wifi, instead of guessing based on current settings. This relates to a discussion in abarisain/dmix#256 and makes it easier to get sensible, per-wifi settings yet possible to use global.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment