Added filter to limit the search results to stations or shows

kingosticks / mopidy-tunein

Mopidy extension for playing music from tunein

Apache License 2.0

65 stars 14 forks source link

Added filter to limit the search results to stations or shows #34

Closed foxfabi closed 5 years ago

foxfabi commented 5 years ago

Hello, this is my proposition for #1 Let me know what you think about it.

kingosticks commented 5 years ago

Thanks for taking a look. Do all requests support filtering?

kingosticks commented 5 years ago

It would also be quite nice if we could somehow extend mopidy_to_tunein_query() to handle this so users can set the filter dynamically rather than having to hardcode it in their config. It'd be a bit ugly but we could shoehorn this into the comment, or maybe even the genre field (makes the most sense of any of them). I'm not totally sold on this idea, feel free to shoot it down.

foxfabi commented 5 years ago

Thanks for taking a look. Do all requests support filtering?

Yes, i can also limit a browse request only to shows or stations.

http://opml.radiotime.com/Browse.ashx?id=c57941 http://opml.radiotime.com/Browse.ashx?id=c57941&filter=p http://opml.radiotime.com/Browse.ashx?id=c57941&filter=s

The 'filter' parameter affect the behavior for search and browse, but I have only considered him for the search.

foxfabi commented 5 years ago

did you notice? i'm still learning git and python 😄

kingosticks commented 5 years ago

Having finally gotten around to trying this out I see this filtering applied to everything breaks the results in some cases. For example:

Browse -> By Location -> Europe -> United Kingdom -> Liverpool

Returns plenty of results from TuneIn (http://opml.radiotime.com/Browse.ashx?render=json&id=r102038&filter=s) but Mopidy-Tunein doesn't show anything. Contrast that with London (http://opml.radiotime.com/Browse.ashx?render=json&id=r102038&filter=s) which does work and you can see the difference in the results format and why _flatten() goes wrong . The TuneIn API is total garbage.

foxfabi commented 5 years ago

The pull request did not change the browser function or _flatten.

I can reproduce the described issue without applying this filtering to browse... Mopidy-Tunein with Iris frontend: Browse> Music> 70's (works) Mopidy-Tunein on Iris frontend: Browse> Music> 80's (empty result) even though http://opml.radiotime.com/Browse.ashx?id=c100000781 delivers a result

Why there is no open issue about this? What is your suggestion regarding further action?

kingosticks commented 5 years ago

Correct, you didn't change flatten, that's not what I said. But the way it's implemented it adds the filter param to every request. So, yes, it did change the browse results. I'll have a go at fixing it tomorrow.

foxfabi commented 5 years ago

With the last commit now both Music> 80's and Liverpool show the corresponding results.

kingosticks commented 5 years ago

If we only want to apply the filter for searching, it should just be added to the args in search().

I'll have another round of testing this weekend just to make sure there are no search queries that end up with the same issue. If memory serves the search results are always in the same flat format so it should be OK. Having this filter feature available for browsing would be nice to get working but we can sort that out separately. I like this feature, I personally don't want the podcasts in my results - thanks!

kingosticks commented 5 years ago

Merged in https://github.com/kingosticks/mopidy-tunein/commit/514ef1959ed51c47c402fb307b63bf3536e914e7 with some adjustments. Thanks.