search

synesthesiam / homeassistant-satellite

Streaming audio satellite for Home Assistant
MIT License
188 stars 23 forks source link

[WIP] Add support for miniaudio #47

Closed bertybuttface closed 1 year ago

bertybuttface commented 1 year ago

WARNING: this is currently untested, I will test it tomorrow when I'm awake.

Can merge this PR, #46 or both.

bertybuttface commented 1 year ago

@beqabeqa473 if you have time to test this please have at it :)

beqabeqa473 commented 1 year ago

AttributeError: module 'miniaudio' has no attribute 'StreamInfo'

beqabeqa473 commented 1 year ago

Are you sure you correctly understood how miniaudio works? It is not like pyaudio. In the future test your work before asking someone to test.

bertybuttface commented 1 year ago

This is clearly marked as WIP and I clearly called it out as untested. I have not run miniaudio even once. You seem to have experience with it though?

I wrote this on my phone at 2AM because you didn't like my other PR using PyAudio (which I might add works perfectly).

Can you try being a bit nicer? You are not my boss. If my code has a bug how about you help me fix it rather than telling me off because you aren't happy with it.

bertybuttface commented 1 year ago

In fact actually I can't be bothered. Do it yourself if you want pyminiaudio that badly. I've done most of the work already, it will require a few simple tweaks around the callback function.

beqabeqa473 commented 1 year ago

Not a good practice to push untested code and ask to merge it.

On 10/22/23, bertybuttface @.***> wrote:

In fact actually I can't be bothered. Do it yourself if you want pyminiaudio that badly.

-- Reply to this email directly or view it on GitHub: https://github.com/synesthesiam/homeassistant-satellite/pull/47#issuecomment-1774047952 You are receiving this because you were mentioned.

Message ID: @.***>

-- with best regards Beqa Gozalishvili Tell: +995593454005 Email: @.*** Web: https://gozaltech.org Skype: beqabeqa473 Telegram: https://t.me/gozaltech facebook: https://facebook.com/gozaltech twitter: https://twitter.com/beqabeqa473 Instagram: https://instagram.com/beqa.gozalishvili

bertybuttface commented 1 year ago

Do you know what the [WIP] tag means? I pushed this so we can collaborate.

You have interacted with me 4 times across 2 pull requests and 3 of those interactions were negative.

Is the way you are choosing to interact likely to help us get this working or is it likely to alienate other volunteers?

Are you going to help or are you just going to criticise? I'd be happy to reopen this if you can adjust your attitude.

beqabeqa473 commented 1 year ago

Nothing negative.

I gave you an advice to use miniaudio, because it is actively maintained and has wider support of audio subsystems. It also has eliminated some bugs which are still appear in pyaudio. As for wip, i understand that, but then you should not ask to merge this pr before testing it and asking someone to test when it definitely is incomplete.

bertybuttface commented 1 year ago

How can I create a PR and mark it as WIP without creating a PR?

I asked for your assistance because I was too tired to continue writing code, I marked this PR as WIP, I put a clear WARNING disclaimer saying it was untested and I said "If you have time to test this please test it". I couldn't have been clearer.

I was hoping you would respond in the spirit of open source and pickup the baton where I left it. Instead you've come on here and made multiple negative comments.

What is your desired goal here? For us to continue working on this PR and get it merged for everyone to use or do you think making snide comments from the sideline is likely to motivate me to do so?

My plan was to wake up and finish a bit before work, instead I read your comments and get into this with you.

beqabeqa473 commented 1 year ago

It is up to you. I am still ready to test, but from my point of view, it is not a good practice to push code which cannot be run from the beginning. i know that it is not possible to think of everything at once, and code can have bugs, but pushing code that is not tested by the author doesn't make sense. That was my main concern when writing a comment. for someone it is ok, for someone not, don't be affended by that, it is ok.

bertybuttface commented 1 year ago

You skipped past two warnings (WIP in the title and WARNING in block capitals). You also skipped past the part where I asked you to test it because I had not. That is not on me, that is on you.

WIP Pull Requests are an accepted workflow, if it bothers you that much you are welcome to create your own PR.