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.

Sign up for GitHub

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

Jump to bottom

self.codec is not set appropriately on Mac #468

Open
xsdg opened this issue May 27, 2024 · 7 comments
Open

self.codec is not set appropriately on Mac #468

xsdg opened this issue May 27, 2024 · 7 comments

Comments

@xsdg
Copy link
Collaborator

xsdg commented May 27, 2024

Found by @muammar in #466

muammar@pupa ~/git/mkchromecast [master]
± % bin/mkchromecast --debug --discover                                                                                                                                                                         !3222
Traceback (most recent call last):
  File "/Users/muammar/git/mkchromecast/bin/mkchromecast", line 285, in <module>
    mkcc = mkchromecast.Mkchromecast()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/muammar/git/mkchromecast/bin/../mkchromecast/__init__.py", line 194, in __init__
    elif self.codec in constants.CODECS_WITH_BITRATE:
         ^^^^^^^^^^
AttributeError: 'Mkchromecast' object has no attribute 'codec'. Did you mean: 'vcodec'?

The issue is here:

mkchromecast/mkchromecast/__init__.py

Line 144 in 1d14cdf

elif self.backend == "node":

On Mac, the default backend is "node" and the default codec is "mp3", which is the only codec supported by the node backend. When args.codec is set to anything else, we override and set self.codec to "mp3", which is correct. But when args.codec is correct, we never copy it into self.codec, and self.codec remains unset.
@xsdg
Copy link
Collaborator Author

xsdg commented May 27, 2024

@psmgeelen Is this something you'd like to work on? No worries if not. If so, let me know how much guidance would be helpful.

@psmgeelen
Copy link
Contributor

@xsdg Yes! I would be. In terms of support, I would like to point out that I don't own a Mac, even though by the look of this, it doesn't seem that I really need one. Let me review properly this weekend and get back to you with a PR. What was the formatting again so I can connect my PR to the issue?

@xsdg
Copy link
Collaborator Author

xsdg commented May 28, 2024

Sounds good. I would suggest writing a new unit test that replicates this issue (see https://github.com/muammar/mkchromecast/blob/master/tests/test_instantiate.py ), and then making sure that your fix causes that unit test to start passing.

As for connecting things, you can mention an issue with #<issue number>. As for the actual integrations, see https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

@psmgeelen psmgeelen mentioned this issue Jun 1, 2024
Merged
@psmgeelen
Copy link
Contributor

@xsdg I hope this is what you needed :)

@xsdg
Copy link
Collaborator Author

xsdg commented Jun 3, 2024

@muammar Could you confirm whether this bug is fixed?

@psmgeelen
Copy link
Contributor

@xsdg Let me know if you have something else for me to fix ;)

@muammar
Copy link
Owner

muammar commented Jun 7, 2024

@muammar Could you confirm whether this bug is fixed?

It's fixed. I wanted to find the problem with the Sonos list not being populated, but got caught up with some stuff at work. Thanks for the follow up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@xsdg @muammar @psmgeelen