LGTM! Thanks!

• Hopefully they make a new release soon :3
• Do you know how to get notifications working? Adding pygobject3 and libnotify to propagatedBuildInputs didn't seem enough. Not a deal-breaker but thought I'd ask 😁.

I haven't had much success in making notifications work. Someone has a minimal example that mimicks how it's done in mkchromecast but if you add --pure to that it emits gi.repository.GLib.Error: g-exec-error-quark: Failed to execute child process “dbus-launch” (No such file or directory) (8). The system tray icon still works, it only won't emit notifications during the Chromecast device scan as far as I can tell.

Should've addressed the CR feedback now. Let me know if there's anything I need to change

Pushed changes following most feedback above.

How do we version/name a package based on a master branch? Do we add it to all-packages.nix as mkchromecastMaster and pin it against the specific commit master is at now? Edit: looks like that's the general behaviour based on some other packages' Nix derivations.

We could also mimic other packages by adding a master.nix, beside the stable release, but I do agree that we need to package master somehow because it's been a while since mkchromecast made a release.

We can just keep calling the attribute mkchromecast and set the version to unstable-YYYY-MM-DD, where the date is that of the commit used.

However, if using master turns out to be just as complicated, I think we can go with it as it is right now.

I'm happy to go ahead with this PR, and then give packaging master a try sometime later this week.

Alright, but you still have to change the version of PyChromecast used.

@dotlambda the tests here are actually I/O bound through testing an actual Chromecast Audio device ("CCA"), I don't think we want that, right? I've just added a further comment explaining why it's disabled.

(sorry for the silly diff, relevant change is on line 52)

Good to see this PR already being submitted. Looks very good :D master is actually more stable and supports the latest PyChromecast as well. Since it has been more than a month since the request for a new version, I'd suggest using the latest master. You can find the changes in #59280.

So can this PR be closed now as the package was already added? Any reason to keep it open @Shou?

@aanderse it hasn't been merged yet, the above PR was closed. I'm just waiting for feedback here

@Shou Ah sorry, misread. I was just trying to triage.

@dotlambda seeing as you have had the most review on this PR is there anything outstanding from your perspective? Do you have any feedback for @Shou?

@bobvanderlinden or @dtzWill Are either of you able to provide the requested feedback here?

This version works for me! Tested on NixOS with older Chromecast. This version did not work for me. It raises an exception when casting videos or casting the screen. Command line for videos:

result/bin/mkchromecast --port 8080 --video -i myvideo.mp4

Command line for casting screen:

result/bin/mkchromecast --port 8080 --video --screencast --resolution 720p

[2019-07-14 18:48:26,269] ERROR in app: Exception on /stream [GET]
Traceback (most recent call last):
  File "/nix/store/4hn5lqcl115d90xik5v97nj1z246gq7h-python3.7-Flask-1.0.2/lib/python3.7/site-packages/flask/app.py", line 2292, in wsgi_app
    response = self.full_dispatch_request()
  File "/nix/store/4hn5lqcl115d90xik5v97nj1z246gq7h-python3.7-Flask-1.0.2/lib/python3.7/site-packages/flask/app.py", line 1815, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/nix/store/4hn5lqcl115d90xik5v97nj1z246gq7h-python3.7-Flask-1.0.2/lib/python3.7/site-packages/flask/app.py", line 1718, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/nix/store/4hn5lqcl115d90xik5v97nj1z246gq7h-python3.7-Flask-1.0.2/lib/python3.7/site-packages/flask/_compat.py", line 35, in reraise
    raise value
  File "/nix/store/4hn5lqcl115d90xik5v97nj1z246gq7h-python3.7-Flask-1.0.2/lib/python3.7/site-packages/flask/app.py", line 1813, in full_dispatch_request
    rv = self.dispatch_request()
  File "/nix/store/4hn5lqcl115d90xik5v97nj1z246gq7h-python3.7-Flask-1.0.2/lib/python3.7/site-packages/flask/app.py", line 1799, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/nix/store/1fb58idhqg2904imzcss7jid3nr1f0yn-mkchromecast-0.3.8.1/lib/mkchromecast/video.py", line 225, in stream
    process = Popen(command, stdout=PIPE, bufsize=-1)
  File "/nix/store/gbwx5sc4w0i0ipnmsa0nsk88svs983vl-python3-3.7.2/lib/python3.7/subprocess.py", line 775, in __init__
    restore_signals, start_new_session)
  File "/nix/store/gbwx5sc4w0i0ipnmsa0nsk88svs983vl-python3-3.7.2/lib/python3.7/subprocess.py", line 1522, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'ffmpeg': 'ffmpeg'

Might still be fine for now to init at 0.3.8.1 and hope that a new release will come soonish.

EDIT: After installing ffmpeg into my path it works fine. What I find interesting is that my PR didn't have this problem. I also tried updating pychromecast to 3.2.0, but that resulted in the same exception. I guess this issue was resolved in master (compared to 0.3.8.1). For 0.3.8.1 I'd say adding ffmpeg to the propagated builds is required for this tool to work correctly.

Thanks @bobvanderlinden! @Shou sound good?

@aanderse @bobvanderlinden have done that now

Thanks @Shou! @bobvanderlinden Hopefully you'll be able to test and merge this at some point... unfortunately I don't have the hardware available currently to do so.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/110

Hello, I'm a bot and I thank you in the name of the community for your contributions.

Nixpkgs is a busy repository, and unfortunately sometimes PRs get left behind for too long. Nevertheless, we'd like to help committers reach the PRs that are still important. This PR has had no activity for 180 days, and so I marked it as stale, but you can rest assured it will never be closed by a non-human.

If this is still important to you and you'd like to remove the stale label, we ask that you leave a comment. Your comment can be as simple as "still important to me". But there's a bit more you can do:

If you received an approval by an unprivileged maintainer and you are just waiting for a merge, you can @ mention someone with merge permissions and ask them to help. You might be able to find someone relevant by using Git blame on the relevant files, or via GitHub's web interface. You can see if someone's a member of the nixpkgs-committers team, by hovering with the mouse over their username on the web interface, or by searching them directly on the list.

If your PR wasn't reviewed at all, it might help to find someone who's perhaps a user of the package or module you are changing, or alternatively, ask once more for a review by the maintainer of the package/module this is about. If you don't know any, you can use Git blame on the relevant files, or GitHub's web interface to find someone who touched the relevant files in the past.

If your PR has had reviews and nevertheless got stale, make sure you've responded to all of the reviewer's requests / questions. Usually when PR authors show responsibility and dedication, reviewers (privileged or not) show dedication as well. If you've pushed a change, it's possible the reviewer wasn't notified about your push via email, so you can always officially request them for a review, or just @ mention them and say you've addressed their comments.

Lastly, you can always ask for help at our Discourse Forum, or more specifically, at this thread or at #nixos' IRC channel.

It would be a shame of this wasn't merged.

@copumpkin @zimbatm @aanderse @dotlambda

Tested this with my Chromecast Audio but ran into what appears to be this issue: muammar/mkchromecast#308. Seems like it's probably just an issue with mkchromecast itself, though. It does run and connect to the Chromecast at least.