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

Feature Request - Shuffle Mode #66

Open
ghostbuster84 opened this issue Feb 10, 2022 · 5 comments
Open

Feature Request - Shuffle Mode #66

ghostbuster84 opened this issue Feb 10, 2022 · 5 comments

Comments

@ghostbuster84
Copy link

ghostbuster84 commented Feb 10, 2022

Hello, the jellyfin-skill port has shuffle mode. I ported this skill to implement it and once I have it working I would like to merge it with the main skill. I'm happy to help with anything else. I did manage to get a Jellyfin skill working since @tuxfoo's stopped working. https://github.com/ghostbuster84/jellyfin-skill https://github.com/ghostbuster84/emby-skill

@rickyphewitt
Copy link
Owner

Hey @ghostbuster84!
Having a shuffle mode would be great! Feel free to open a PR with the changes and I'll take a look.

How different is jellyfin from Emby? I'm not opposed to supporting both servers in this skill if the changes are minimal. We could include an option in the config that would enable 'jellyfin support' or something like that.

What do you think?

@ghostbuster84
Copy link
Author

I'm still working on getting the shuffle part working. It works fine in jellyfin, but I can't get it working in enby. (Not sure where i went wrong trying to merge the skills) I think adding jellyfin support would be nice. I don't think much is different as jellyfin is forked from emby. I will try to see if I can get the emby skill to just access jellyfin without any changes to the code. Then the process would be easier as we would just have to change the calls and verbal responses.

@ghostbuster84
Copy link
Author

I am still working on getting it to work. My family is remodeling our kitchen so its been busy.

@rickyphewitt
Copy link
Owner

No problem! Do you want to link a branch that you are working on? I can also look into a shuffle mode in Emby and see what the differences may be. I've been thinking about the best way to support both services (and potentially more flavors as Emby is open source). Here are a few things that might make sense to implement/consider.

  • Handling Multiple API Clients
    • Create a common base client with as much of the shared code as possible. Then create an Emby and Jellyfin client that overrides/add the custom areas.
    • We could even make the base client an interface and then force all the clients implement the common methods. This would allow the skill to talk to a consistent api and it doesn't have to know what underlying client is being used.
    • A bit of both (inheritance/interface) might be the best approach.
  • Customizing the Phrases
    • I think the best approach here would be to make the phrases mycroft says as generic as possible.
    • The tricky part her is that the intents "play {song} from Emby" would all have to be updated to include Emby and JellyFin. Another thought here is that we wouldn't want you to say "play {song} from Jellyfin when you are using Emby. (though I don't think this is a showstopper) If we add another variant of Emby we'd have to update this.
    • Though now that the Common Play Framework is integrated users don't have to specify "Emby" when wanting to play from it.
  • Testing
    • Right now the testing is good, but not great. I run a suite of local tests against a real emby server and verify things are working. I then grab responses and use mock's to simulate an emby server when running in CI
    • We would need to ensure we test against both an emby and jellyfin server. Again we'd want to ensure that we don't just copy/paste the test classes. We might be able to use pytest parameterize or similar to run each client through the same test code
    • In the end I'd like to be able to spin up an emby and jellyfin server in CI via docker with a set of sample music/data and run live tests against that.

I don't think we need to implement all of the above before we add jellyfin support. But as we are adding jellyfin support we should consider these scenarios and move toward a common goal/architecture.

Enjoy the kitchen remodel 😄

@ghostbuster84
Copy link
Author

ghostbuster84 commented Feb 20, 2022

On my fork of the emby skill, i only have the main branch. I've just commited changes as i go. I know the shuffling works in jellyfin in my jellyfin repo (forked from tuxfoo which is a fork from your emby skill). I am trying to merge the shuffle intents from there, but it's taking some time. As for the intents (play _______ from emby/jellyfin) All of that is set up in the jellyfin skill and i will try to merge to the emby skill. Feel free to check it over. Once i get shuffling working I will create a pull request. I also invited you to be a creator in my fork of the emby skill so you don't have to worry about breaking the main repo.

Also I have both a jellyfin and emby server set up at home so i can test both. Any test i do i either run on my PC running Zorin OS 16 or on my 5 picrofts using the google AIY voicekits for speaker/mic.

@ghostbuster84 ghostbuster84 changed the title Feature Request Feature Request - Shuffle Mode Feb 21, 2022
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

No branches or pull requests

2 participants